Skip to content

Fix flaky test: TestRefreshableMVQuerySettings::test_query_settings_apply_to_refreshes - #781

Merged
koletzilla merged 2 commits into
mainfrom
test/refreshable-mv-query-settings-retry
Nov 6, 2026
Merged

koletzilla merged 2 commits into
mainfrom
test/refreshable-mv-query-settings-retry

Conversation

@koletzilla

@koletzilla koletzilla commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

On the Native leg of test_cloud it failed once with id 4000 was not written to hackers (run 41827567747). It's not a regression: #777 touches no MV code, and the HTTP leg and the nightlies passed.

Why it fails

The test:

  1. inserts a row
  2. runs SYSTEM REFRESH VIEW + SYSTEM WAIT VIEW
  3. then checks the row landed in the target with the view's settings applied.

Every project.run_sql call opens a new connection, so on Cloud each statement can hit a different replica. That breaks the test in two ways:

  1. The wait does not wait. Before ClickHouse 26.10, SYSTEM WAIT VIEW on one replica does not see a refresh requested on another and returns at once (Fix SYSTEM WAIT VIEW not waiting for a refresh requested on another replica ClickHouse#120623). The read then runs before the refresh is done.
  2. The refresh does not see the insert. SharedMergeTree is eventually consistent across replicas, so a refresh running on a different replica than the insert can miss the new row (same lag as Reduce test flakyness: wait for data replication #715). The target then stays without it until the next scheduled refresh.

What changes

The test checks the same things as before, it's now more sync and waits when sync is not possible:

  • Add select_sequential_consistency: 1 to the model's query_settings. It ends up in the view's SETTINGS clause, which is what the refresh runs with, so the refresh syncs with Keeper and sees the row. Fixes 2. Harmless on local MergeTree. The adapter's connection default for Shared databases does not help here, since it only applies to statements dbt sends, not to the server-side refresh.
  • Poll for the row after the refresh with the existing retry_until_assertion_passes helper, instead of trusting SYSTEM WAIT VIEW. Fixes 1. The refresh itself is still issued once. this is what we do in other tests to wait for the results being fully processed

Note

Low Risk
Changes are limited to integration test synchronization; no adapter or runtime behavior is modified.

Overview
Fixes intermittent failures in TestRefreshableMVQuerySettings::test_query_settings_apply_to_refreshes on Cloud Native, where each run_sql can hit a different replica.

The refreshable MV model now includes select_sequential_consistency: 1 in query_settings so server-side refreshes see recently inserted source rows across replicas. After SYSTEM REFRESH VIEW / SYSTEM WAIT VIEW, assertions go through a new matched_department_after_refresh helper that polls with retry_until_assertion_passes instead of assuming wait completed on the read replica (pre–26.10 SYSTEM WAIT VIEW behavior). The test still verifies join_use_nulls in DDL and NULL matched_department after refresh.

Reviewed by Cursor Bugbot for commit 0c6649a. Bugbot is set up for automated code reviews on this repo. Configure here.

Copilot AI 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 recommended

No active review issues are identified for the test-only changes aimed at reducing refresh-test flakiness.

No open issues 🎉

What changed in this PR

Reduces flakiness in the refreshable materialized-view query-settings integration test on multi-replica ClickHouse Cloud services. Changes are limited to tests; adapter behavior is unchanged.

Changes:

  • Adds select_sequential_consistency to the model’s query settings so refreshes can see recently inserted rows across replicas.
  • Polls for the expected row after requesting a refresh, while preserving assertions that join_use_nulls applies.
File Description
tests/​integration/​adapter/​materialized_view/​test_materialized_view_query_settings.py Adds consistency settings and retry-based row checks to stabilize refresh assertions.

Review effort: Balanced

@koletzilla
koletzilla merged commit 554f355 into main Nov 6, 2026
16 checks passed
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