Repository navigation
[fix](regression) Scope Hive split and Paimon writer thread checks - #69230
Conversation
### What problem does this PR solve?
`test_hive_compress_type_large_data` compares exact file and split counts against streaming EXPLAIN estimates. A cold catalog can report `(approximate)inputSplitNum=1024` even though the fixture contains 16 files. Use statement-local `SET_VAR(enable_external_table_batch_mode=false)` for both exact plan checks. Keep the synchronous and explicitly forced streaming result checks.
`test_paimon_write_thread_lifecycle` treats growth in all BE JVM threads as a writer leak. Existing native blocking workers attach to the JVM on their first JNI call and remain attached until the worker exits. A stable global baseline is therefore not guaranteed, even when every writer closes its executor correctly. Check the writer-owned `doris-paimon-` threads through the existing process-thread endpoint instead, accounting for Linux's 15-byte thread-name limit. Require them to disappear within 30 seconds after each of ten write phases, and retain total process-thread counts in diagnostics. This detects even one surviving writer thread without increasing a tolerance or moving a baseline.
### Release note
None
### Check List (For Author)
- Test
- [x] Regression test
- [x] Manual test (details below)
- Behavior changed:
- [x] No production behavior changes; regression assertions now measure the intended resources.
- Does this need documentation?
- [x] No.
Local validation used the original DEBUG/ASAN build `6c27d4d8b0b4649f6517e15792fba672a54e2935`, an isolated FE/BE, Hive 3/HDFS, Spark 4/Paimon, and MinIO:
- **Before:** ran both unmodified suites. Hive failed the exact split assertion with the approximate 1024 estimate. With `parallel_pipeline_task_num=4`, Paimon failed bounded warm-up with JVM counts `451, 497, 525, 587, 606, 676`. Thread dumps mapped the growing attached threads to `p_normal_blocki`; writer compaction threads had exited. The failure reproduces the invalid global-count assumption, through the warm-up assertion rather than the later baseline assertion.
- **After:** restarted the BE, kept parallelism at 4, and ran both fixed suites through `run-regression-test.sh`; **2 suites passed, 0 failed, 0 skipped**. Paimon executed 120 inserts, verified 120,000 rows, and passed all ten writer-thread checks. Its attached blocking-worker count still grew from 54 to 368, so passing does not depend on a pre-warmed JVM. BE logs confirm the compaction executor actually ran.
- Hive used the original fixture: 16 files, 734,675,596 bytes, eight partitions. Both plan checks and all synchronous/forced-streaming result checks passed.
- Verified Paimon's actual `ExecutorThreadFactory` name truncates to `doris-paimon-co` on JDK 17/Linux. `git diff --check` passed.
|
run buildall |
|
/review |
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
No new actionable findings in the two changed regression suites at 1a836c72d7a00880189252e8ff10bbdec989fc24.
The Hive EXPLAIN hints select complete split enumeration for this eight-partition fixture and restore the prior session setting afterward. Both exact-plan checks are treated consistently; the existing synchronous and explicitly forced streaming result checks remain. The Paimon assertion now observes the writer-owned compaction executors: the process endpoint reads current /proc thread names, the prefix survives Linux name truncation, and repeated committed writes with compaction trigger 2 exercise the supplied executor in pinned Paimon 1.4.2. Successful sink close shuts down that executor before publishing commit messages. A surviving matching thread causes the bounded poll to fail; normal native workers attaching to the JVM no longer cause false positives.
Applicable critical checkpoints were checked individually:
| Checkpoint | Conclusion |
|---|---|
| Task goal and evidence | Both changes address the stated flaky assumptions. Traced EXPLAIN/hint/session/split enumeration and INSERT/sink/JNI/executor close flows; runtime execution was prohibited. |
| Focus and reuse | Changes are confined to the two tests and reuse the existing statement hint, process-thread endpoint and retry helper. |
| Concurrency and locking | Paimon remains nonConcurrent; polling runs on the suite thread. No new shared mutable state, locks, lock-order change or heavy operation inside a changed lock scope. |
| Lifecycle and initialization | Writer-owned executor shutdown precedes successful commit-message publication; polling and diagnostics match that boundary. SET_VAR values restore in statement-finally cleanup. No static/global initialization change. |
| Configuration | No new configuration. Existing Hive overrides apply to individual statements. Eight fixture partitions stay below the existing partition-batch threshold; explicit streaming checks still force their own thresholds. |
| Symbol/storage compatibility | No production symbols, storage formats or upgrade behavior change. |
| Thrift/protobuf compatibility | No definition, field id or enum change. |
| Parallel paths | Both exact EXPLAINs are fixed. Synchronous and forced streaming Hive reads remain; Paimon JNI selection and existing abort/close paths were traced. |
| Special conditions and errors | Missing mandatory endpoint data, HTTP/JSON errors, failed EXPLAIN predicates and retry timeouts propagate as test failures. Comments explain the selector and exact-enumeration choices. |
| Test coverage | Hive retains compression/partition/predicate checks across scan modes. Paimon performs 120 sequential writes across ten checked phases; existing unit coverage checks shutdown of an active executor. Failure-path tests were inspected, not run. |
| Expected results | No result file or Hive expected value changes. Paimon writes distinct ids and checks 120,000 rows. Existing external fixture bytes were not regenerated. |
| Observability | Phase logs retain backend totals and matching writer-name counts, including on assertion failure. EXPLAIN failures retain the plan text. |
| Transactions and persistence | No production transaction, EditLog, replay or failover behavior changes. |
| Data writes and crash behavior | Test writes use the existing synchronous prepared/closed/committed path. Error/abort cleanup uses the existing shutdown boundary; no new production atomicity or crash-recovery behavior. |
| FE/BE variable propagation | No new variable or serialization path. Existing connector-session capture sees the query-local Hive setting. |
| Performance | Work remains bounded to 120 inserts and per-phase backend polling; complete Hive enumeration covers the small existing fixture. No production hot-path change. |
| Other issues | Rechecked selector collisions, lazy executor activation, cold/warm catalog behavior, closure semantics and test isolation; no substantiated introduced bug remained. |
Core data-visibility, version, delete-bitmap and native memory-accounting implementations are unchanged; error propagation in the changed assertions was verified statically. There were no existing inline threads or confirmed existing P0/P1 blockers. No additional user focus was supplied.
Validation limits: static review only, with no builds, tests or repository source edits. The author's reported local runtime validation was not independently rerun, and external fixture binaries were unavailable for regenerating the unchanged expected counts. Both changed paths and the identified risks were covered; no unresolved code question remains.
Review converged after 1 round: both complete reviewers and the separate risk reviewer returned NO_NEW_VALUABLE_FINDINGS, and all risks were independently resolved with evidence.
Review converged after 1 round(s).
|
run buildall |
What problem does this PR solve?
test_hive_compress_type_large_datacompares exact file and split counts against streaming EXPLAIN estimates. A cold catalog can report(approximate)inputSplitNum=1024even though the fixture contains 16 files. Use statement-localSET_VAR(enable_external_table_batch_mode=false)for both exact plan checks. Keep the synchronous and explicitly forced streaming result checks.test_paimon_write_thread_lifecycletreats growth in all BE JVM threads as a writer leak. Existing native blocking workers attach to the JVM on their first JNI call and remain attached until the worker exits. A stable global baseline is therefore not guaranteed, even when every writer closes its executor correctly. Check the writer-owneddoris-paimon-threads through the existing process-thread endpoint instead, accounting for Linux's 15-byte thread-name limit. Require them to disappear within 30 seconds after each of ten write phases, and retain total process-thread counts in diagnostics. This detects even one surviving writer thread without increasing a tolerance or moving a baseline.Release note
None
Check List (For Author)
Local validation used the original DEBUG/ASAN build
6c27d4d8b0b4649f6517e15792fba672a54e2935, an isolated FE/BE, Hive 3/HDFS, Spark 4/Paimon, and MinIO:parallel_pipeline_task_num=4, Paimon failed bounded warm-up with JVM counts451, 497, 525, 587, 606, 676. Thread dumps mapped the growing attached threads top_normal_blocki; writer compaction threads had exited. The failure reproduces the invalid global-count assumption, through the warm-up assertion rather than the later baseline assertion.run-regression-test.sh; 2 suites passed, 0 failed, 0 skipped. Paimon executed 120 inserts, verified 120,000 rows, and passed all ten writer-thread checks. Its attached blocking-worker count still grew from 54 to 368, so passing does not depend on a pre-warmed JVM. BE logs confirm the compaction executor actually ran.ExecutorThreadFactoryname truncates todoris-paimon-coon JDK 17/Linux.git diff --checkpassed.Check List (For Reviewer who merge this PR)