refactor: meta: reorganize txn fetched records#20033
Conversation
c18e3df to
d5828ca
Compare
🤖 CI Job Analysis
📊 Summary
❌ NO RETRY NEEDEDAll failures appear to be code/test issues requiring manual fixes. 🔍 Job Details
🤖 AboutAutomated analysis using job annotations to distinguish infrastructure issues (auto-retried) from code/test issues (manual fixes needed). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c18e3df5e1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
xp-trumpet
left a comment
There was a problem hiding this comment.
@xp-trumpet reviewed 16 files and all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on drmingdrmer).
# Summary Add a typed transaction helper for meta KV operations that records reads, stages writes, and retries failed CAS commits through a small manager API. # Details `MetaTxnManager` now drives retryable transaction closures, while `MetaTxn` keeps the read and write sets for one attempt. Reads registered with `get_for_update` become commit-time `eq_seq` guards, and fetched records can assert present or absent state before staging writes back to the same key. Dictionary rename and table-tag drop use the helper to replace hand-built transaction loops. The helper uses the KV API associated error type instead of forcing `MetaError`, and the databend-meta-client dependency is updated to the version needed by the generic transaction path.
d5828ca to
69eea3e
Compare
xp-trumpet
left a comment
There was a problem hiding this comment.
@xp-trumpet reviewed 3 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on drmingdrmer).
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
refactor: meta: reorganize txn fetched records
Summary
Transaction get results are now modeled as fetched records and the txn module layout groups related record handles and MetaTxn internals together.
Details
The former for-update handle names now use
FetchedRecord,PresentRecord, andAbsentRecord, which describes the record returned by a get operation instead of implying update behavior.get_for_updatestill names the read mode that installs the CAS guard.MetaTxn manager, read state, test helpers, and tests now live under the
meta_txnmodule, while fetched record handles live underfetched_record. Public reexports remain available from the meta API crate root.refactor: txn: encode for-update existence state
Summary
MetaTxn for-update reads now carry their asserted existence state in the handle type, so callers that require present or absent records can continue with a narrower API after the guard succeeds.
Details
some_or_unknown_errornow returns a present-only handle with direct access to the storedSeqV, whilenone_or_exist_errorreturns an absent-only handle that only supports creating the guarded record. This removes optional-value checks from callers after existence has already been validated.Table tag reference errors now report the tag name without exposing the table id, with the operation context appended to the message.
refactor: txn: generalize KV error type from MetaError to KV::Error
Summary
The transaction layer previously hard-coded
MetaErroras the KV errortype throughout
send_txn,ForUpdate,MetaTxn, andMetaTxnManager.This change propagates
KV::Error(or bounded associated types) instead,making the txn module usable with any
KVApiimplementation.Details
send_txnis now generic overKV: kvapi::KVApi + ?Sized, returningKV::Errorinstead ofMetaError.ForUpdateis split into twoimplblocks: a no-bound block forread-only accessors and
delete, and aKV::Error: From<InvalidArgument>block for
put.MetaTxngains a similar split: a no-bound block (new,delete),a
From<PbDecodeError>block (get,get_for_update,read_key), anda
From<InvalidArgument>block (put).decode_rawbecomes generic over errorE: From<PbDecodeError>insteadof returning a fixed
MetaError.MetaTxnManager::runnarrows the caller error bound fromFrom<MetaError>toFrom<KV::Error>.feat(meta): add ForUpdate existence-guard methods
Summary
Add two guard methods to the ForUpdate transaction handle that turn a read
key's presence or absence into the key's own declared error, and adopt them in
rename_dictionary. They replace the presence checks a CAS step otherwise
open-codes, while keeping the error at the key's own layer.
Details
key is absent — the precondition for a key a write depends on.
— the precondition for a key that must be free before it is created.
leaving conversion (e.g. to AppError) to the caller, so ForUpdate stays
independent of the higher-level error enums.
checks.
text; DatabaseId keys are used for their stable, concrete error Display.
feat(meta): add MetaTxn CAS transaction helper for KV API
Summary
Introduce MetaTxn, a reusable helper that models one optimistic-concurrency
transaction attempt against the KV backend, and MetaTxnManager, which drives a
closure as a compare-and-swap retry loop with a fresh transaction per attempt.
The first API, DictionaryApi::rename_dictionary, is migrated onto this model;
its observable behavior is unchanged.
Details
into eq_seq guards at commit, so a value a write depends on cannot be left
unguarded. execute() consumes self, making "commit at most once" a
compile-time guarantee. Its read and write sets sit behind Arc<Mutex<..>> so
the manager and the closure it drives operate on one shared set.
CAS loop, handing it a fresh MetaTxn each attempt; the closure takes the
transaction by value to remain Send under #[async_trait].
key and transaction, so a staged write always targets the guarded key.
must be free) followed by moving the dictionary id, replacing the previous
hand-rolled transaction loop.
patch tag) for the KV transaction APIs the helper builds on.
retry-on-conflict, and write staging.
The crate now re-exports MetaTxn, MetaTxnManager, and ForUpdate.
Tests
Type of change
Related Issues
This change is