morningman opened a new pull request, #70309:
URL: https://github.com/apache/doris/pull/70309
### Background
`query_p0/update/update_after_create_rollup` checks partial `UPDATE` on a
merge-on-write table that has a rollup. `ALTER TABLE ... ADD ROLLUP` starts an
FE job that sends one alter task per tablet to the BEs, and `SHOW ALTER TABLE
ROLLUP` reports the job's state. Separately, a load through the move-memtable
sink (`enable_memtable_on_sink_node`, on by default) can add a `DeltaWriterV2
<tablet>` and a `MemTableWriter <tablet>` child to its profile for each tablet;
since #64620 the BE attaches them only when the session's `profile_level` is at
least 2, the default.
### What problem does this PR solve?
Issue Number: None
Related PR: #65310 (the branch-4.1 fix of the rollup suite by @shuke987,
ported here), #59863 (added test_writer_profile on branch-4.0, by
@0AyanamiRei), #65232 (pins its first half to profile level 1 on branch-4.1, by
@shuke987), #64620 (the master gate that test_writer_profile covers), #64378
(made profile level 2 the default), #61842 (moved the rollup suite from
nereids_p0 to query_p0)
PR Type: Test
Problem Summary:
`update_after_create_rollup` sleeps 10 s after `ADD ROLLUP` and then
requires EXPLAIN of an unrelated GROUP BY to pick `rollup1`, so it fails
whenever the rollup job takes longer; as in #65310, it now waits until the job
reports FINISHED. `test_writer_profile`, which branch-4.x has and master lacks,
checks that a load's profile hides the writer details at profile level 1 and
shows them at level 2; master has had that gate since #64620, with no test.
Both files become identical to branch-4.2.
### Details
- **Failure:** `update_after_create_rollup` has not failed on its own in
master's public pipelines: since June 30, P0 ran it 7,257 times and cloud_p0
739 times, and all 55 failures were in builds where most suites failed. A
forced slow rollup job reproduces it (see Testing).
- **Cause:** The rollup job converts all 54 tablets on the BEs, so how long
it takes depends on the BE's alter task queue, while the suite allows a fixed
10 s. The EXPLAIN check also depends on the optimizer choosing the rollup,
which suites such as `rollup_p0/test_rollup_agg` cover.
- **Stability evidence:** `getAlterRollupFinalState` polls `SHOW ALTER TABLE
ROLLUP` every second for up to 300 s and returns the final state, so a
CANCELLED job still fails. With each tablet's alter task delayed by 5 s,
master's suite fails at the EXPLAIN check and the ported one passes after 91 s.
- **Scenarios:** In `test_writer_profile`, an `INSERT INTO ... SELECT` at
`profile_level=1`, whose profile must contain neither `DeltaWriterV2` nor
`MemTableWriter`, then the same INSERT at `profile_level=2`, whose profile must
contain both; cloud mode is skipped.
- **What existing tests miss:** The closest suite,
`query_profile/test_load_channel_profile`, turns `enable_memtable_on_sink_node`
off and checks only that level 3 shows writer details; no suite checks the
level-1 gate of the move-memtable sink.
- **Fails when broken:** Run at the default level 2, the first half fails
with `should not contain DeltaWriterV2`, the profile that a missing gate
produces at level 1. With the second INSERT at level 1, the second half fails
with `should contain DeltaWriterV2`.
- **External dependencies:** Both INSERTs read `load/data by line.json` from
the regression bucket through the `S3` TVF, with the bucket and credentials
from the regression config.
### Testing
All runs used `./run-regression-test.sh --run -d query_p0/update -s
update_after_create_rollup` or `--run -d query_profile -s test_writer_profile`
against a local single-FE, single-BE cluster built from master.
| Suite | Version | Result |
|---|---|---|
| `update_after_create_rollup` | master | passes |
| `update_after_create_rollup` | this PR | passes; the job was FINISHED
about 1 s after `ADD ROLLUP` |
| `update_after_create_rollup` | master, with BE debug point
`SchemaChangeJob::process_alter_tablet.leave.sleep` (5 s per tablet) | fails:
`Explain and check failed, expect contains 'rollup1'`, with the job RUNNING at
9/54 tasks |
| `update_after_create_rollup` | this PR, same debug point | passes after
waiting 91 s |
| `update_after_create_rollup` | this PR, with `CANCEL ALTER TABLE ROLLUP`
right after `ADD ROLLUP` | fails as it should: `expected: <FINISHED> but was:
<CANCELLED>` |
| `test_writer_profile` | this PR | passes; the level-2 profile has a
`DeltaWriterV2` and a `MemTableWriter` child, the level-1 profile neither |
| `test_writer_profile` | this PR without #65232's `set profile_level=1` |
fails: `should not contain DeltaWriterV2 ==> expected: <false> but was: <true>`
|
| `test_writer_profile` | this PR, second INSERT at level 1 | fails as it
should: `should contain DeltaWriterV2 ==> expected: <true> but was: <false>` |
My local regression config's OSS key is rejected (`InvalidAccessKeyId`), so
the `test_writer_profile` runs used a copy whose two INSERTs read
`numbers("number" = "2")` instead of the `S3` TVF; nothing else differs. The
PR's NonConcurrent pipeline runs that suite as committed, and P0 and cloud_p0
run the rollup suite.
### Notes for Reviewers
- `test_writer_profile` reads each profile 500 ms after its INSERT, as on
branch-4.2, without checking `Profile Completion State`. Every local run read a
COMPLETE profile, and on branch-4.x the suite has failed only in broken builds
since #65232 (6 of 2,211 runs).
- Two more suites from the same branch-4.1 batch need nothing on master:
master's renamed `test_jdbc_refresh_catalog_schema_refresh_non_blocking`
(#65126) already has #65335's fixes, and master #65696 replaced #65234's suite
with an FE unit test.
### Release note
None
### Check List (For Author)
- Behavior changed: No
- Does this need documentation: No
- Pre-submission status: Partially verified: `test_writer_profile` ran
locally only with its S3 source replaced by `numbers()`; the committed form
runs in the PR's NonConcurrent pipeline
### 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; this task covered four branch-4.1 test fixes whose files are missing on
master. After the check showed that master already covers two of them, the
author chose to port the rollup-suite fix as branch-4.2 has it, to bring
test_writer_profile with its branch-4.1 fix to master as a test of #64620, and
to verify both on a local cluster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
