fix(regression): Make test_analyze_mv row_count assertion stable after truncate#64419
Merged
morrySnow merged 1 commit intoJun 15, 2026
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
The row_count_for_nereids (column 4 of show index stats) can be either -1 (not yet reported by BE) or 0 (reported as 0 rows for empty table) right after truncate table. Asserting exactly -1 creates a race condition with BE tablet reporting, especially on cloud where CloudTabletStatMgr unconditionally sets rowCountReported=true. Change the assertion to accept both -1 and 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
yujun777
force-pushed
the
fix-flaky-test-analyze-mv-rowcount
branch
from
June 11, 2026 08:58
1839fa6 to
ed91cd0
Compare
Contributor
Author
|
run buildall |
Contributor
Author
|
run nonConcurrent |
Contributor
|
/review |
morrySnow
approved these changes
Jun 15, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
There was a problem hiding this comment.
Code review completed. I found no blocking issues in this PR.
Critical checkpoint conclusions:
- Goal and proof: The change addresses a real flaky assertion after
truncate table;show index statsstrict row-count can be-1before tablet row-count reporting completes or0after the empty-table report arrives. The suite still later waits for and asserts the stable0state for both the MV and base index. - Scope: The modification is small and focused, limited to the flaky regression assertion.
- Concurrency/lifecycle: The relevant behavior is asynchronous tablet-stat reporting after truncate. No new production concurrency or lifecycle code is introduced.
- Config/compatibility/protocol: No configuration, storage format, protocol, or FE/BE variable-passing changes.
- Parallel paths: The later stable row-count checks remain unchanged, so this only relaxes the immediate race window and does not remove end-state coverage.
- Test coverage/results: This is a regression-test-only fix; no
.outresult update is needed for this assertion-only change. - Observability/performance/transaction/persistence: No new production behavior is introduced, so these are not materially affected.
User focus points: no additional user-provided review focus was present.
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 15, 2026
…r truncate (#64419) ## Problem `statistics/test_analyze_mv.groovy` line 614 asserts `assertEquals("-1", result_row[0][4])` immediately after `truncate table`, expecting `report_row_count_for_nereids` to be -1 (unreported). This is a race condition: BE asynchronously reports new tablet stats (0 rows for empty table) to FE, and if the report arrives before the assertion, the value is 0 instead of -1. On cloud, this is amplified by `CloudTabletStatMgr` unconditionally setting `rowCountReported=true`, making the -1 state exceptionally short-lived or unobservable. ## Root Cause After `truncate table`, the FE stat manager (`CloudTabletStatMgr` / `TabletStatMgr`) updates `MaterializedIndex.rowCount` and sets `rowCountReported=true`. The test assertion races with this update: - If BE hasn't reported yet → `getRowCountForIndex(id, true)` returns -1 ✓ - If BE has reported → returns 0 ✗ (test fails) ## Fix Change the assertion to accept both -1 and 0, since both are valid states for an empty table after truncate: - -1: tablet row count not yet reported - 0: tablet row count reported as 0 (empty table) Add assertion message with actual value for debuggability. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 15, 2026
…r truncate (#64419) ## Problem `statistics/test_analyze_mv.groovy` line 614 asserts `assertEquals("-1", result_row[0][4])` immediately after `truncate table`, expecting `report_row_count_for_nereids` to be -1 (unreported). This is a race condition: BE asynchronously reports new tablet stats (0 rows for empty table) to FE, and if the report arrives before the assertion, the value is 0 instead of -1. On cloud, this is amplified by `CloudTabletStatMgr` unconditionally setting `rowCountReported=true`, making the -1 state exceptionally short-lived or unobservable. ## Root Cause After `truncate table`, the FE stat manager (`CloudTabletStatMgr` / `TabletStatMgr`) updates `MaterializedIndex.rowCount` and sets `rowCountReported=true`. The test assertion races with this update: - If BE hasn't reported yet → `getRowCountForIndex(id, true)` returns -1 ✓ - If BE has reported → returns 0 ✗ (test fails) ## Fix Change the assertion to accept both -1 and 0, since both are valid states for an empty table after truncate: - -1: tablet row count not yet reported - 0: tablet row count reported as 0 (empty table) Add assertion message with actual value for debuggability. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 15, 2026
zclllyybb
added a commit
that referenced
this pull request
Jun 15, 2026
…mpt (#64536) Problem Summary: Automated code review runs relied on the reviewer to infer which module-specific AGENTS.md files apply to a pull request. Recent Litefuse traces showed that reviews often skipped ancestor guides such as the repository root AGENTS.md, fe/AGENTS.md, and be/test/AGENTS.md even when the changed files made them applicable. This change fetches the PR changed file list, derives every existing AGENTS.md file from the changed file ancestor directories, records the list in the review context, and injects the required guide paths directly into the first Codex review prompt. We tested 5 PRs that did not read the AGENTS.md file during previous pipeline runs, and the new results are as follows: | Origin PR | Test PR | Run | Litefuse Trace | Result | |---|---:|---:|---|---| | #64478 | zclllyybb#36 | `27536249602` | `d76b77ac4e0d52d96f413b154c9f2571` | Read All | | #64458 | zclllyybb#37 | `27536258049` | `8db34388a8379acdcd9e48720f11225f` | Read All | | #64392 | zclllyybb#38 | `27536266784` | `a5e37fba03375a9517f7a67e33f70d39` | Read All | | #64489 | zclllyybb#39 | `27536275680` | `22ace18d70ece96b0ca7fa73123b62da` | Read All | | #64419 | zclllyybb#41 | `27538239601` | `1803c040e4140d61e840e4e1526190e7` | Read All |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
statistics/test_analyze_mv.groovyline 614 assertsassertEquals("-1", result_row[0][4])immediately aftertruncate table, expectingreport_row_count_for_nereidsto be -1 (unreported). This is a race condition: BE asynchronously reports new tablet stats (0 rows for empty table) to FE, and if the report arrives before the assertion, the value is 0 instead of -1.On cloud, this is amplified by
CloudTabletStatMgrunconditionally settingrowCountReported=true, making the -1 state exceptionally short-lived or unobservable.Root Cause
After
truncate table, the FE stat manager (CloudTabletStatMgr/TabletStatMgr) updatesMaterializedIndex.rowCountand setsrowCountReported=true. The test assertion races with this update:getRowCountForIndex(id, true)returns -1 ✓Fix
Change the assertion to accept both -1 and 0, since both are valid states for an empty table after truncate:
Add assertion message with actual value for debuggability.
🤖 Generated with Claude Code