Repository navigation
[fix](test) run search score TopN case in nonConcurrent group - #67402
Conversation
Related PR: apache#67363 (same fix on branch-4.1); suite introduced by apache#65821 and backported via apache#67327. test_search_score_topn_predicates runs in the shared concurrent P0 group and intermittently fails on its first query with "[INTERNAL_ERROR]SearchExpr should not be executed without inverted index". master carries the identical suite grouping as branch-4.1 (where it was observed flaky, 2/4 runs), so move it to the nonConcurrent group here too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
run nonConcurrent |
|
run nonConcurrent |
|
Status update on verification: the NonConcurrent pipeline has now failed three times in a row on this PR (builds 1036154, 1036404, 1036651) and in all three the run collapsed before reaching All three show the same environment signature rather than a test problem: I am not re-triggering further. What is verified so far: the suite passes locally with the tag applied, and the tag itself is the same grouping 41 other inverted_index_p0 cases already use. The final CI confirmation that it executes green inside the nonConcurrent group needs that pipeline to complete a run. |
…lush task teardown (#67442) ### What problem does this PR solve? Issue Number: close #67428 Related PR: #66889 (introduced the UAF) Problem Summary: ASAN heap-use-after-free in `SharedMemtable::~SharedMemtable()` during group-commit memtable flush task teardown, introduced by #66889. `PartOfGroupMemtableFlushTask` holds its `SharedMemtable` by `shared_ptr` but only a `weak_ptr` to the `FlushToken`. When `run()`'s local `shared_ptr<FlushToken>` drops the last reference at the end of `run()`, destruction cascades `FlushToken` -> `RowsetWriter` -> `RowsetWriterContext`; the thread pool then destroys the task object, and `~SharedMemtable()` dereferences the dangling raw `RowsetWriterContext* rowset_ctx` in `remove_segment_allocated_lsns()`. Reproduced by the nonConcurrent pipeline on two unrelated PRs (#67404, #67402). ### What changed? - `SharedMemtable` now owns `std::shared_ptr<SegmentAllocatedLsnMap>` captured from the group writer context at submission; insert/remove go through the owned map. A null map stands for "no LSN allocation" (equivalent to `need_allocated_lsn()`, since `GroupRowsetWriter::init()` creates the map exactly when needed). This keeps the precise cleanup dependency alive without extending the whole `RowsetWriter` lifetime — the approach recommended in the #67428 triage. - Add `SegmentAllocatedLsnMap::contains_segment()` for test assertions. - Regression tests covering: (a) the last token/writer owner released while a group flush task finishes, (b) a queued subtask running after its weak token expired, (c) cancellation, plus LSN-entry cleanup assertions on the flush-error path. ### Verification ASAN BE UT, both directions: **without** the fix the new tests abort with the exact reported UAF (`SUMMARY: AddressSanitizer: heap-use-after-free rowset_writer_context.h:202:9 in doris::RowsetWriterContext::remove_segment_allocated_lsns`); **with** the fix the `MemTableFlushExecutor*` tests pass 8/8 (two runs).
|
/review |
There was a problem hiding this comment.
No actionable findings in the complete diff at head 5d46ec6. The one-line change moves this existing suite to the framework's serial phase while retaining p0 membership.
Core invariants: production data visibility, version selection, delete bitmaps and memory ownership are unaffected; existing result comparisons and expected-error checks remain active.
Critical checkpoint conclusions:
- Goal and proof: comma-separated group parsing selects SINGLE; its one-thread executor serializes these bodies after NORMAL suite completion. This establishes scheduling behavior statically; elimination of the reported intermittent SearchExpr failure was not verified at runtime.
- Scope and reuse: minimal metadata edit using the established nonConcurrent mechanism; suite name, SQL and data path are retained.
- Concurrency and locks: NORMAL script futures wait for suite closure through a Phaser before SINGLE work starts. No shared variable or lock changes. Existing DOCKER/SINGLE overlap is framework behavior; no global isolation claim is made.
- Lifecycle and initialization: existing suite connection/output cleanup and Phaser deregistration remain intact; no new static initialization or ownership.
- Configuration: no production configuration item or dynamic setting added.
- Compatibility: no symbol, protocol or storage-format change.
- Parallel paths: checked score/cache and score/delete sibling tests; no corresponding edit was shown necessary.
- Conditions and errors: no new condition; the LIMIT+OFFSET overflow case retains its test/exception assertion.
- Coverage: twelve existing query checks retain equality, MATCH, range, LIMIT/OFFSET, multiple/nested/negated SEARCH and score-filter coverage, plus the overflow negative case.
- Results: all twelve qt tags match the unchanged golden blocks; each query explicitly orders the outer result by id. Output lookup uses the unchanged script path rather than group. No new or modified expected results.
- Observability: existing suite/group and failure logging is adequate for this registration change.
- Transactions and persistence: no transaction, EditLog, persistence or failover logic changed.
- Writes and failure handling: existing DROP-before-CREATE, INSERT and SYNC setup is preserved; no write implementation changed.
- FE/BE variables: no new field or session variable; existing SET statements are unchanged.
- Performance: intentionally adds this small suite to serial work; no new loops, repeated scans or production hot-path cost.
- Other issues and selection: checked repository serial-job configuration and triggers retain the suite. Concurrent-only invocations using runNonConcurrent=false intentionally skip it. No verified duplicate-execution, output-lookup or selection defect was found.
Existing review context contains no inline findings; existing_blocking_comment_ids is empty. No additional user focus was supplied. Both the full-scope reviewer and separate scheduling-risk reviewer completed missed-issue rechecks; no unresolved in-scope candidate remains. Review converged after 2 rounds.
Static-only review: no builds or tests were run. Runtime stability, live CI results and external TeamCity job arguments were not verified. These limits do not establish whether serial execution cures the reported flake.
Review converged after 2 round(s).
…t_generation (#70308) ### Background An auto-partition table with `partition.retention_count = N` keeps only its N latest historical partitions: the FE's `DynamicPartitionScheduler` drops older ones on every run, once per `dynamic_partition_check_interval_seconds` (600 by default). `partition_p0/auto_partition/test_auto_new_recycle` lowers that interval to 1 s and checks the partition count after each load or ALTER. `audit/test_sql_digest_generation` checks that the audit log gets an `sql_digest` only for queries slower than the FE config `sql_digest_generation_threshold_ms`, once with the threshold at 0 and once at 100000. Both suites are in group `nonConcurrent`. ### What problem does this PR solve? Issue Number: None Related PR: #65375 and #65646 (the branch-4.1 recycle-suite fixes by @shuke987, ported here), #65343 (the branch-4.1 digest-suite fix by @shuke987, ported here), #65435 (master's earlier move of the recycle suite's day-boundary guard) PR Type: Test Problem Summary: Both suites rely on timing the product does not promise. The recycle suite checks the partition count after a fixed `sleep(8000)` (5 s after an ALTER), while the drop happens asynchronously at an unbounded time after the config change; branch-4.1's NonConcurrent pipeline failed at the first such check repeatedly (#65375). The digest suite's threshold-0 check needs the query's elapsed time to be above 0 ms. This PR gives both suites their branch-4.1/4.2 form: the recycle suite polls the partition count every second, for at most 1200 s, and widens its day-boundary guard to cover the poll; the digest query adds `sleep(1)`. Both files become identical to branch-4.2. ### Details - **Failure:** On master's public NonConcurrent pipeline neither suite has failed on its own since mid-July: of about 8,000 runs each, the recycle suite failed 3 times and the digest suite 87 times, all in builds whose cluster had died (most suites failing). The failures that motivated the fixes are on branch-4.1's pipeline (#65375); #65343 made the digest query deterministic there. - **Cause:** `ADMIN SET FRONTEND CONFIG` sets the scheduler's new interval at once, but the scheduler sleeps in 5-second slices and notices the change only at the next slice. It then runs `executeDynamicPartition` over every registered table before the drop. So the drop lands 0 to 5 s plus one full pass after the config change, and a pass longer than about 3 s fails the 8-second check. In the digest suite, `AuditLogHelper` writes a digest only when `elapseMs > sql_digest_generation_threshold_ms`, and nothing bounds the elapsed time of `select *` on a two-row table from below (9 to 15 ms locally). With `sleep(1)` on each row it takes about 2 s, still far below the second check's 100000 ms. - **Stability evidence:** The recycle checks now wait for the drop and fail only when the count misses the target for 1200 s. The poll runs on the suite thread, because the framework sets `Awaitility.pollInSameThread()` (#68338), so it does not leak connections. Shortening one of master's waits to 1 s reproduces the branch-4.1 failure shape, and the ported suite passes; mutants show that both suites still catch wrong results (see Testing). ### Testing All runs used `./run-regression-test.sh --run -d partition_p0/auto_partition -s test_auto_new_recycle` or `--run -d audit -s test_sql_digest_generation` against a local single-FE, single-BE cluster built from master. | Suite | Version | Result | |---|---|---| | `test_auto_new_recycle` | master | passes | | `test_auto_new_recycle` | this PR | passes; the polls reached the target after 1, 2, 1, 3 and 3 s | | `test_auto_new_recycle` | master, with the wait before the 10-partition check cut to 1 s | fails as #65375 describes: `expected: <101> but was: <10>` (master's `assertEquals` has its arguments swapped, so 101 partitions were still there) | | `test_auto_new_recycle` | this PR, expecting 11 partitions, poll capped at 20 s | fails as it should: `ConditionTimeoutException` | | `test_sql_digest_generation` | master | passes | | `test_sql_digest_generation` | this PR | passes; the query took 2,024 and 2,025 ms | | `test_sql_digest_generation` | this PR, second threshold 1000 ms | fails as it should: `Count of sql_digest changed` | The PR NonConcurrent pipeline runs both suites; cloud_p0 skips `nonConcurrent` suites. ### Notes for Reviewers - The day-boundary guard before the final `now()`-based section goes from 100 s to 1300 s, as in #65646, so that it covers the 1200 s poll plus the insert and checks. It sleeps only when that section would otherwise start within 1300 s of midnight. - Four more suites fixed in the same branch-4.1 batch need nothing on master: #65340 (master #68449 enables the publish debug point on all FEs, superseding the open port #65556), #65929 (master #68329 fixed the actual cause, a connection leak), #65885 (master #69723 removed the V1 scanner whose message it also accepted), and #67363 (ported as #67402). ### Release note None ### Check List (For Author) - Behavior changed: No - Does this need documentation: No - Pre-submission status: Ready ### AI Disclosure - Tool: Claude Code (Claude Opus 5.5) - Generated: the port (both suites taken from branch-4.2), the local verification runs, and this description - Human review: not stated - Prompts: The author asked to continue moving branch-4.2's changes back to master, where six branch-4.1 test fixes were recorded as one task. After the check showed that master already covers four of them and has not failed on the other two, the author chose to port those two exactly as branch-4.2 has them and to verify them on a local cluster. Co-authored-by: shuke <shuke@selectdb.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
What problem does this PR solve?
Issue Number: N/A
Related PR: #67363 (same fix on branch-4.1), #67327 (backported the suite to branch-4.1), #65821 (introduced the suite on master)
Problem Summary:
test_search_score_topn_predicates(added on master by #65821) runs in the shared concurrent P0 group. On branch-4.1 it fails intermittently on its very first query, right afterCREATE TABLE+INSERT+sync, with:This was observed on branch-4.1 CI (2 failures out of 4 runs) and fixed there by #67363, which moves the case into the
nonConcurrentgroup. master carries the identical suite grouping ("p0"), so it is exposed to the same intermittent failure when the case races other P0 cases. This PR applies the same one-line fix to master to keep it out of the concurrent group.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)