morningman opened a new pull request, #66995:
URL: https://github.com/apache/doris/pull/66995
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #66469 (the master PR this picks), #66884 / #66920 (the
follow-up that is
**already on branch-4.1** and was written against this fix)
Problem Summary:
Cherry-pick of #66469 to branch-4.1. All six root causes it fixes also exist
on
branch-4.1 (2 FE product bugs + 3 test/CI fixes; item 5 is partly already
fixed here —
see the conflict notes).
1. **[load] Broker load pending-task retry self-conflicts on its own label**
(FE product
bug). `BrokerLoadPendingTask.executeTask()` runs `getAllFileStatus()`
then `beginTxn()`,
and `LoadTask.exec` retries the whole task on failure (retryTime=3), with
`onTaskFinished()` inside the same try. Once the txn exists, any failure
makes every
retry hit `LabelAlreadyUsedException` against the job's OWN PREPARE txn,
burn all
retries within milliseconds and cancel the job with a misleading
`Label [...] has already been used, relate to txn [...], status
[PREPARE]` that masks
the real cause. Fix: make `beginTxn()` retry-idempotent — reuse an
already-assigned
`transactionId`, and on `LabelAlreadyUsedException` adopt the label's txn
iff it is ours
(`callbackId == job id`, status PREPARE). Foreign conflicts and lookup
failures still
rethrow the original exception. 3 Mockito unit tests.
2. **[load] Nereids planning failures must cancel the job with the real
cause** (FE product
bug, the trigger's escape route).
`org.apache.doris.nereids.exceptions.AnalysisException`
extends RuntimeException, so it bypassed `onPendingTaskFinished`'s
`catch (UserException)` (which cancels immediately with the real message)
and fell into
the generic retry path. Catch it alongside UserException. The new test
drives the real
propagation path (mocked `NereidsLoadingTaskPlanner.plan` throws through
the real
`LoadLoadingTask.init`/`createLoadingTask` chain) and asserts the job is
CANCELLED with
the real message and no loading tasks.
3. **plugin_compaction.groovy**: treat base compaction's `E-808`
(`BE_NO_SUITABLE_VERSION`)
as benign, exactly like cumulative's E-2000/E-2010. Suites still verify
the compaction
effect via their own rowset/segment-count asserts.
4. **compaction_width_array_column**: with `BUCKETS 2` the fixture loads a
~56GB
197-segment overlapping rowset into a single tablet; compaction writes
the full output
on the same mount before deleting the input, which structurally ENOSPCs
on 100GB CI data
disks. Bump to `BUCKETS 16` (~7GB/tablet) and raise the load to 16G
exec_mem_limit /
load_parallelism 1.
5. **inverted_index_p2/test_show_data**: branch-4.1 has already fixed three
of the four
sub-suites independently (#65387, #65927). This pick adds only what is
missing here:
the three `wait_timeout` guards, and the `test_show_data_for_bkd`
comparison — the one
sub-suite branch-4.1 never fixed, which still did an exact `assertEquals`
on index sizes
produced by two different writer paths.
6. **cold_heat_separation_p2/table_modify_resouce_and_policy(+_by_hdfs)**:
both phases
waited only for `tablets[0]` to finish cooldown, then asserted
`remote_data_size > 0`
for EVERY replica row. Replicated cooldown is
leader-uploads/followers-follow-async, so
a lagging follower failed the assert with zero grace. Make the wait cover
all replica
rows (local==0 && remote>0) with the same 100x10s budget.
### Conflicts resolved
**`BrokerLoadJob.java`** applied without conflict and is byte-identical to
master's version
of the two changed hunks. Worth calling out: branch-4.1 already carries
#66920 (pick of
#66884), which resets `transactionId = 0` in
`CloudBrokerLoadJob.unprotectedExecuteRetry()`
with the comment *"so beginTxn() does not blindly reuse a transaction that
was aborted …
beginTxn() will … adopt the transaction only after verifying that it belongs
to this job."*
That behaviour is exactly what this PR introduces, so branch-4.1 currently
has the
follow-up without its base. `CloudBrokerLoadJob` does not override
`beginTxn()`, so it
inherits the fix.
**`BrokerLoadJobTest.java` (import block)** — branch-4.1 has not taken
master's
JMockit → Mockito migration (#62221), so the file is still JMockit-based
while the picked
tests are Mockito-based. Kept branch-4.1's JMockit imports and added the
imports the picked
tests need. `mockito-core` and `mockito-inline` are already test-scoped in
`fe-core` on
branch-4.1, and 18 other `fe-core` test classes already mix both frameworks,
so no
dependency or migration work is needed.
**`test_show_data.groovy`** — branch-4.1 already fixed this exact flake in
#65387 and
#65927, with a *stricter* 15% ratio gate plus an absolute size-range gate.
branch-4.1's
version was kept in all four conflicting comparisons: taking master's text
would have
loosened 15% → 20% and deleted the range assertions (leaving #65387's helper
closures
dead). Only the additive parts of the pick were taken.
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [x] Yes. (a) Broker load jobs whose pending task is retried after the
job's txn was
already begun now reuse (or adopt) that txn instead of being cancelled
with a
misleading "Label has already been used" error. (b) Nereids planning
failures now
cancel the load job immediately with the real cause instead of going
through the
generic retry path. No behavior change outside those failure paths.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
--
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]
