Skip to content

[fix](case) Relax show data compaction size check#65392

Open
shuke987 wants to merge 1 commit into
apache:branch-4.0from
shuke987:fix-branch40-show-data-compaction-size
Open

[fix](case) Relax show data compaction size check#65392
shuke987 wants to merge 1 commit into
apache:branch-4.0from
shuke987:fix-branch40-show-data-compaction-size

Conversation

@shuke987

@shuke987 shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Relax test_show_data_with_compaction so it no longer compares physical SHOW DATA sizes exactly after full compaction.
  • Check the large documents table against observed mode-specific ranges: 170-200 KB for non-cloud and 55-70 KB for cloud/S3-style runs.
  • Replace the older broad 10% documents-table comparison with explicit range checks, and keep the small insert table as a relative closeness check.

Testing

  • git diff --check -- regression-test/suites/inverted_index_p2/test_show_data.groovy
  • Same logical fix passed the branch-4.1 single-suite remote run before the final range-only assertion adjustment: ./run-regression-test.sh --run -d inverted_index_p2 -s test_show_data_with_compaction -parallel 1

@shuke987 shuke987 requested a review from morningman as a code owner July 8, 2026 12:52
@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

shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

/review

@shuke987

shuke987 commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

run buildall

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

Automated review summary:

I found one blocking test-coverage regression in the changed assertion logic and submitted it as an inline comment.

Critical checkpoints:

  • Changed files reviewed: regression-test/suites/inverted_index_p2/test_show_data.groovy.
  • Existing inline review context: none was present in .code-review.csS0H3/pr_review_threads.md, so there was no duplicate thread to suppress.
  • Required AGENTS.md files: none were listed for this PR.
  • Repo-local code-review SKILL.md: not found in this checkout after hidden-path searches, so the review proceeded with the prompt contract and shared-ledger workflow.
  • Validation limits: the checkout is grafted/shallow and lacks the base parent locally; I used GitHub PR patch metadata as the authoritative diff. I did not run the Doris regression suite locally.

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

Subagent conclusions:

  • optimizer-rewrite found no optimizer/rewrite, semantic-equivalence, parallel join, or aggregate-path issue.
  • tests-session-config proposed TSC-1; I independently verified and accepted it as MAIN-1, now submitted as the inline comment.
  • Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same ledger/comment set after MAIN-1 was accepted.

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

Automated review summary:

I found one blocking test-coverage regression in the changed assertion logic and submitted it as an inline comment.

Critical checkpoints:

  • Changed files reviewed: regression-test/suites/inverted_index_p2/test_show_data.groovy.
  • Existing inline review context: none was present in .code-review.csS0H3/pr_review_threads.md, so there was no duplicate thread to suppress.
  • Required AGENTS.md files: none were listed for this PR.
  • Repo-local code-review SKILL.md: not found in this checkout after hidden-path searches, so the review proceeded with the prompt contract and shared-ledger workflow.
  • Validation limits: the checkout is grafted/shallow and lacks the base parent locally; I used GitHub PR patch metadata as the authoritative diff. I did not run the Doris regression suite locally.

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

Subagent conclusions:

  • optimizer-rewrite found no optimizer/rewrite, semantic-equivalence, parallel join, or aggregate-path issue.
  • tests-session-config proposed TSC-1; I independently verified and accepted it as MAIN-1, now submitted as the inline comment.
  • Convergence round 2 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the corrected RIGHT-side line 836 anchor after the misplaced position-based comment was deleted.

assertTrue(Math.abs(with_index_size - another_with_index_size)
<= 0.1 * Math.max(with_index_size, another_with_index_size),
"index size mismatch beyond 10% tolerance: with_index=${with_index_size}, without_index=${another_with_index_size}")
assert_documents_show_data_size_in_range.call(with_index_size, "documents table with inverted index compaction")

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.

This drops the actual comparison between the compaction-enabled and compaction-disabled paths. The suite deliberately builds one table with inverted_index_compaction_enable=true and another with it false, but after this change both values are only checked against broad independent ranges. For example, in cloud mode 55 KB and 70 KB would both pass even though the two paths differ by more than 20%, so a regression in compaction size accounting could slip through. Please keep the absolute sanity range if needed, but also retain a relative comparison between with_index_size and another_with_index_size.

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