feat(storage): per table try wait epoch #18622
Conversation
src/storage/src/hummock/utils.rs
Outdated
| @@ -606,10 +620,17 @@ pub(crate) async fn wait_for_epoch( | |||
| return Err(HummockError::wait_epoch("tx dropped").into()); | |||
| } | |||
| Ok(Ok(_)) => { | |||
| let max_committed_epoch = *receiver.borrow(); | |||
| if max_committed_epoch >= wait_epoch { | |||
| // TODO: should handle the corner case of drop table | |||
There was a problem hiding this comment.
If we don't handle the corner case in this PR, does it mean that wait_for_epoch will be blocked forever? For example, when a batch query is issued to CN but before execution the table is dropped, will the query be blocked at waif_for_epoch forever?
There was a problem hiding this comment.
Just updated the code.
A separate BatchQueryCommitted variant is introduced to HummockReadEpoch to provide an extra hummock version id to wait on for batch query. In the hummock version of the provided id, the table id must have been registered to hummock. In this way, if the table id does not exist in a later version, the table must have been dropped, and then we will return with an error.
For the wait_epoch in streaming executor, we can assume that when the streaming executor exists, its streaming job will prevent the table from being dropped, and therefore when we see that a table id does exist in the latest version, the table must not be already dropped, and then we can wait for the table id be registered in later version.
wenym1
force-pushed
the
yiming/per-table-try-wait-epoch
branch
from
228bcfe
to
c74a62a
Compare
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Previously in
try_wait_epoch, we only specify an epoch to wait, and then we wait for the global max committed epoch to bump up. In partial checkpoint, there won't be max committed epoch any more, and we wait on the committed epoch of a specific table id.For most current calls on try_wait_epoch, there is an associated table id to wait on, such as
StorageTable. The only exception is thewait_epoch_commitrpc used in configuration change. In this PR, we will pick a random existing table id to wait on, and if there is no existing table id, we can just skip calling it. After #18312, this logic will be no longer necessary.Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.