feat(sql-backend): integrate HummockManager with model V2 #14355
Conversation
zwang28
changed the title
This reverts commit 09145f0.
| @@ -75,6 +76,8 @@ pub struct MetaSrvEnv { | |||
| /// Client to connector node. `None` if endpoint unspecified or unable to connect. | |||
| connector_client: Option<ConnectorClient>, | |||
|
|
|||
| pub hummock_seq: Option<Arc<SequenceGenerator>>, | |||
There was a problem hiding this comment.
We can wrap the Option<Arc<SequenceGenerator>> as a struct HummockSequenceIdManager and provides method like fn next_compaction_task_id(&self).
| @@ -659,7 +707,12 @@ impl HummockManager { | |||
| ) -> Result<HummockSnapshot> { | |||
| let snapshot = self.latest_snapshot.load(); | |||
| let mut guard = write_lock!(self, versioning).await; | |||
| let mut pinned_snapshots = BTreeMapTransaction::new(&mut guard.pinned_snapshots); | |||
| self.check_context_with_meta_node(context_id).await?; | |||
There was a problem hiding this comment.
Question: at the very beginning version of code, we atomically check the context id when we commit to meta store. Now we only check the context id before we do the transaction. Will it cause bug if the context id becomes invalid after we have checked it but before we commit it to meta store?
There was a problem hiding this comment.
As mentioned by the comments in check_context_with_meta_node, "Caller should hold versioning lock, to sync with HummockManager::release_contexts". By adhering to that rule, correctness is guaranteed, because all callers interested in context_id are executed sequentially.
It's OK context_id has become invalid when commit to meta store. What to prevent is, e.g. pin_version after release_contexts for a invalid context.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
This PR makes HummockManager support both metadata model V1 and model V2.
create_trx_wrapperandcommit_multi_var.sql_meta_storeis provided.check_contextis moved out ofcommit_multi_var, explicitly called in pin/unpin methods.BTreeMapTransactionWrapper, which dispatches requests toBTreeMapTransaction<V1>orBTreeMapTransaction<V2>.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.