[indexer][watermarks][2/n] committer writes upper bounds to watermarks table #19649
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
wlmyng
force-pushed
the
indexer/pruner-config
branch
2 times, most recently
from
b8f4fea
to
5cdcd79
Compare
wlmyng
force-pushed
the
indexer/committer-updates-watermarks-upper-bound
branch
from
09e384e
to
2482cda
Compare
wlmyng
force-pushed
the
indexer/committer-updates-watermarks-upper-bound
branch
from
2482cda
to
7fdf941
Compare
wlmyng
force-pushed
the
indexer/pruner-config
branch
2 times, most recently
from
5065a0a
to
1157e65
Compare
wlmyng
force-pushed
the
indexer/committer-updates-watermarks-upper-bound
branch
from
7fdf941
to
03be8f5
Compare
wlmyng
force-pushed
the
indexer/committer-updates-watermarks-upper-bound
branch
from
03be8f5
to
ec26f93
Compare
…iating an indexer writer and reader into separate functions, and use those functions in tests where we previously clobbered everything into a single function.
make life easier, requires epochs_to_keep with optional overrides. otherwise, no retention policy at all strumming the pruning config. pruner.rs file will define prunable tables. the toml file when parsed into Rust config will warn if it tries to name any variants not supported by indexer code. additionally, there's also a check to make sure that prunable tables actually exist in the db
wlmyng
force-pushed
the
indexer/pruner-config
branch
from
1157e65
to
84392bf
Compare
wlmyng
force-pushed
the
indexer/committer-updates-watermarks-upper-bound
branch
from
93e6585
to
fc07d0e
Compare
wlmyng
force-pushed
the
indexer/committer-updates-watermarks-upper-bound
branch
from
fc07d0e
to
4939ddf
Compare
| @@ -212,6 +228,22 @@ async fn commit_checkpoints<S>( | |||
| }) | |||
| .expect("Persisting data into DB should not fail."); | |||
|
|
|||
| state | |||
| .update_watermarks_upper_bound( | |||
| PrunableTable::iter().collect(), | |||
There was a problem hiding this comment.
what about tables that are un-prunable while we still want to track the hi watermark, like epochs?
There was a problem hiding this comment.
i think readers can just use the available data itself as the range, if needed
There was a problem hiding this comment.
it's counter-intuitive that watermarks is actually prunable_table_watermarks -- I propose to extend that to all tables if it's not the plan.
There was a problem hiding this comment.
we can do that in a separate pr
There was a problem hiding this comment.
i see what you mean
i dont think it'll hurt for pruner to prune a subset of the tables that committer pushes upper bounds to. For those unpruned tables, the reader_lo will remain at 0. Or perhaps yeah we can have a prunable_table_watermarks instead of a flat watermarks
There was a problem hiding this comment.
yeah I think having reader_lo at 0 seems benign too, and watermarks can be more useful covering all tables, for example if we want to check if all tables have reached certain checkpoint (except snapshot table), we can go to watermarks and run one quick query to do that.
There was a problem hiding this comment.
not trying to block you, and I can do the extension too if that helps
There was a problem hiding this comment.
oh no worries, better to hash it out now when the table doesnt exist yet
i like the idea of being able to check overall instance status
## Description Currently, tests that require spinning up an indexer do so through `start_test_indexer_impl` or `start_test_indexer`. This is further complicated by a single `start` function that accepts a `ReaderWriterConfig`. We can simplify this by exposing two functions with optional parameters that can be configured by the caller. Part of a stack of PRs for watermarks 1. simplify setting up test indexer: #19663 2. update pruner config: #19637 3. committer writes upper bounds #19649 4. pruner writes lower bounds: #19650 5. pruner prunes (wip) ## Test plan How did you test the new or updated feature? --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
wlmyng
force-pushed
the
indexer/pruner-config
branch
from
84392bf
to
1bbe93b
Compare
Description
Adds
watermarkstable, and has committer writing upper bounds to it.The committer works on the checkpoints level, so to simplify what it needs to do, we can have the committer call
store.update_watermarks_upper_bound()once when it completes its batch of work. The upper bound update function relies onPrunableTableto tell it which of epoch, cp, or tx should be used to update the upper bound.Part of a stack of PRs for watermarks
Test plan
How did you test the new or updated feature?
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.