Skip to content

[fix](cloud) Hide KV_TXN_MAYBE_COMMITTED from clients - #62244

Merged
luwei16 merged 3 commits into
apache:masterfrom
luwei16:fix/meta-service-downgrade-kv-txn-maybe-committed
Apr 20, 2026
Merged

[fix](cloud) Hide KV_TXN_MAYBE_COMMITTED from clients#62244
luwei16 merged 3 commits into
apache:masterfrom
luwei16:fix/meta-service-downgrade-kv-txn-maybe-committed

Conversation

@luwei16

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: None

Problem Summary: Preserve MetaService retry handling for KV_TXN_MAYBE_COMMITTED internally, but downgrade the final response code to KV_TXN_COMMIT_ERR so older BE/FE clients do not treat the proto2 unknown enum as OK.

Release note

None

Check List (For Author)

  • Test: Cloud unit test

    • ./run-cloud-ut.sh --run --filter='meta_service_test:MetaServiceTxnStoreRetryableTest.*' -j 8
  • Behavior changed: Yes (MetaService no longer exposes KV_TXN_MAYBE_COMMITTED to clients; it still retries internally and returns KV_TXN_COMMIT_ERR externally)

  • Does this need documentation: No

### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Preserve MetaService retry handling for KV_TXN_MAYBE_COMMITTED internally, but downgrade the final response code to KV_TXN_COMMIT_ERR so older BE/FE clients do not treat the proto2 unknown enum as OK.
### Release note
None
### Check List (For Author)
- Test: Cloud unit test
- ./run-cloud-ut.sh --run --filter='meta_service_test:MetaServiceTxnStoreRetryableTest.*' -j 8
- Behavior changed: Yes (MetaService no longer exposes KV_TXN_MAYBE_COMMITTED to clients; it still retries internally and returns KV_TXN_COMMIT_ERR externally)
- Does this need documentation: No
@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?

@luwei16

Copy link
Copy Markdown
ContributorAuthor

run buildall

@luwei16

Copy link
Copy Markdown
ContributorAuthor

run buildall

w41ter
w41ter previously approved these changes Apr 8, 2026
@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Apr 8, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 83.33% (5/6) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage78.52% (1799/2291)
Line Coverage64.30% (32343/50301)
Region Coverage65.13% (16286/25005)
Branch Coverage55.63% (8704/15646)

gavinchou
gavinchou previously approved these changes Apr 8, 2026
@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 59.38% (19/32) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.63% (27364/37165)
Line Coverage57.28% (295358/515662)
Region Coverage54.56% (246206/451241)
Branch Coverage56.17% (106626/189833)

### What problem does this PR solve?
Issue Number: None
Related PR: apache#62244
Problem Summary: Add cloud unit tests that cover the MetaServiceProxy downgrade paths for KV_TXN_MAYBE_COMMITTED so the PR's retry-handling change is exercised on both read and commit RPCs.
### Release note
None
### Check List (For Author)
- Test: Cloud unit test
- ./run-cloud-ut.sh --run --filter='meta_service_test:MetaServiceTxnStoreRetryableTest.*' -j8
- Behavior changed: No
- Does this need documentation: No
@luwei16
luwei16 dismissed stale reviews from gavinchou and w41ter via 58c0555April 10, 2026 07:31
@github-actionsgithub-actionsBot removed the approved Indicates a PR has been approved by one committer. label Apr 10, 2026
@luwei16

Copy link
Copy Markdown
ContributorAuthor

run buildall

1 similar comment
@luwei16

Copy link
Copy Markdown
ContributorAuthor

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage `` 🎉
Increment coverage report
Complete coverage report

@luwei16

Copy link
Copy Markdown
ContributorAuthor

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage53.03% (20135/37969)
Line Coverage36.58% (189353/517680)
Region Coverage32.82% (146957/447716)
Branch Coverage33.98% (64357/189421)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.64% (27380/37183)
Line Coverage57.31% (295789/516096)
Region Coverage54.48% (246156/451848)
Branch Coverage56.20% (106777/190003)

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/63) 🎉
Increment coverage report
Complete coverage report

@luwei16

Copy link
Copy Markdown
ContributorAuthor

run nonConcurrent

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.63% (27378/37183)
Line Coverage57.30% (295743/516096)
Region Coverage54.46% (246057/451848)
Branch Coverage56.19% (106756/190003)

@hello-stephen

Copy link
Copy Markdown
Contributor

FE Regression Coverage Report

Increment line coverage 0.00% (0/63) 🎉
Increment coverage report
Complete coverage report

@HastyshellHastyshell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Apr 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@luwei16
luwei16 merged commit 48bad76 into apache:masterApr 20, 2026
30 of 31 checks passed
github-actionsBot pushed a commit that referenced this pull request Apr 20, 2026
Problem Summary: Preserve MetaService retry handling for
KV_TXN_MAYBE_COMMITTED internally, but downgrade the final response code
to KV_TXN_COMMIT_ERR so older BE/FE clients do not treat the proto2
unknown enum as OK.
### Release note
None
### Check List (For Author)
- Test: Cloud unit test
- ./run-cloud-ut.sh --run
--filter='meta_service_test:MetaServiceTxnStoreRetryableTest.*' -j 8
- Behavior changed: Yes (MetaService no longer exposes
KV_TXN_MAYBE_COMMITTED to clients; it still retries internally and
returns KV_TXN_COMMIT_ERR externally)
github-actionsBot pushed a commit that referenced this pull request Apr 20, 2026
Problem Summary: Preserve MetaService retry handling for
KV_TXN_MAYBE_COMMITTED internally, but downgrade the final response code
to KV_TXN_COMMIT_ERR so older BE/FE clients do not treat the proto2
unknown enum as OK.
### Release note
None
### Check List (For Author)
- Test: Cloud unit test
- ./run-cloud-ut.sh --run
--filter='meta_service_test:MetaServiceTxnStoreRetryableTest.*' -j 8
- Behavior changed: Yes (MetaService no longer exposes
KV_TXN_MAYBE_COMMITTED to clients; it still retries internally and
returns KV_TXN_COMMIT_ERR externally)
yiguolei pushed a commit that referenced this pull request Apr 22, 2026
 (#62644)
Cherry-picked from #62244
Co-authored-by: Luwei <814383175@qq.com>
yiguolei pushed a commit that referenced this pull request Apr 22, 2026
 (#62642)
Cherry-picked from #62244
Co-authored-by: Luwei <814383175@qq.com>
@yiguoleiyiguolei mentioned this pull request May 20, 2026
@morningmanmorningman mentioned this pull request Jun 3, 2026
zhaorongsheng pushed a commit to zhaorongsheng/doris that referenced this pull request Jun 4, 2026
Problem Summary: Preserve MetaService retry handling for
KV_TXN_MAYBE_COMMITTED internally, but downgrade the final response code
to KV_TXN_COMMIT_ERR so older BE/FE clients do not treat the proto2
unknown enum as OK.
### Release note
None
### Check List (For Author)
- Test: Cloud unit test
- ./run-cloud-ut.sh --run
--filter='meta_service_test:MetaServiceTxnStoreRetryableTest.*' -j 8
- Behavior changed: Yes (MetaService no longer exposes
KV_TXN_MAYBE_COMMITTED to clients; it still retries internally and
returns KV_TXN_COMMIT_ERR externally)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.dev/4.0.6-mergeddev/4.1.1-mergedreviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@luwei16@hello-stephen@gavinchou@w41ter@Hastyshell@yiguolei