Skip to content

[BugFix] Keep base_version in sync with base_metadata in publish_version#76313

Merged
luohaha merged 1 commit into
StarRocks:mainfrom
banmoy:fix_publish_oss
Jul 14, 2026
Merged

[BugFix] Keep base_version in sync with base_metadata in publish_version#76313
luohaha merged 1 commit into
StarRocks:mainfrom
banmoy:fix_publish_oss

Conversation

@banmoy

@banmoy banmoy commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Why I'm doing:

publish_version() has an implicit invariant: the local scalar base_version must always equal base_metadata->version(). The single→batch conversion recovery branch (handling a txn that already published successfully in single mode, whose txnlog was deleted, but whose FE response was lost) reassigns base_metadata to meta@(base_version+1) and continues the loop — without advancing base_version to match. From that point on, base_version != base_metadata->version() for the rest of the call.

This currently produces no observable data-plane bug in OSS (the only other reader of the scalar is an unreachable DCHECK in the schema-change path), but it's a real invariant violation waiting to bite any future code that reads base_version to mean "the version base_metadata is at."

What I'm doing:

  • Sync base_version = base_metadata->version() right after the reassignment in the skip branch.
  • Add DCHECK_EQ(base_version, base_metadata->version()) where log_applier is initialized, as a regression guard for this invariant.
  • Add NoOpPublishTest.single_to_batch_conversion_keeps_base_version_in_sync, which drives the skip branch with a 3-txn batch. Verified the added DCHECK actually fires (Check failed: base_version == base_metadata->version() (1 vs. 2)) when the fix line is reverted, and passes cleanly with it restored.

What type of PR is this:

  • BugFix
  • Feature
  • Enhancement
  • Refactor
  • UT
  • Doc
  • Tool

Does this PR entail a change in behavior?

  • Yes, this PR will result in a change in behavior.
  • No, this PR will not result in a change in behavior.

If yes, please specify the type of change:

  • Interface/UI changes: syntax, type conversion, expression evaluation, display information
  • Parameter changes: default values, similar parameters but with different default values
  • Policy changes: use new policy to replace old one, functionality automatically enabled
  • Feature removed
  • Miscellaneous: upgrade & downgrade compatibility, etc.

(Additive only: two new leader-side monitoring metrics appear in the FE /metrics output. No existing metric, SQL surface, parameter, or persisted state changes.)

Checklist:

  • I have added test cases for my bug fix or my new feature
  • This pr needs user documentation (for new or modified features or behaviors)
    • I have added documentation for my new feature or new function
    • This pr needs auto generate documentation
  • This is a backport pr

Bugfix cherry-pick branch check:

  • I have checked the version labels which the pr will be auto-backported to the target branch
    • 4.1
    • 4.0
    • 3.5

@mergify mergify Bot assigned banmoy Jul 14, 2026
@github-actions github-actions Bot requested review from luohaha and meegoo July 14, 2026 01:50
@github-actions github-actions Bot added the 4.1 label Jul 14, 2026
@CelerData-Reviewer

CelerData-Reviewer commented Jul 14, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 7c9d97efe3

ℹ️ 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".

@huailiu1122

huailiu1122 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

@mergify rebase

@mergify

mergify Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

rebase

✅ Branch has been successfully rebased

@mergify

mergify Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Deprecation notice: This pull request comes from a fork and was rebased using bot_account impersonation. This capability will be removed on July 1, 2026. After this date, the rebase action will no longer be able to rebase fork pull requests with this configuration. Please switch to the update action/command to ensure compatibility going forward.

@CelerData-Reviewer

CelerData-Reviewer commented Jul 14, 2026

Copy link
Copy Markdown

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 429a5dc66b

ℹ️ 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".

@huailiu1122

huailiu1122 commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

@mergify rebase

@mergify

mergify Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

@banmoy @claude @huailiu1122
[BugFix] Keep base_version in sync with base_metadata in publish_version
2f5ebe1 
When a single-mode publish already succeeded and its txnlog was deleted
but FE never received the response, the txn gets re-sent as the first
element of a batch publish. The recovery branch that handles this
reassigns base_metadata to the newer version but left the base_version
scalar stale, breaking the base_version == base_metadata->version()
invariant relied on elsewhere in the function. Sync base_version
alongside base_metadata and add a DCHECK to guard the invariant.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KFu5UZqgfiVXuyUSnttF8J
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

[Java-Extensions Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

[FE Incremental Coverage Report]

✅ pass : 0 / 0 (0%)

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

[BE Incremental Coverage Report]

✅ pass : 2 / 2 (100.00%)

file detail

path covered_line new_line coverage not_covered_line_detail
🔵 be/src/storage/lake/transactions.cpp 2 2 100.00% []

@luohaha luohaha merged commit d1d2da9 into StarRocks:main Jul 14, 2026
65 of 66 checks passed
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

@Mergifyio backport branch-4.1

@github-actions github-actions Bot removed the 4.1 label Jul 14, 2026
@mergify

mergify Bot commented Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

backport branch-4.1

✅ Backports have been created

Details

@mergify mergify Bot mentioned this pull request Jul 14, 2026
Merged
23 tasks
wanpengfei-git pushed a commit that referenced this pull request Jul 14, 2026
@mergify @banmoy @claude
[BugFix] Keep base_version in sync with base_metadata in publish_vers…
ceed1db 
…ion (backport #76313) (#76328)

Co-authored-by: PengFei Li <lpengfei2016@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@luohaha luohaha luohaha approved these changes
@sevev sevev sevev approved these changes
@meegoo meegoo Awaiting requested review from meegoo

Assignees

@banmoy banmoy

Labels

4.1-merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@banmoy @CelerData-Reviewer @huailiu1122 @luohaha @sevev