Skip to content

branch-4.1: [fix](test) stabilize BDB unmatched transaction test#65757

Open
shuke987 wants to merge 1 commit into
apache:branch-4.1from
shuke987:pick-65587-branch-4.1
Open

branch-4.1: [fix](test) stabilize BDB unmatched transaction test#65757
shuke987 wants to merge 1 commit into
apache:branch-4.1from
shuke987:pick-65587-branch-4.1

Conversation

@shuke987

@shuke987 shuke987 commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: None

Related PR: #65587

Problem Summary:

Backport the stabilization of BDBEnvironmentTest.testReadTxnIsNotMatched to branch-4.1. The release-branch test partially mocked a live JE RepImpl with JMockit, which could miss or intercept registerVLSN inconsistently while BDB background threads were active. It also read follower state before asynchronous replication was guaranteed visible.

This tailored pick waits for the baseline value on followers, uses a method-scoped Mockito spy that throws only from postLogCommitHook, restores both original JE environment fields in finally, and verifies the exact locally committed value after restart.

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.

Validation:

  • Target FE UT passed twice locally.
  • git diff --check passed.

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?

@shuke987

Copy link
Copy Markdown
Collaborator Author

run buildall

@shuke987

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.

Reviewed PR 65757. I did not find a correctness issue to raise.

The change is test-only and focused on stabilizing BDBEnvironmentTest.testReadTxnIsNotMatched: follower visibility now waits for the exact baseline key/value, the temporary JE RepImpl spy is restored before shutdown/reopen, and the reopened former master must contain the exact locally committed key/value.

Critical checkpoint conclusions:

  • Goal/test proof: satisfied for the intended local-commit-after-ack-failure invariant.
  • Scope: one focused FE test file; no production behavior changes.
  • Concurrency/lifecycle: bounded follower polling handles asynchronous replay, cached database handles are reused, and both JE implementation fields are restored in finally before lifecycle operations.
  • Branch compatibility: branch-4.1 already declares Awaitility and Mockito 4.11 core/inline, and its pinned Doris JE fields and hook signature match the reflection/stub.
  • Persistence/result correctness: JE logs the commit before the injected post-commit exception; after reopen the test checks count + 1, the exact key, and exact value bytes.
  • Config/protocol/security/observability/data-write behavior: no production surface is changed.
  • Parallel/special paths: allowing normal VLSN registration and possible follower delivery is intentional because acknowledgement failure does not imply non-delivery.
  • Test coverage: strengthened against follower replay races and false-positive value checks.
  • User focus: no additional user-provided review focus.

Validation was static only, as required by the review-runner contract. The exact PR range passes git diff --check. I did not run builds or FE tests because thirdparty/installed and thirdparty/installed/bin/protoc are absent; the PR reports the focused target test passed twice locally.

@shuke987
shuke987 marked this pull request as ready for review July 17, 2026 08:07
@shuke987
shuke987 requested a review from yiguolei as a code owner July 17, 2026 08:07
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