Skip to content

fix(regression): Make test_analyze_mv row_count assertion stable after truncate#64419

Merged
morrySnow merged 1 commit into
apache:masterfrom
yujun777:fix-flaky-test-analyze-mv-rowcount
Jun 15, 2026
Merged

fix(regression): Make test_analyze_mv row_count assertion stable after truncate#64419
morrySnow merged 1 commit into
apache:masterfrom
yujun777:fix-flaky-test-analyze-mv-rowcount

Conversation

@yujun777

@yujun777 yujun777 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

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.

🤖 Generated with Claude Code

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

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
yujun777 force-pushed the fix-flaky-test-analyze-mv-rowcount branch from 1839fa6 to ed91cd0 Compare June 11, 2026 08:58
@yujun777

Copy link
Copy Markdown
Contributor Author

run buildall

@yujun777

Copy link
Copy Markdown
Contributor Author

run nonConcurrent

@morrySnow

Copy link
Copy Markdown
Contributor

/review

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Jun 15, 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.

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

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 stats strict row-count can be -1 before tablet row-count reporting completes or 0 after the empty-table report arrives. The suite still later waits for and asserts the stable 0 state 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 .out result 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.

@morrySnow
morrySnow merged commit fad12a0 into apache:master Jun 15, 2026
32 checks passed
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>
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 |
yiguolei pushed a commit that referenced this pull request Jun 16, 2026
… stable after truncate #64419 (#64503)

Cherry-picked from #64419

Co-authored-by: yujun <yujun@selectdb.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
morningman pushed a commit that referenced this pull request Jun 30, 2026
… stable after truncate #64419 (#64501)

Cherry-picked from #64419

Co-authored-by: yujun <yujun@selectdb.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by one committer. dev/4.0.7-merged dev/4.1.3-merged reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants