Skip to content

[fix](cloud) Avoid conflicts when recycling transactions with the same label#65746

Open
wyxxxcat wants to merge 1 commit into
apache:masterfrom
wyxxxcat:recycle_label_conflict
Open

[fix](cloud) Avoid conflicts when recycling transactions with the same label#65746
wyxxxcat wants to merge 1 commit into
apache:masterfrom
wyxxxcat:recycle_label_conflict

Conversation

@wyxxxcat

@wyxxxcat wyxxxcat commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Concurrent recycling of transactions with the same label updates the same label
key and may cause transaction conflicts.

Group transactions by label and recycle each group sequentially while processing
different label groups concurrently.

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@wyxxxcat
wyxxxcat force-pushed the recycle_label_conflict branch from 42bb721 to f3364b6 Compare July 17, 2026 03:50
@wyxxxcat

Copy link
Copy Markdown
Collaborator Author

run buildall

@wyxxxcat

Copy link
Copy Markdown
Collaborator Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes: the production grouping is sound, but the changed tests remove material negative-path coverage for the retained conflict retry and the new per-label worker failure contract.

Critical checkpoint conclusions:

  • Goal and proof: grouping by the encoded (instance, db, label) key correctly serializes same-label cleanup while preserving cross-label concurrency, and the positive same-label test demonstrates zero internal conflicts. The inline issue identifies the missing proof for external-conflict retry, exhaustion, cancellation, and recovery.
  • Scope and clarity: the semantic change is focused; the other recycler hunks are formatting-only.
  • Concurrency and lifecycle: each label group owns a moved key vector; same-label work is sequential; page transitions join all workers; SyncExecutor::when_all waits for every callback or cancellation signal before captured helpers are destroyed. No deadlock, dangling capture, or silent cancellation was found.
  • Error handling and atomicity: each cleanup is one KV transaction. Real conflicts remain possible from live label writers or lease handoff, but retries use fresh transactions; exhaustion returns nonzero and leaves recycle metadata discoverable for a later scan. No silent partial-success acknowledgement was found.
  • Persistence and compatibility: no protocol or storage-format change is introduced. Normal/versioned commit, lazy commit, abort, timeout abort, and operation-log replay all preserve the label identity used for grouping and for the actual txn-label key.
  • Parallel paths, configuration, and observability: all producer/replay paths were traced; no new config is added; existing logs and metrics remain adequate for this change.
  • Performance: memory remains bounded by one range page, and serialization is limited to transactions sharing a label; concurrency across labels is retained.
  • Tests and validation: positive grouping coverage is present, but the negative paths called out inline are not. Per the automated-review contract, no local build or test was run; this was a static review. Formatter/checkstyle checks were passing, while Cloud UT and compile checks were still pending at the last check.
  • User focus: no additional user-provided focus was specified.

Two full-review passes and a separate risk-focused pass converged with no additional valuable findings after M-1 was accepted and deduplicated.

Comment thread cloud/test/recycler_test.cpp
@wyxxxcat
wyxxxcat force-pushed the recycle_label_conflict branch from f3364b6 to aecd0db Compare July 17, 2026 06:06
@wyxxxcat

Copy link
Copy Markdown
Collaborator Author

run buildall

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants