Skip to content

[fix](regression) Fix flaky materialized view load-open test - #67138

Open
0AyanamiRei wants to merge 1 commit into
apache:masterfrom
0AyanamiRei:fix/mv-load-open-flaky
Open

[fix](regression) Fix flaky materialized view load-open test#67138
0AyanamiRei wants to merge 1 commit into
apache:masterfrom
0AyanamiRei:fix/mv-load-open-flaky

Conversation

@0AyanamiRei

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: N/A

Related PR: N/A

Problem Summary:

test_materialized_view_load_open used createMV, which waits for the latest materialized view job in the shared database. Under parallel suite execution, it can observe a concurrent job as FINISHED and execute ADD COLUMN while its own table is still in ROLLUP state.

Replace all four calls with create_sync_mv so each materialized view creation waits for the job belonging to its base table.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test
    • No need to test or manual test. Explain why:
      • Other reason: regression-test synchronization-only change; not run per request.
  • 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: N/A

Related PR: N/A

Problem Summary: The regression case used createMV, which waits for the latest materialized view job in the shared database. Under parallel suite execution, it could observe a concurrent job as FINISHED and run ADD COLUMN while its own table was still in ROLLUP state. Use create_sync_mv for all four materialized views so each wait is scoped to its base table.

### Release note

None

### Check List (For Author)

- Test: No need to test (test-only synchronization change; not run per request)
- Behavior changed: No
- Does this need documentation: No
@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?

@sollhui

sollhui commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

run buildall

@sollhui

sollhui commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

/review

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

LGTM

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

Review opinion: approve. I found no blocking issue and no inline finding to raise.

Critical checkpoint conclusions:

  • Goal and evidence: The change fixes the concrete parallel-suite race. The deprecated createMV polls the database-wide latest MV job, while each replacement waits for the latest MV job belonging to its own base table. The four changed tables are uniquely owned by this suite, so unrelated jobs in the shared rollup_p0 database cannot satisfy the new waits.
  • Scope and clarity: The patch is focused: it replaces all four unsafe helper calls and repairs the final newline. The MV names, base tables, projections, aliases, grouping, DDL/DML, stream load, and expected results are otherwise unchanged.
  • Concurrency: This checkpoint is applicable because normal suites can run concurrently in the same directory-derived database. The sibling suite reuses the MV names but owns distinct *_mm tables; exact TableName filtering excludes those jobs. The changed suite itself executes these calls sequentially, and no same-table competing MV producer exists.
  • Lifecycle and errors: create_sync_mv waits for the rollup job, asserts FINISHED, retains the post-job table-state delay, and issues SYNC before later inserts or schema changes. SQL failure, cancellation, and timeout remain fail-closed. No new resource ownership, static initialization, lock ordering, or release lifecycle is introduced.
  • Configuration: No configuration item or dynamic-setting behavior changes. Existing dynamic-partition and light-schema-change properties remain test inputs only.
  • Compatibility: No production symbol, RPC/thrift field, serialization, storage format, FE/BE contract, cloud/shared-nothing behavior, or rolling-upgrade path changes.
  • Parallel paths and conditions: All four unsafe calls are migrated, covering dynamic partition on/off and light schema change true/false. No new conditional branch is introduced, and the sibling path already waits by its distinct base tables.
  • Test coverage and results: The existing end-to-end suite exercises all four variants, later inserts, both schema changes, stream load, and ordered result checks. SQL semantics and the .out data are unchanged, so no result-file update is expected. I did not run builds or tests because this review runner explicitly prohibits them; the PR also records that this synchronization-only change was not run.
  • Observability: The established helper logs the table-filtered SHOW query and status transitions and asserts the terminal state, which is sufficient for this regression synchronization path. No production metric or log change is warranted.
  • Transactions, persistence, and data writes: No production transaction, EditLog/replay, failover, or write implementation changes. The existing test DML now begins only after the intended rollup job and metadata synchronization complete.
  • FE/BE variables: No transmitted variable or scattered send path is added or changed.
  • Performance: The scoped no-op DROP, bounded polling, and SYNC add modest test-only setup work in exchange for deterministic ordering; no production hot path, memory accounting, CPU, or allocation behavior changes.
  • Other core invariants: No production data visibility, visible-version, delete-bitmap, nullable-column, memory-safety, or security boundary is modified. The stronger wait prevents ADD COLUMN from racing a table still in ROLLUP state.

User focus: No additional user-provided focus was specified; the complete PR review found no additional issue.

Review completion: One convergence round completed. Two independent full-review agents and one separate risk-focused agent all returned NO_NEW_VALUABLE_FINDINGS after missed-issue rechecks. The main review independently cleared job correlation and helper side-effect risks, and the final changed-file/unresolved-candidate sweep found nothing outstanding.

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.

3 participants