Skip to content

[fix](regression) Disable auto compaction for physical row inspections#65536

Open
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:codex/fix-physical-inspect-auto-compaction
Open

[fix](regression) Disable auto compaction for physical row inspections#65536
shuke987 wants to merge 1 commit into
apache:masterfrom
shuke987:codex/fix-physical-inspect-auto-compaction

Conversation

@shuke987

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

Issue Number: N/A

Related PR: #65211, #64508

Problem Summary:

Some unique-key MOW regression cases deliberately bypass the delete bitmap and assert historical physical rows, sequence values, skip bitmaps, and rowset versions. These assertions require the pre-compaction rowsets to remain available, but the affected tables leave automatic compaction enabled. Background compaction can therefore merge the expected old rowsets before qt_inspect or an equivalent raw-row query runs, while all user-visible SQL results remain correct.

This PR disables table-level automatic compaction only for the tables whose assertions intentionally depend on historical physical layout. It covers nine case files, including publish-conflict, read-from-old, delete-sign, sequence-column, and auto-increment physical-row checks. Golden results and user-visible correctness assertions are unchanged.

The source scan reviewed all master cases that set skip_delete_bitmap=true without already disabling automatic compaction. Four remaining files were intentionally excluded because the variable is used only for debug-mode validation, routine-load waiting, insert execution setup, or binlog TVF behavior; they do not assert historical table rowsets.

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

    Validation: test_flexible_partial_update_publish_conflict_seq passed as one lock-serialized suite on the authorized branch-4.1 test cluster using the patched master case file. The run confirmed the new table property in SHOW CREATE TABLE and completed all logical and physical-row assertions with 0 failed suites. git diff --check origin/master...HEAD also passed.

  • 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

### What problem does this PR solve?

Issue Number: close #xxx

Related PR: apache#65211, apache#64508

Problem Summary: Regression cases that bypass the delete bitmap and assert historical physical rows can race with background compaction. Compaction may merge the expected old rowsets before the assertion while the user-visible result remains correct. Disable table-level auto compaction only for cases and table definitions that intentionally inspect those physical versions.

### Release note

None

### Check List (For Author)

- Test: Regression test
    - `test_flexible_partial_update_publish_conflict_seq`
- Behavior changed: No
- Does this need documentation: No
@shuke987

Copy link
Copy Markdown
Collaborator Author

/review

@shuke987

Copy link
Copy Markdown
Collaborator Author

run buildall

@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?

@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 the PR at f7f58a9a4c1ac49f1696ad7f6c25d5df0f94dfee against base 2c5f1574737970b6123c1f0ac24d14a36f366381. I do not see a blocking issue.

Checkpoint conclusions:

  • Goal and proof: The PR's goal is to stabilize regression tests that intentionally inspect pre-compaction physical rows for unique-key MoW partial-update/publish behavior. The added table property is applied to tables that later use skip_delete_bitmap, skip_delete_sign, qt_inspect/inspect_rows, hidden version columns, or equivalent physical-row assertions.
  • Scope: The patch is narrowly limited to regression table properties plus newline normalization. No production code or golden result rows are changed.
  • Concurrency and lifecycle: No runtime concurrency code is changed. Existing publish/debug-point concurrency in the tests remains the same; the new property only prevents background compaction from removing rowsets that the tests intentionally inspect.
  • Configuration and compatibility: disable_auto_compaction is an existing table property. The FE create-table path stores it in table metadata, shared-nothing tablets receive it through tablet schema creation, and cloud tablet meta/scheduler paths also honor it. The property remains applicable through stream-load publish and table truncate/reuse flows.
  • Parallel paths and coverage: I checked related skip_delete_bitmap/qt_inspect/physical-row cases. The remaining no-property suites using skip_delete_bitmap are binlog TVF, insert execution setup, routine-load waiting, or MV debug-mode validation, not comparable historical-row inspection cases.
  • Test result files: The corresponding .out files contain no DDL, SHOW CREATE TABLE, PROPERTIES, or disable_auto_compaction output, so the unchanged golden files are consistent with this patch.
  • Data correctness, persistence, performance, and observability: No data-path, persistence-format, metric, or logging behavior is changed. The compaction disablement is scoped to regression test tables.

I did not run builds or regression tests in this review-only runner. The review was completed with one convergence round: the main review plus two full-review subagents and one risk-focused subagent all returned no new valuable findings, and there are no unresolved candidates or inline comments.

User focus: no additional user-provided review focus was supplied.

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