refactor(state-table): make consistent op an enum in state table #16471
Conversation
| pub enum StateTableOpConsistencyLevel { | ||
| Inconsistent, | ||
| ConsistentOldValue, | ||
| LogStoreEnabled, | ||
| } |
There was a problem hiding this comment.
It seems that these three levels are in a progressive relationship. Could you elaborate more on how LogStoreEnabled is different from ConsistentOldValue and why we need this?
There was a problem hiding this comment.
LogStoreEnabled has the same op consistency requirement as ConsistentOldValue. The difference is that, for ConsistentOldValue, in release mode that has no old value sanity check, the behavior of ConsistentOldValue is the same as Inconsistent, which simply writes and stores only the new value to state store, but for LogStoreEnabled, we will also write and upload the old value to state store.
The old value is written and uploaded to state store so that it can be combined with the new value to replay the change log. The change log will be used in partial checkpoint and subscription.
LogStoreEnabled is not used in this PR yet. In will be used when we enable L0 log store for subscription.
| @@ -58,6 +58,21 @@ pub struct MaterializeExecutor<S: StateStore, SD: ValueRowSerde> { | |||
| conflict_behavior: ConflictBehavior, | |||
|
|
|||
| version_column_index: Option<u32>, | |||
|
|
|||
| may_have_downstream: bool, | |||
There was a problem hiding this comment.
Why this is may_have_downstream, instead of has_downstream?
There was a problem hiding this comment.
The flag only gets changed when there is newly created downstream, and won't be changed when some downstreams are dropped, so the value can be false positive.
There was a problem hiding this comment.
Sure. That's the same with my understanding. But are there any problems if we also handle the drop and re-enable the switch?
@BugenZhao mentioned this optimization
#16348 (comment)
There was a problem hiding this comment.
Previously we cannot do this because there is no enough information in the barrier with drop streaming job command. @chenzl25 Can you provide with more details?
But I think it can be implemented if we incrementally maintain a full copy of downstream information in the mv executor.
There was a problem hiding this comment.
Take AddMutation as an example. It contains pub adds: HashMap<ActorId, Vec<PbDispatcher>> to represent how many dispatchers are to be added for an actor, but for UpdateMutation, it only has dropped_actors: HashSet<ActorId> so we don't have enough information to maintain the dispatcher number for MaterializedView.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Previously, we have a bool flag
is_consistent_opin state table. After #15301, whenis_consistent_opis true, we will further have anis_log_storeto represent whether we need to write and store the old value. In this PR, we change theis_consistent_opto an enumStateTableOpConsistencyLevelwith the three enum cases.Besides, before this PR, the materialized executor need to carefully decide whether the consistency level is changed when receiving a barrier and whether to call
commitorcommit_with_switch_consistent_op. In this PR, it will change to always call a commit method and specify the expected op consistency level. The state table will change whether the op consistency level is changed, and only if changed, it then pass the switch info to state store.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.