Skip to content

[fix](regression) Wait for sync MV row count - #68015

Open
yujun777 wants to merge 1 commit into
apache:masterfrom
yujun777:fix/doris-24834-wait-mv-row-count
Open

yujun777 wants to merge 1 commit into
apache:masterfrom
yujun777:fix/doris-24834-wait-mv-row-count

Conversation

@yujun777

@yujun777 yujun777 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

The regression test could run EXPLAIN before the synchronous materialized view index row count was reported to FE, causing unstable MV rewrite selection. This change polls SHOW INDEX STATS <table> <mv> and waits for report_row_count_for_nereids to reach the expected value before checking the rewrite plan.

Test Plan

  • ./run-regression-test.sh --run -d rollup_p0 -s test_create_mv_and_mtmv
  • Result: 1 suite passed, 0 failed

Release note

None

### What problem does this PR solve?

Issue Number: close #DORIS-24834

Related PR: None

Problem Summary: The regression test could run EXPLAIN before the synchronous materialized view index row count was reported to FE, causing unstable MV rewrite selection. Poll the MV index statistics until the Nereids row count reaches the expected value before checking the rewrite plan.

### Release note

None

### Check List (For Author)

- Test: Regression test: rollup_p0/test_create_mv_and_mtmv

- Behavior changed: No

- Does this need documentation: No

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@yujun777

Copy link
Copy Markdown
Contributor Author

buildall

@yujun777

Copy link
Copy Markdown
Contributor Author

/review

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

Approval opinion: no blocking findings.

The added wait addresses the reported race by polling the same strict synchronous-MV row count that Nereids consumes before asserting the rewrite plan. Round 1 converged with both full-review passes and the separate readiness/lifecycle risk pass returning NO_NEW_VALUABLE_FINDINGS; the main review independently resolved every risk item. No additional user review focus was supplied.

Critical checkpoint conclusions:

  • Goal and proof: the fifth SHOW INDEX STATS field is report_row_count_for_nereids, backed by OlapTable.getRowCountForIndex(indexId, true), exactly as used by StatsCalculator. The subsequent EXPLAIN remains the behavioral oracle. The author reports ./run-regression-test.sh --run -d rollup_p0 -s test_create_mv_and_mtmv passing; this automated review was static-only and did not independently run it.
  • Scope and clarity: the change is confined to the affected regression suite and adds only the readiness gate and diagnostics needed before the existing EXPLAIN.
  • Concurrency: shared-nothing TabletStatMgr and the cloud stats path update the row count asynchronously. Strict mode stays unknown until all visible index tablets report, including the empty third partition, and exact [2] rejects transient unknown or zero observations. SHOW and the following EXPLAIN execute on the same FE.
  • Lifecycle and failure handling: createMV first waits for the rollup job to finish; the new loop then waits for stats publication. It is bounded, SQL/conversion/interruption failures propagate, and timeout preserves logged observations plus the final actual value.
  • Configuration and parallel paths: no configuration is added. The normal 60-second stats cadence and cloud P0 10-second cadence both fit comfortably inside the bound; shared-nothing and cloud paths provide the same strict planner-facing signal.
  • Compatibility and protocol: no production API, persisted format, FE/BE field, rolling-upgrade behavior, or function symbol changes.
  • Conditions and tests: the immutable two-row fixture yields exactly two MV groups, so [2] is deterministic. The readiness assertion is internal synchronization rather than a result-set oracle; the existing ordered MTMV result checks and EXPLAIN check remain unchanged.
  • Observability: every probe logs the complete SHOW result and derived row counts; no production metrics or logs are needed for this test-only change.
  • Transactions, writes, and persistence: only the existing test fixture performs writes; production transactionality, edit-log behavior, visibility rules, and crash recovery are untouched.
  • Performance and other risks: probes are one-row local metadata reads and stop as soon as readiness is observed. No hot production path, memory-accounting issue, deadlock, or distinct correctness problem was found.

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