Uh oh!
There was an error while loading. Please reload this page.
[fix](test) Make test_analyze_long_string Case 5 stable against sample rows randomness - #64408
Conversation
hello-stephen
commented
Jun 11, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
yujun777
commented
Jun 11, 2026
run buildall |
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
yujun777
commented
Jun 12, 2026
run nonConcurrent |
morrySnow
commented
Jun 12, 2026
/review |
zclllyybb
commented
Jun 12, 2026
skip buildall |
There was a problem hiding this comment.
Reviewed PR #64408 against the repository code-review skill and existing review context. I found no blocking issues.
Critical checkpoint conclusions:
- Goal/test proof: The PR targets flakiness in Case 5 of
test_analyze_long_stringby ensuring every possiblesample rows 3DUJ1 sample contains abig_strvalue longer thanstatistics_max_string_column_length. This matches the intended assertion path, and the PR's nonConcurrent regression check is passing. - Scope/focus: The change is limited to test fixture data in one regression suite and is appropriately small.
- Concurrency/lifecycle/locks: No new concurrent code, lifecycle management, or locking is introduced.
- Config/compatibility/protocol/storage: No config item, protocol, storage format, or compatibility behavior is changed.
- Parallel paths: Case 5 specifically covers forced DUJ1 sampling; the existing suite still covers full, sample percent 100, partition-path exception, disabled limit, and sync analyze behavior.
- Conditional checks/error handling: No new conditional logic or error handling is added.
- Test coverage/result files: The changed test remains assertion-based like the existing suite and does not require an
.outfile. I did not run the suite locally; I verified the PR check output shows NonConcurrent Regression passing. - Observability/performance/security: Not applicable for this test-data-only change.
User focus: no additional user-provided review focus was present.
yujun777
commented
Jun 12, 2026
need modify |
dbdcddb to
6057383Compareyujun777
commented
Jun 12, 2026
run buildall |
6057383 to
8003eeeCompare8003eee to
0008f79Compare…yze cases
The root cause is a race condition: after inserting data, the BE may
not have reported the row count to FE yet. When OlapAnalysisTask is
created with info.rowCount == 0 and tableSample != null, doExecute()
returns early without running any SQL, so the column finishes with
FINISHED state but an empty message — the expected skip reason from
the long-string assert_true guard is never generated.
Fix:
1. Suite.groovy: Add waitRowCountReady(db, table, expectedRowCount)
that polls SHOW DATA FROM table until the BE-reported row count
reaches the expected value.
2. test_analyze_long_string.groovy: Call waitRowCountReady after
inserts for both sample analyze cases:
- Case 3 (sample percent 100): test_analyze_long_string_sample
- Case 5 (sample rows 3 with DUJ1): test_analyze_long_string_duj1
3. Case 5 data: Keep the pre-existing change that uses repeat('z', 2048)
for all rows — this guards against sample randomness missing the
long row even when row count is properly reported.
Co-Authored-By: Claude <noreply@anthropic.com>0008f79 to
b10903bCompareyujun777
commented
Jun 12, 2026
run buildall |
zclllyybb
commented
Jun 13, 2026
/review |
There was a problem hiding this comment.
No blocking issues found in this PR.
Checkpoint conclusions:
- Goal and proof: The change targets the sampled-analyze race where
AnalysisInfo.rowCountis captured before BE row-count reporting catches up. The new waits are placed before the Case 3 and Case 5ANALYZEstatements, which is beforeAnalysisManager.buildAnalysisJobInfo()snapshots row count, so they address the intended failure mode. - Scope: The patch is small and focused: one regression-framework helper plus two uses in
test_analyze_long_string. - Concurrency/lifecycle: The only relevant concurrency is asynchronous BE row-count reporting versus FE analyze-job creation. The test now waits for the BE-reported count before creating the analyze job. No production locks, static lifecycle, or new persistent state are introduced.
- Configuration/compatibility: No new config, storage format, RPC/thrift, or rolling-upgrade compatibility surface.
- Parallel paths: Both sampled paths covered by this test are handled: sample percent 100 and forced DUJ1 sample rows. Full analyze and sync analyze paths do not depend on this sampled-empty shortcut.
- Conditional logic: The wait condition is concrete (
SHOW DATArow count reaches the expected inserted rows) and scoped to regression stabilization. - Test coverage/results: The strengthened regression test covers the flaky path directly; no
.outchange is needed because this suite uses assertions. I did not rerun tests locally, but PR checks show NonConcurrent Regression, P0 Regression, External Regression, compile, formatter, and license checks passing. - Observability/performance: The helper logs
SHOW DATAwhile polling; the 120s/3s polling cost is limited to regression tests. - Transaction/persistence/data correctness: No production transaction or persistence behavior changes; the test waits after committed inserts and before analyze job creation.
- FE/BE variable passing: Not applicable.
- User focus: No additional user-provided focus points were present, and I found no focus-specific issues.
Uh oh!
There was an error while loading. Please reload this page.
…e rows randomness (#64408) ## Problem `test_analyze_long_string` Case 5 (and potentially Case 3) can flake because after inserting data, the BE may not have reported the row count to FE yet. When `OlapAnalysisTask.doExecute()` runs with `info.rowCount == 0` and `tableSample != null`, it returns early without executing any SQL — the column finishes with `FINISHED` state but an empty message, so the expected skip reason from the `assert_true` long-string guard is never produced: ``` expected skip reason visible for col big_str, got msg= ==> expected: <true> but was: <false> ``` The audit log confirms that no sampling SQL was issued for `big_str` in the failing run — the task was short-circuited entirely. ## Fix 1. **Suite.groovy**: Add `waitRowCountReady(db, table, expectedRowCount)` that polls `SHOW DATA FROM db.table` via `sql_return_maparray` until the BE-reported row count reaches the expected value. 2. **test_analyze_long_string.groovy**: Call `waitRowCountReady` after inserts for both sample analyze cases: - Case 3 (sample percent 100) - Case 5 (sample rows 3, DUJ1 template) 3. Case 5 data uses `repeat('z', 2048)` for all rows — a secondary defense against sample randomness missing the long row even when row count is properly reported. Co-authored-by: Claude <noreply@anthropic.com>
…e rows randomness (#64408) ## Problem `test_analyze_long_string` Case 5 (and potentially Case 3) can flake because after inserting data, the BE may not have reported the row count to FE yet. When `OlapAnalysisTask.doExecute()` runs with `info.rowCount == 0` and `tableSample != null`, it returns early without executing any SQL — the column finishes with `FINISHED` state but an empty message, so the expected skip reason from the `assert_true` long-string guard is never produced: ``` expected skip reason visible for col big_str, got msg= ==> expected: <true> but was: <false> ``` The audit log confirms that no sampling SQL was issued for `big_str` in the failing run — the task was short-circuited entirely. ## Fix 1. **Suite.groovy**: Add `waitRowCountReady(db, table, expectedRowCount)` that polls `SHOW DATA FROM db.table` via `sql_return_maparray` until the BE-reported row count reaches the expected value. 2. **test_analyze_long_string.groovy**: Call `waitRowCountReady` after inserts for both sample analyze cases: - Case 3 (sample percent 100) - Case 5 (sample rows 3, DUJ1 template) 3. Case 5 data uses `repeat('z', 2048)` for all rows — a secondary defense against sample randomness missing the long row even when row count is properly reported. Co-authored-by: Claude <noreply@anthropic.com>
Problem
test_analyze_long_stringCase 5 (and potentially Case 3) can flake becauseafter inserting data, the BE may not have reported the row count to FE yet.
When
OlapAnalysisTask.doExecute()runs withinfo.rowCount == 0andtableSample != null, it returns early without executing any SQL — thecolumn finishes with
FINISHEDstate but an empty message, so the expectedskip reason from the
assert_truelong-string guard is never produced:The audit log confirms that no sampling SQL was issued for
big_strinthe failing run — the task was short-circuited entirely.
Fix
Suite.groovy: Add
waitRowCountReady(db, table, expectedRowCount)that polls
SHOW DATA FROM db.tableviasql_return_maparrayuntilthe BE-reported row count reaches the expected value.
test_analyze_long_string.groovy: Call
waitRowCountReadyafterinserts for both sample analyze cases:
Case 5 data uses
repeat('z', 2048)for all rows — a secondary defenseagainst sample randomness missing the long row even when row count is
properly reported.