mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
38a38dda22b44692cb3ed317b1f510a9b9b01493
32955
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
38a38dda22 |
[fix](ci) Use GPT-6 Sol for review capacity fallback (#69988)
### What problem does this PR solve? Related PR: #69953 When GPT-6.1 Sol encounters five capacity failures, review recovery currently switches to GPT-6 Astra, which has continued to encounter capacity failures in observed runs. Change the capacity fallback to `gpt-6-sol` and update the existing test fixtures and assertions to exercise that target. The production change is one workflow argument. The primary model remains `gpt-6.1-sol` with `xhigh`; the five-failure threshold, seven total capacity attempts, 300-second backoff, same-session recovery, shared budget, ledger and round preservation, head-change checks, and submission verification are unchanged. |
||
|
|
7e6f6cb3a3 |
[fix](fe) Resolve DEFAULT(column) on Iceberg tables from the write schema (#69488)
### What problem does this PR solve? Issue Number: None Related PR: #66538, #57531, #65851 Problem Summary: **In short.** On an Iceberg table, `DEFAULT(column)` anywhere except `UPDATE ... SET` and `MERGE ... VALUES` reads the default from the read schema, which carries no Iceberg write-default. This PR resolves it against the write schema that the statement pinned for its target, as branch-4.1 and branch-4.2 do. **Background.** An Iceberg `write-default` applies to rows written after it is set. So it lives in the writer schema that a write statement pins (`IcebergWriteSchemaContext`), not in the columns of the read schema. #65851 resolves `DEFAULT(column)` against that pinned schema in `UPDATE ... SET`, `MERGE ... UPDATE SET` and `MERGE ... INSERT VALUES` (`ConnectorWriteSchemaUtils`). Every other `DEFAULT(column)` either fails at binding or reaches `RewriteDefaultExpression` (#57531), which reads the default from the bound column. branch-4.1 #66538, also in branch-4.2, resolves these against the pinned write target too. It rejects `DEFAULT(column)` on an Iceberg column in a statement that does not write the table. **The problem, and what it cost.** Reproduced on a local cluster with a `file://` format-version 3 table: `CREATE TABLE t (id INT, c INT)`, `ALTER TABLE t ADD COLUMN d INT DEFAULT 42`, one row `(0, 0)`, so `d = 42`. | Statement | Master today | After this PR (same as branch-4.2) | |---|---|---| | `INSERT INTO t (id, c, d) VALUES (1, 1, DEFAULT(d))` | `Unknown column 'd' in 'table list'` | writes 42 | | `INSERT INTO t (id, c, d) SELECT 2, 2, DEFAULT(d) FROM t WHERE id = 0` | writes NULL | writes 42 | | `INSERT INTO t (id, c, d) SELECT 3, 3, DEFAULT(d)` | `Unknown column 'd'` | writes 42 | | `UPDATE t SET c = 100 WHERE d = DEFAULT(d)` | updates nothing: `d = NULL` makes the plan an empty set | updates every row, `d = 42` is pushed down to the scan | | `MERGE INTO t USING (SELECT 0 AS id) s ON t.id = s.id AND t.d = DEFAULT(t.d) WHEN MATCHED THEN UPDATE SET c = 300` | matches nothing | updates row 0 | | `SELECT DEFAULT(d) FROM t` | NULL | `DEFAULT(d) must reference a column of the table that this statement writes` | The `UPDATE` and `MERGE` cases fail silently: the statement succeeds and changes nothing. **How this PR fixes it.** - fe-core cannot tell from a column whether its connector keeps defaults in the read schema. Hive does: an HMS default shows up in both schemas. Iceberg does not. The new connector capability `SUPPORTS_WRITE_ONLY_DEFAULTS` says which kind a table is. The Iceberg connector declares it. The Hive connector passes it on to iceberg-on-HMS tables, like the other per-table capabilities, but only through the two-argument `getTableSchema`. Since #69299, a statement that pins the table at its latest snapshot reads the schema from the snapshot overload, which reflects none of them, so on an iceberg-on-HMS table such a statement keeps master's results (for example, `UPDATE t SET c = 100 WHERE d = DEFAULT(d)` still changes no row). Reflecting the sibling's capabilities there changes planning for all of them and is left to a follow-up PR. - For a target that declares it, `InsertUtils.normalizePlan` resolves `DEFAULT(column)` in `INSERT ... VALUES` and in the select lists of `INSERT ... SELECT`, including the replacements of `SELECT * REPLACE(...)`, against the pinned write schema. It does this by name, before the source relations are bound, so the name always refers to a target column, as in branch-4.2. In MySQL, `DEFAULT(column)` in a `SELECT` refers to a source column instead. A derived table in `FROM` is a query of its own and is left alone: `DEFAULT(column)` in it still names a column of its own `FROM` clause. - `RewriteDefaultExpression` takes the default for a `DEFAULT(column)` bound to such a table from the pinned write column with the same field id. That covers the `WHERE` of `UPDATE` and the `ON` of `MERGE`. A MERGE resolves it whatever its WHEN branches, also when they all delete, as branch-4.2 does. If the statement pinned no write schema for that table, it fails. A `DELETE` pins the writer schema too, for its delete sink, but it writes no row values, so `RowLevelDmlCommand` marks its target and `DEFAULT(column)` on it fails with `DEFAULT(d) cannot be used in a DELETE: the defaults of this table apply only to the rows that a statement writes`, as in branch-4.2, whose DELETE installs no write schema context. Master reads NULL there, so `DELETE FROM t WHERE d = DEFAULT(d)` deletes nothing; resolved to the write-default it would delete every row with `d = 42`. - A resolved default is cast to the type of its writer column before the expression around it is bound, as `RewriteDefaultExpression` already does for a bound `DEFAULT(column)`. The parsed literal `-1` is a TINYINT, so without the cast `bit_count(DEFAULT(c))` on an INT column would count 8 bits instead of 32. The external `UPDATE`, `MERGE` and changelog paths use the same helper. `RewriteDefaultExpression` rejects a bound `DEFAULT(column)` whose writer column has another type than the read column the statement was bound with, which happens only when the table changed outside Doris after Doris cached it, and asks to refresh the table: the expressions around it were bound against the read type. - Hive, Paimon and internal tables do not declare the capability, so they behave as before. - The new enum constant changes the connector plugin API surface, so `connector.plugin.api.version` goes from 26.0 to 27.0, as for the other recent changes to that surface (#69299, #69588, #69298, #69683 and #69835 took 22.0 to 26.0). **Where `DEFAULT(column)` resolves on an Iceberg target (direct Iceberg catalogs).** | Statement | Position of `DEFAULT(column)` | Result | branch-4.2 | |---|---|---|---| | `INSERT` / `INSERT OVERWRITE` | `VALUES` rows | the target column's write-default, typed as the column | same | | | the INSERT's own select lists (with or without `FROM`, set operation branches, `SELECT * REPLACE`) | the target column's write-default, by name, before binding | same, except `* REPLACE`, which branch-4.2 binds to the source | | | `GROUP BY` / `ROLLUP` outputs, a derived table in `FROM`, `WITH` definitions, subqueries, `WHERE` / `ON` | bound like any `DEFAULT(column)` (last row) | same, except derived tables, which branch-4.2 resolves against the target | | | a `VALUES` branch of a set operation | fails with an unknown column, as on master | same | | | a qualifier other than the target (`DEFAULT(src.c)`, `DEFAULT(ctl.db.t.c)`) in the positions above | fails: `column in assignment list is invalid` | same | | `UPDATE` | `SET` / `WHERE` | write-default | same | | `MERGE` | `ON` and `WHEN ... AND` conditions, whatever the WHEN branches, also when they all delete | write-default | same | | | `UPDATE SET` / `INSERT VALUES` | write-default | same | | `DELETE` (also `DELETE ... USING`) | anywhere | fails: `cannot be used in a DELETE` | fails: its DELETE has no write schema context | | any | a bound `DEFAULT(x.c)` | `x` is the written table: its write-default by field id; an OLAP or Hive table: that column's default; another Iceberg table: fails with `must reference a column of the table that this statement writes` | same | A bound `DEFAULT(column)` whose writer column has another type than the read column fails and asks to refresh the table. Where `DEFAULT(column)` is bound rather than resolved by name (the third row), it keeps its meaning from before this PR and names a column of its own `FROM` clause, as on master and branch-4.2: for example in `INSERT INTO t SELECT id, DEFAULT(d) FROM src GROUP BY id, d` with an OLAP or Hive `src`, it takes the default of `src.d`. **Results.** - Local cluster, with the table above: the table shows what each statement did before and after. `EXPLAIN UPDATE t SET c = 5 WHERE d = DEFAULT(d)` goes from an empty set to a scan with `d = 42` pushed down to Iceberg. - Unit tests: 6 new tests in `InsertUtilsTest`, 6 in the new `RewriteDefaultExpressionTest` and 1 in `RowLevelDmlCommandTest`. They cover VALUES, and INSERT ... SELECT both from a staging relation whose own column is also `name` and without FROM: each resolves to the pinned default, and an unknown column fails. `DEFAULT(column)` inside a derived table, qualified or not and also under a UNION branch, is left to binding. A `SELECT * REPLACE(DEFAULT(name) AS name)` replacement resolves like the rest of the select list and keeps the star's qualifier and `EXCEPT` list. The pinned write column wins over the read column. A statement that writes nothing fails. A table without the capability is unchanged. With master's `InsertUtils` and `RewriteDefaultExpression`, the 5 tests of the fix fail and the 2 tests of the unchanged path pass. The `SELECT * REPLACE` test fails with this PR's previous `InsertUtils`, where the replacement stays `DEFAULT(name)`. `IcebergConnectorTest`, `HiveConnectorMetadataSiblingDelegationTest` and `ConnectorPluginSurfaceTest` pass. So do the fe-core tests around the write schema: `ConnectorWriteSchemaUtilsTest`, `ExternalRowLevelUpdatePlanBuilderTest`, `ExternalRowLevelMergePlanBuilderTest`, `ChangelogRowLevelDmlTransformTest` and `PluginDrivenExternalTableTest`. Two of the tests pin the typing: `bit_count(DEFAULT(bits))` gets the default as an INT in VALUES and in SELECT, and fails without the cast; a BIGINT writer column over an INT read column is rejected, directly and under an IF bound to INT. `DEFAULT(column)` on the target of a DELETE fails even with a pinned write-default, and only a DELETE marks its target (an UPDATE does not); skipping the check, or marking UPDATE instead of DELETE, makes exactly these two tests fail. `PositionDeleteRowLevelDmlTransformTest`, `PhysicalPlanTranslatorIcebergRowLevelDmlTest`, `DeleteFromCommandTest` and `UpdateCommandTest` pass. - Regression: `test_iceberg_initial_defaults` now writes rows 20 to 22 with `DEFAULT(default_int)` in VALUES and in INSERT ... SELECT with and without FROM. It then updates them with `WHERE default_int = DEFAULT(default_int)` and `MERGE ... ON ... = DEFAULT(...)`, for Parquet and ORC, and checks the rows in the `.out`. A plain `SELECT DEFAULT(default_int)` must fail, and so must a `DELETE ... WHERE default_int = DEFAULT(default_int)` on rows 20 to 22, which hold the write-default. The suite passed locally before the DELETE block was added, and the whole suite, DELETE block included, has since passed in two External regression runs on CI. With master's FE it fails at the first of these statements: `Unknown column 'default_int' in 'table list'`. |
||
|
|
fe58e58386 |
[feature](cloud) Support declaring compute_group on routine load and async MV (#69749)
### What problem does this PR solve? Issue Number: None Related PR: #67010 Problem Summary: **In short.** This ports branch-4.1 #67010 (also on branch-4.2) to master: CREATE / ALTER ROUTINE LOAD and async materialized views accept a `compute_group` property in cloud mode, so a background job can be pinned to a compute group instead of inheriting one implicitly, and the declared group is re-checked before every task. **Background.** In cloud mode every query runs in a compute group. A routine load job snapshots the compute group of the session that created it, and an async MV has no compute group of its own: an automatic refresh runs wherever `admin` resolves to, and a manual `REFRESH` borrows the group of the session that triggered it. **The problem, and what it cost.** On master nothing lets a user choose where a background job runs: | | master today | with this PR | |---|---|---| | Routine load | runs in whatever group the creating session was on; ALTER cannot move it | `"compute_group" = "cg_etl"` on CREATE or ALTER pins it | | Async MV, automatic refresh | runs where `admin` resolves to | runs in the declared group | | Async MV, manual `REFRESH` | runs in the triggering session's group, so the same MV refreshes in different places depending on who triggered it | runs in the declared group; an MV that declares none keeps borrowing the session's group | | Async MV, `REFRESH ... WITH DRY RUN` and `EXPLAIN REFRESH` | run and plan where `admin` resolves to | run and plan in the declared group; an MV that declares none resolves as today | | A pinned group is dropped, or the owner's USAGE on it is revoked | (no pinning) | the next task fails with that reason instead of a later "no available BE found" | | `compute_group` in non-cloud mode, or `DEFAULT` | (no property) | rejected at CREATE / ALTER | Four smaller defects go with it on master: CREATE ROUTINE LOAD checks the `workload_group` in the session's compute group namespace although the job runs elsewhere; `RoutineLoadJob.plan()` builds the sink's plan context from the raw snapshot field rather than the effective group; and the statement re-parsed when a routine load job is loaded from metadata resolves its `workload_group` again. On image load that lookup runs before any workload group exists (`PersistMetaModules` reads `routineLoadJobs` before `workloadGroups`, and until then the Env holds the empty `WorkloadGroupMgr` its constructor created), and a failure there sets the job to the final `CANCELLED`, so a job created with `workload_group` is cancelled by the first restart that loads it from an image (shown by a unit test with an empty manager, not reproduced on a cluster). And when the partition refresh fails, which `RoutineLoadJob.update()` runs before it takes the job lock, it pauses a NEED_SCHEDULE job without that lock, so a STOP that completes in between is turned back into PAUSED on the master while replay keeps STOPPED (Kafka metadata failures and Kinesis shard listing alike). **How this PR fixes it.** Same design as #67010: - The declaration is stored in the existing property maps (`RoutineLoadJob.jobProperties`, `MTMV.mvProperties`) and written only when declared: no new persisted field, journal type or meta version, and an absent key keeps today's implicit resolution, which is also how a later binding model tells a pinned group from an inherited one. `DEFAULT` is rejected because that model reserves it for "follow the owner's default group"; non-cloud mode rejects the property, and `ConnectContext.setCloudCluster()` is a no-op outside cloud mode. - `ComputeGroupBindingUtil` holds the two checks: `validateDeclaredComputeGroup` at CREATE / ALTER (USAGE for the current user, then existence, the order of `USE @<group>`), and `checkComputeGroupBeforeTask` before every task, against the identity the task runs as. A missing group keeps `INTERNAL_ERR`, so a routine load job is paused retryably and resumes by itself when the group comes back (a group scaled to zero backends leaves the cluster map and returns); a revoked USAGE uses `CANNOT_RESUME_ERR`, so `ScheduleRule` does not keep resuming it. - Routine load resolves the effective group in `RoutineLoadJob.getCloudCluster()`, which backend selection, the task sent to BE, the workload group namespace, the plan context and the `ComputeGroup` column of SHOW ROUTINE LOAD all use, so every Kafka job follows it (Kinesis routine load is rejected in cloud mode, the only mode with compute groups). `RoutineLoadTaskScheduler` re-checks the declared group before backend allocation and before `beginTxn`. The partition refresh of a Kafka job runs the same check before its metadata request, which goes through the group's backends and, after RESUME, comes before any task: a dropped group pauses the job with `Compute group '<name>' not found.` instead of `no alive backends` and advice to check the Kafka properties. An auto resume attempt skips that check: it refreshes a job that is still PAUSED, which cannot be paused again with another reason, so the attempt goes on to NEED_SCHEDULE and the check before the first task records a revoked USAGE as `CANNOT_RESUME_ERR`. Every pause from the refresh (this check, a failed Kafka metadata request, a failed Kinesis shard listing) goes through `RoutineLoadJob.pauseIfNeedSchedule()`, which takes the job lock and checks NEED_SCHEDULE again, so it cannot undo a concurrent STOP. CREATE validates the workload group in the declared group's namespace. An ALTER that sets `compute_group` validates in that group the workload group the job has afterwards (the one the ALTER sets, or else the one it keeps), as the owner the tasks run as. An ALTER that sets `workload_group` alone is still not checked, as on master. The statement re-parsed at metadata load adopts both names without resolving either, because a failure there would cancel the job for good: the declared group can have been dropped (and the stored statement is the CREATE one, so a job moved to another group by ALTER still names its old group there), and on image load the workload groups are not loaded yet. - CREATE / ALTER ROUTINE LOAD start a suspended declared group the way `USE @<group>` does (`waitForAutoStart`), on the statement thread before any job lock, since they send their Kafka metadata requests (custom partitions, offset conversion) through that group's backends. An ALTER that sets `compute_group` and `kafka_offsets` together converts the offsets in the group it declares, because the job's current group may be gone. The scheduler keeps resolving backends by name without starting a suspended group, as master does for every cloud routine load job. - SHOW CREATE ROUTINE LOAD writes `compute_group` when the job declares one. It writes the declared value, which follows ALTER, and never the effective group, so a job recreated from the statement is pinned the same way, and the statement of a job that declares nothing does not pin the group it inherited. SHOW CREATE MATERIALIZED VIEW already renders every MV property. - An async MV applies the declared group, then the session's group, on the planning context of the task and on every refresh context, and re-checks the declared group before each refresh. Each refresh context reads the declaration once and binds, records and re-checks that value. `REFRESH ... INCREMENTAL WITH DRY RUN` runs outside the task, on the current FE, in a context of its own: it binds the declared group to that context and re-checks it the same way. `EXPLAIN REFRESH` binds it to its planning context without the re-check, as the task does for its own. Neither borrows the session's group for an MV that declares none, as on master. Where master differs from branch-4.x: - master's `MTMVTask` splits a refresh into the IVM incremental path and the partition / complete path, each with its own context. `applyComputeGroup()` is factored out of the existing `setupComputeGroup()`, and the re-check runs in both paths. - master's routine load also has a Kinesis source, but `KinesisRoutineLoadJob` rejects cloud mode, so a Kinesis job never has a compute group to declare. - `REFRESH ... WITH DRY RUN` (from master's IVM) and `EXPLAIN REFRESH` exist only on master. Both build their context in `RefreshMTMVCommand` instead of `MTMVTask`, so that command binds the declared group itself, and the dry run re-checks it. - SHOW CREATE ROUTINE LOAD writes the declared `compute_group`; branch-4.x does not. - An ALTER that sets `compute_group` checks the job's workload group in that group; branch-4.x does not, so there a job moved to a group without its workload group fails every task. - The Kafka partition refresh checks the declared group before its metadata request, except on an auto resume attempt; branch-4.x has no check there. - Each MV refresh context reads the declaration once to bind and re-check it; branch-4.x reads it twice. - The 4.x unit test for a task without a task context is not carried over: `MTMVJob` always creates a task with a context, and master's `setupComputeGroup()` already relies on it. - The three unit test classes are converted to JUnit 5, as master requires, and `MTMVComputeGroupTest` gains two cases for the master-only parts: `setupComputeGroup()` applies and records the declared group, and the MV re-check fails for a declared group that no longer exists while it ignores the session's group. `RefreshMTMVCommandTest` gains four for the dry run and EXPLAIN REFRESH contexts, and `RoutineLoadJobTest` one for SHOW CREATE ROUTINE LOAD. - `ConnectContextTest.testThreadInfoUsesSessionCloudCluster` (#68111) sets the session variable directly instead of calling `ConnectContext.setCloudCluster()`, which is now a no-op outside cloud mode; branch-4.x made the same change when it picked #68111 (#68948). **Results.** The unit tests below pass locally. The regression suites need a cloud cluster and were not run locally: `test_routine_load_compute_group` and `test_mtmv_compute_group` are p0 suites, so P0 runs their non-cloud rejection checks and cloud_p0 runs them in cloud mode (the routine load part also needs Kafka); `test_compute_group_binding_isolation` is a docker suite that starts its own two-group cloud cluster. |
||
|
|
2b8908b883 |
[feature](be) Count the changelog TTL cleanups in the BE metrics (#69868)
### What problem does this PR solve? Issue Number: #69907 Related PR: None Problem Summary: **In short.** The BEs clean up the changelog of a table by TTL, but `/metrics` cannot tell whether that cleanup runs, fails, or reclaims anything. This PR counts each cleanup on the BE as it finishes, in three new series: how many succeeded or failed, how many bytes of expired changelog they dropped, and when one last succeeded. **Background.** A table with `changelog.enable` keeps its changelog in a hidden changelog index, and `changelog.ttl_seconds` bounds how long the changelog is kept. The BEs remove what is older than that in a cumulative compaction of a tablet of that index (a changelog tablet): rowsets whose commit TSO is at or before the TTL cutoff are left out of the merge, so the output replaces them without their rows. Two submitters start these compactions, both as `CHANGELOG_COMPACTION`: the changelog TTL scanner, for tablets it finds holding expired rowsets, and the regular compaction producer; either compaction picks expired rowsets first. **The problem, and what it cost.** - No alert can watch the cleanup. The bvars `changelog_ttl_discovered`, `changelog_ttl_submitted` and `changelog_ttl_failed` are only on the brpc `/vars` page, and they count what the TTL scanner found and submitted, not what the compactions did: a cleanup that fails after it was submitted counts nowhere, and nothing counts the bytes removed. - The only changelog series in `/metrics`, `compaction_task_state_total{type="changelog"}`, is the number of changelog compactions running and pending. A cleanup that keeps failing lets the changelog grow without bound, and nothing on the BE says why. **How this PR fixes it.** - Three series, counted as a compaction that drops expired changelog ends, and registered at startup so they exist from 0: | Series | Type | Labels | Value | | --- | --- | --- | --- | | `doris_be_changelog_ttl_cleanup_total` | counter | `result`=success/failed | the compactions that drop expired changelog rowsets, counted as each ends | | `doris_be_changelog_ttl_reclaimed_bytes_total` | counter | | the total disk size of the expired rowsets the successful ones dropped | | `doris_be_changelog_ttl_last_success_timestamp_seconds` | gauge | | when one last succeeded, in seconds since the epoch | - A compaction is a cleanup when one of its inputs is expired changelog: at or below the visible version its TTL snapshot was taken at, with a commit TSO at or before the cutoff. That is exactly what `filter_changelog_ttl_rowsets` leaves out of the merge, and the two now ask one predicate, `Compaction::is_expired_changelog_rowset`, which `pick_expired_changelog_rowset` uses too. It is decided from the inputs rather than from what the merge filtered, so that a cleanup that fails before its merge, a cloud compaction that cannot refresh its tablet meta for example, is counted as failed. - Counted where the execution of the compaction ends: `CumulativeCompaction::execute_compact` in shared-nothing mode, `CloudCumulativeCompaction::record_compaction_success` and `record_compaction_failure` in cloud mode. A compaction that does not get to run has not started a cleanup and is not counted: one that finds the tablet under another cumulative compaction, whether in its preparation or as its execution takes the cumulative compaction lock again, and one whose job the meta service refuses. - In cloud mode the BE that runs the compaction counts it, so a query sums over the BEs; the shared storage is not measured. The dropped rowsets become stale rowsets, whose files the stale rowset sweep or the recycler deletes later; the bytes are counted when the rowsets leave the tablet. - The bvars stay as they are. **Results.** Before this PR, the `/metrics` of a BE has none of these series. After it, on a local shared-nothing cluster, a table with `changelog.ttl_seconds` = 1 and two rows inserted (regression `test_changelog_ttl_cleanup_metrics`): | | `cleanup_total{result="success"}` | `cleanup_total{result="failed"}` | `reclaimed_bytes_total` | `last_success_timestamp_seconds` | | --- | --- | --- | --- | --- | | BE started | 0 | 0 | 0 | 0 | | once the changelog rows have been cleaned up | 1 | 0 | 1112 | the time of the cleanup | The 1112 bytes are what the BE logs for that changelog compaction: `input_rowsets_total_size=1.09 KB`, `output_rowset_total_size=0`, `filtered_row_num=2`. **Classes, and how they call each other.** - `DorisMetrics`: the three series. - `Compaction`: `is_expired_changelog_rowset` (new), shared by `pick_expired_changelog_rowset` and `filter_changelog_ttl_rowsets`; `record_changelog_ttl_cleanup` (new) counts a cleanup from the inputs. - `CumulativeCompaction`, `CloudCumulativeCompaction`: call it as the execution ends. ``` CumulativeCompaction::execute_compact shared-nothing └─ CompactionMixin::execute_compact ─► merge: filter_changelog_ttl_rowsets ─┐ └─ record_changelog_ttl_cleanup(ok) │ is_expired_changelog_rowset CloudCumulativeCompaction::execute_compact cloud │ └─ CloudCompactionMixin::execute_compact ─► merge: same filter ─────────────┤ └─ record_compaction_success ─► record_changelog_ttl_cleanup(true) │ record_compaction_failure ─► record_changelog_ttl_cleanup(false) │ └─ DorisMetrics::changelog_ttl_* ◄─────────┘ (from the inputs) ``` Nothing else changes: no config, no thrift or proto change, and what the compactions pick and drop stays the same. |
||
|
|
9dc9dc7566 |
[feature](ivm) Report how a refresh task refreshed an IVM MV, and count the refresh tasks on the Master (#69856)
### What problem does this PR solve? Issue Number: #69907 Related PR: None Problem Summary: **In short.** A refresh of an IVM materialized view can keep the MV up incrementally or quietly recompute it, and nothing tells the two apart: `tasks("type"="mv")` does not say which kinds of work a task committed, and `/metrics` has no IVM series at all. This PR makes each refresh task of an IVM MV report how it refreshed the MV (`IvmRefreshMethod` in `tasks()`) and why it failed or fell back, keeps a refresh summary on the MV for the IVM status table that follows, and counts the tasks on the Master in five new counters. **Background.** An IVM MV is refreshed by a task that runs one or more attempts: the incremental attempt (the delta of its base tables' streams, after rebuilding the MV partitions an invalidation marked), a partition rebuild, and a whole-MV rebuild. A request that may fall back (AUTO, INCREMENTAL FALLBACK, PARTITIONS FALLBACK) moves on to the next attempt when one gives up, for example to COMPLETE after the delta meets a MIN/MAX boundary. `tasks()` already reports `RefreshMode` (which partitions the task covered), `IvmFallbackReason` (why the incremental attempt gave up, or why a strict request failed) and `IvmRebuiltPartitions`. **The problem, and what it cost.** - An operator cannot tell a refresh that kept the MV up incrementally from one that recomputed it. `RefreshMode` says `COMPLETE` for any whole-MV scope, and `IvmFallbackReason` is set even when the attempt that replaced the delta wrote nothing, or when a strict request failed. - No alert can watch IVM refreshes. The asynchronous MV counters (`async_materialized_view_task_success_num`, `_failed_num`, `_skip_num`) cover every MV and say nothing about the method, the fallbacks, or why tasks fail. An MV that falls back to COMPLETE on every run, the expensive failure mode of IVM, goes unnoticed until the load of the cluster shows it. - A task that the full MV task queue turns away never runs, and nothing counts it. - The task history keeps only `max_persistence_task_count` tasks, so how many times in a row an MV has failed, or when it last fell back, cannot be read back from it. **How this PR fixes it.** - `MTMVTask` notes each batch that commits data (the delta, a partition rebuild, a whole-MV rebuild) and decides `IvmRefreshMethod` once, as the task publishes its result: `NONE`, `INCREMENTAL`, `PARTITION_REBUILD`, `COMPLETE`, or `MIXED` when more than one kind committed, typically the rebuild of invalidated partitions before the delta. `tasks("type"="mv")` gets the `IvmRefreshMethod` column, NULL while a task runs and for a task of an MV that is not an IVM MV. - A task falls back when the incremental attempt gives up and an attempt replacing it commits data. Not fallbacks: building the first baseline of an MV no refresh has succeeded on, the rebuild of invalidated partitions before the delta, an explicit COMPLETE or PARTITIONS request, and a fallback that commits nothing. - A failed task reports why: the `IvmFailureReason` a request that may not fall back failed with, or else the stage it failed in, `QUERY_UNUSABLE`, `PARTITION_SYNC_FAILED`, `EXECUTION_FAILED` or `OTHER`, told apart by where the failure was thrown rather than by its message. - The method, the failure reason and whether the task fell back are journaled with the task, as new Gson fields that an older FE ignores. The MV keeps an `IvmRefreshSummary`, updated from each task result in `MTMV.addTaskResult`, which the Master and the replay of the same journal record run alike, from what the record carries: consecutive failures, the latest success and failure (with the reason and the error message, cut to 512 characters), the latest method and run time, and the fallback count with the latest reason. A canceled task leaves it as it is. Nothing reads it in this PR; `information_schema.ivm_mv_status` and the per-MV series of a follow-up PR do. - Five counters, counted on the Master as each task of an IVM MV ends. Every label combination is registered up front, so each of the 61 series exists from 0: | Series | Type | Labels | Value | | --- | --- | --- | --- | | `doris_fe_ivm_refresh_tasks_total` | counter | `result`=success/no_change/failed/canceled, `method`=none/incremental/partition_rebuild/complete/mixed | refresh tasks that ended; `no_change` is a SUCCESS that committed nothing | | `doris_fe_ivm_refresh_duration_seconds_total` | counter | `method` | the run time, StartTime to FinishTime, of the tasks that succeeded or failed; a canceled task has no finish time | | `doris_fe_ivm_fallbacks_total` | counter | `reason`=the `IvmFailureReason` values, `incomplete_refresh_snapshot` | tasks that fell back | | `doris_fe_ivm_refresh_failures_total` | counter | `reason`=the `IvmFailureReason` values, the four stages | tasks that failed | | `doris_fe_ivm_schedule_rejected_total` | counter | | tasks the full MV task queue turned away | Like the other FE event counters they live in the registry of every FE and only the Master counts in them, so a query sums them over the FEs; a restart or a new Master starts them from 0. - `AbstractTask.onRejected`, a new hook that `TaskProcessor.addTask` calls when the executor queue rejects a task. Both the instant dispatch (`JobScheduler`) and the timer dispatch (`DispatchTaskHandler`) add tasks through it, and `MTMVTask` counts the rejection for an IVM MV. A timer-dispatched task that is turned away is canceled as before, so it also counts as `canceled`. **Results.** Before this PR, `tasks()` has no method column and `/metrics` has none of these series. After it, on a local cluster, one IVM MV refreshed through each case (regression `test_ivm_refresh_method_metrics`): | Refresh | `tasks()`: Status, IvmRefreshMethod, IvmFallbackReason | Counters that moved, besides the run time | | --- | --- | --- | | first AUTO refresh of a new MV | SUCCESS, COMPLETE, NULL | `tasks{success,complete}` | | INCREMENTAL after an insert | SUCCESS, INCREMENTAL, NULL | `tasks{success,incremental}` | | INCREMENTAL with nothing new | SUCCESS, NONE, NULL | `tasks{no_change,none}` | | INCREMENTAL after deleting the row that holds a group's minimum | FAILED, NONE, MIN_MAX_BOUNDARY_HIT | `tasks{failed,none}`, `failures{min_max_boundary_hit}` | | INCREMENTAL FALLBACK in the same state | SUCCESS, COMPLETE, MIN_MAX_BOUNDARY_HIT | `tasks{success,complete}`, `fallbacks{min_max_boundary_hit}` | | COMPLETE | SUCCESS, COMPLETE, NULL | `tasks{success,complete}` | | INCREMENTAL after truncating a base partition and inserting into another | SUCCESS, MIXED, NULL | `tasks{success,mixed}` | The first refresh of a new MV finds every partition still to be filled and goes straight to COMPLETE; it counts no fallback. The MV holds what its base table does after each case. **Classes, and how they call each other.** - `IvmRefreshMethod`, `IvmRefreshFailureStage` (`org.apache.doris.mtmv.ivm`): the methods, and the stages a failure is reported by. - `MTMVTask`: notes committed batches (`recordCommitted`), decides whether giving up the incremental attempt is a fallback (`givingUpTheIncrementalAttemptIsAFallback`), moves the failure stage along, settles the method and the failure reason in `after()`, and counts the task (`recordIvmRefreshMetrics`) and a rejection (`onRejected`). - `IvmRefreshSummary` (`org.apache.doris.mtmv.ivm`): persisted on `MTMV` and updated by `update(MTMVTask)` from `MTMV.addTaskResult`; `MTMV.hasSucceededIvmRefresh` tells the first baseline apart. - `MetricRepo`: registers the five counters. `DoubleCounterMetric` (new) keeps a total in seconds without rounding each addition. - `AbstractTask.onRejected` (new, empty by default), called by `TaskProcessor.addTask`. ``` MTMVTask.run() worker thread ├─ executeIvmAttempt │ ├─ rebuild invalidated partitions ──► recordCommitted(PARTITION_REBUILD) │ └─ delta (IvmIncrRefreshManager) ───► recordCommitted(INCREMENTAL) │ gave up ─► fallingBack = givingUpTheIncrementalAttemptIsAFallback() ├─ PARTITIONS / COMPLETE attempts ───────► recordCommitted(...) ivmFallback once fallingBack └─ failureStage: QUERY_UNUSABLE ─► PARTITION_SYNC_FAILED ─► EXECUTION_FAILED, or an IvmFailureReason onSuccess / onFail / cancel ─► after() ├─ ivmRefreshMethod, ivmFailureReason settled ├─ recordIvmRefreshMetrics() ─► MetricRepo.COUNTER_IVM_* Master only └─ Env.addMTMVTaskResult ─► MTMV.addTaskResult ─► IvmRefreshSummary.update(task) (journaled as ADD_TASK; the replay calls addTaskResult the same way) TaskProcessor.addTask ── queue full ─► task.onRejected() ─► MTMVTask: COUNTER_IVM_SCHEDULE_REJECTED ``` Nothing else changes: no thrift or proto change, no new config, and the existing columns and series stay as they are. |
||
|
|
fcf143c1aa |
[fix](function) Align timezone offset parsing across FE and BE (#68979)
### What problem does this PR solve? Issue Number: N/A Related PR: #68865 Problem Summary: Timezone offsets can be accepted or rejected differently by FE constant folding and BE runtime CAST. For example, a DATETIME suffix of `-14:00` is valid under the literal policy but is rejected by the BE session-zone resolver. Session offsets such as `+14:30` and `-12:30` can bypass validation through the preloaded BE cache, while FE CONVERT_TZ applies different minute restrictions to bare and UTC/GMT-prefixed offsets. Root cause: these entry points duplicate offset validation and mix session and literal policies. Introduce a shared FE parser using the existing timezone aliases and upstream canonicalization helper, retain separate session/DATETIME/TIMESTAMPTZ policies, validate BE cache endpoints, and construct parsed literal offsets through TimezoneUtils. Strict and non-strict BE CAST use the same checks. Numeric TIMESTAMPTZ offsets are limited to Java's `[-18:00, +18:00]`, retaining historical minute and second offsets within that range. Explicit named and compact suffix recognition is also corrected. This does not add a BE Java alias table. Session alias dispatch is already fixed by #68865. System timezone database differences, ignored external lookup return values and DST rounding order are outside this change. ### Release note Fix inconsistent timezone offset validation in CAST and CONVERT_TZ, reject out-of-range cached session offsets, and correctly recognize explicit timezone suffixes. Numeric TIMESTAMPTZ offsets beyond +/-18:00 are now rejected. ### Check List (For Author) - Test - [x] Regression test - `test_timezone_parser_contract`: generated output through the runner and replayed successfully (1 suite, 0 failures). Compared 55 inputs across DATETIME/TIMESTAMPTZ and five execution paths: 550 results without semantic differences, allowing UTC display-format differences. - Upstream `test_session_time_zone_short_ids`: passed after rebase, covering scans, point queries and stream load. - [x] Unit Test - Earlier focused FE tests: 76 tests, 0 failures/errors, 4 existing skips; BE TimezoneUtilsTest: 5/5 passed. - After rebase: TimeUtilsTest, CoordinatorContextTest and TimeZoneParserTest: 18 tests passed. - [x] Manual test - Rebuilt BE and FE after rebase to master `2d053c83252`; both services report the matching version and Alive=true. ACT session FROM_UNIXTIME on a stored column matches Australia/Darwin without a BE alias table. - Validation limit: the final FE cleanup removing redundant normalization/default arguments and reusing getCanonicalTimeZone has not been rebuilt. The latest regression ran on the preceding build; CI must validate the final source. - clang-format, git diff --check and BE build hygiene passed. The earlier clang-tidy run reported pre-existing modernization diagnostics; it was not a clean clang-tidy pass. - Behavior changed: - [ ] No. - [x] Yes. Offset boundaries and explicit suffix parsing are consistent across the scoped FE/BE paths; invalid endpoints are rejected. - Does this need documentation? - [x] No. This aligns existing offset policies and documents the TIMESTAMPTZ boundary change in the release note. - [ ] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label |
||
|
|
599616cb0d |
[fix](metrics) Escape label values in the Prometheus output of FE (#69837)
### What problem does this PR solve? Issue Number: #69907 Related PR: None Problem Summary: **In short:** the FE writes metric label values into its Prometheus output without escaping them. A table name may contain a backslash, a double quote or a line feed, and in cloud mode the Master FE labels four per-table series with the table name, so a single such table leaves the Master's whole `/metrics` response outside the text format, and Prometheus keeps no sample of a scrape it cannot parse. This PR escapes every label value that is not a literal through one function, as the format requires. Ordinary names are written exactly as before. #### Background In the Prometheus text format a label value is written between double quotes, and a backslash, a double quote or a line feed inside it must be written as `\\`, `\"` and `\n`; no other character is escaped. A scrape whose response does not parse fails as a whole. With `enable_unicode_name_support`, which is on by default, a table name may contain all three characters: `FeNameFormat.UNICODE_TABLE_NAME_REGEX` only requires the last character not to be white space. A local cluster with default settings accepts tables named ``` `a"b` ```, `` `a\b` `` and one with a line feed in the middle. The names of databases, users, workload groups, jobs and catalogs keep their restricted patterns. #### The problem, and what it cost - `PrometheusMetricVisitor` concatenates label values as they are, in every place that writes one: the labels of registered metrics (`visit`), both kinds of histograms, the cloud per-table series (`visitCloudTableStats`), the workload group series and the JVM GC series. - In cloud mode the Master FE exports `doris_fe_table_data_size`, `doris_fe_table_rowset_count`, `doris_fe_table_segment_count` and `doris_fe_table_row_count` for up to `prom_output_table_metrics_limit` (10000) tables, labeled with `db_name` and `table_name`. One table whose name holds one of the three characters makes the response invalid, and every metric of the Master FE is lost while that table is exported. - The coming monitoring of table streams and IVM materialized views labels series with the names of materialized views and streams, which would run into the same problem in both modes. #### How this PR fixes it - `escapeLabelValue` writes a backslash, a double quote and a line feed as `\\`, `\"` and `\n` and leaves every other character alone, which is the full set the format requires. - Every label value that is not a literal goes through it: the two histogram paths and `visit`, the database and table names of the per-table series (escaped once per table, not once per series), the workload group name and the GC name. The JVM GC and workload group names cannot hold these characters today; they go through the same function so that no label value in the class is written raw. - A name without these characters comes back as the same string, so the output of ordinary names does not change, byte for byte, and costs no extra allocation (`String.replace` returns the string itself when nothing matches). - A null label value is written as `null`, as the concatenation without escaping wrote it. Label values can be null: a table warm-up job has no source cluster, so the series of its start and finish times carry a null `src_cluster_name`. #### Results What the cloud per-table series is written as for each table name, and what `promtool check metrics` (Prometheus v3.2.0) says about it: | Table name | Before this PR | promtool | After this PR | promtool | | --- | --- | --- | --- | --- | | `t1` | `table_name="t1"` | OK | `table_name="t1"` | OK | | `a"b` | `table_name="a"b"` | `unexpected end of label value "a"` | `table_name="a\"b"` | OK | | `a\b` | `table_name="a\b"` | `invalid escape sequence '\b'` | `table_name="a\\b"` | OK | | `a\` | `table_name="a\"` | `label value "a\"} 100" contains unescaped new-line` | `table_name="a\\"` | OK | | `a`, line feed, `b` | the sample split over two lines | `label value "a" contains unescaped new-line` | `table_name="a\nb"` | OK | A parser that follows the format turns each escaped value back into the table name itself. #### Classes, and how they call each other - `PrometheusMetricVisitor`: the new private `escapeLabelValue`, called wherever a label value is written. - `MetricsTest`: `testPrometheusVisitorEscapesLabelValues` (a gauge with an escaped and a plain label, and both kinds of histograms), `testCloudTableStatsEscapeTableName` (the four per-table series, with a mocked `CloudTabletStatMgr`) and `testPrometheusVisitorWritesNullLabelValue` (a gauge and a labeled histogram with a null label value). ``` MetricsAction (/metrics) -> MetricRepo.getMetric(PrometheusMetricVisitor) |-- visitJvm GC name -> escapeLabelValue |-- DORIS_METRIC_REGISTER.accept -> visit labels -> escapeLabelValue |-- visitHistograms -> visitHistogram (both) labels -> escapeLabelValue |-- visitCloudTableStats db / table name -> escapeLabelValue `-- visitWorkloadGroup group name -> escapeLabelValue ``` Not changed: the JSON output (`type=json`), which builds its own strings and would need JSON escaping instead; the core output (`type=core`), which writes no labels; the metric names and the `# HELP` texts, which are literals. |
||
|
|
0fe5cc1e31 |
[fix](changelog) Handle disabled TSO in the system table smoke test (#69824)
The system-table smoke test queries every information_schema table. When experimental_enable_feature_changelog is false, querying tso_status intentionally reports "TSO feature is disabled", stopping the suite before the remaining metadata tables are checked. Assert that expected error for the disabled configuration and retain the regular query when enabled. |
||
|
|
f81660bf5f | [chore](github)Update the list of collaborators (#69972) | ||
|
|
c36a40c67b | [fix](regression) Fix test_ttl_lru_evict and test_stale_rowset in cloud file cache (#69847) | ||
|
|
49bfbdecff |
[fix](fe) Avoid table handle lookups for external views (#69946)
### What problem does this PR solve? Related PR: #69836 Selecting an external Iceberg REST view can fail during query binding with `BadRequestException: ... is not an Iceberg compatible table`, even when the catalog supports reading the view definition. `BindRelation` loads statement snapshots before expanding the view SQL. The snapshot loader calls `PluginDrivenExternalTable.resolveConnectorTableHandle`, which unconditionally asks the connector for a table handle using the view name. A REST catalog can reject that table request with HTTP 400 instead of reporting a missing table. The Iceberg connector only converts `NoSuchTableException` to an empty handle, so the error escapes before the existing view-existence fallback can run. Resolve the object's view classification before requesting a table handle. Known views return no table handle and retain the live view-existence check, so a view deleted outside Doris still invalidates its cached lookup. Snapshot loading can then return an empty view pin and continue to view expansion; the referenced base tables keep their normal snapshot handling. Ordinary table handles and table-load errors retain their existing behavior. This addresses the shared handle-resolution boundary, including callers outside the snapshot loader. #69836 proposes skipping the snapshot-loader entry instead. ### Release note Fix external view queries failing when an Iceberg REST catalog rejects table metadata requests for view names. ### Check List (For Author) - Test - [ ] Regression test - [x] Unit Test - [ ] Manual test - [ ] No need to test or manual test. - Behavior changed: - [ ] No. - [x] Yes. External views no longer issue a table-handle request during resolution. - Does this need documentation? - [x] No. - [ ] Yes. Validation: - `run-fe-ut.sh --run` with `PluginDrivenExternalViewHandleTest`, `PluginDrivenExternalTableTest`, `PluginDrivenMvccExternalTableTest`, and `PluginDrivenMissingTableTest`: **164 tests passed**, with no failures, errors, or skips. - Before the fix, both view cases failed with the reproduced table-endpoint error; the ordinary table cases passed. - `mvn checkstyle:check -pl fe-core`: **0 violations**. - `git diff --check` passed. No live Databricks integration test was run. |
||
|
|
44dfa1596f |
[fix](ci) Preserve early review failure notifications and causes (#69469)
### What problem does this PR solve? Related PR: #69386 Problem Summary: When the pending-status API call fails before repository checkout, the failure notification runs outside a Git checkout. Without an explicit repository, `gh pr comment` fails with `fatal: not a git repository`; both the notification and terminal failure step also misidentify the skipped context preparation step as the cause. This occurred in [run 38864234303](https://github.com/apache/doris/actions/runs/38864234303/job/116853272967). Pass `--repo` explicitly and report failed/cancelled pending-status and checkout steps by name. Distinguish actual context failure/cancellation from skipped or unstarted context in both reporting paths, preserving the existing auth/usage-limit recovery messages and failing exit status. |
||
|
|
cb41339ffd |
[fix](ci) Retry transient review GET failures within bounded budgets (#69510)
Related PR: #69469 Problem Summary: A single transient GitHub response currently aborts the review pipeline. [Input resolution for #69322](https://github.com/apache/doris/actions/runs/39794013905/job/119876748768) stopped on a GET 503 despite `Retry-After: 2`; [aggregate resolution for #69438](https://github.com/apache/doris/actions/runs/39760934956/job/119764632659) stopped on a paginated GET 504 after emitting partial output. The shared helper now retries recognized body-free REST GETs on 502/503/504, for at most three attempts. Each attempt has a 30-second limit; attempts and waits share a 90-second budget or the caller's shorter timeout. It respects Retry-After, records every failed attempt, discards failed pagination output, and returns only a successful attempt's complete output. Exhaustion still fails. Writes, GraphQL, unknown argument forms, other HTTP errors, transport/timeouts and JSON parsing errors are not retried. Input resolution can also fail before publishing its PR-number output. Failure notices now fall back to the authorized trigger's numeric PR identity, explicitly select the repository, and name the input-resolution failure in both the notice and terminal error. This fallback does not permit status publication for an unresolved head/base. This change can merge independently. It includes the same explicit `--repo` argument as #69469; a local combination with #69469 and #69504 merged cleanly and passed the macOS suite. |
||
|
|
c31cd7f1cb |
[improvement](be) Read SNII dictionary keys in place in walks and block opens (#69887)
### What problem does this PR solve? Issue Number: None Related PR: #69600 Problem Summary: A regexp without a literal prefix, such as `output MATCH_REGEXP 'transi.*'`, expands by walking every term of the SNII dictionary and testing each key against the pattern. On a 99M-row table, about 40% of the SNII stack samples of that query were spent decoding dictionary entries, twice as much as the pattern match: - `visit_prefix_range` and `visit_term_range` decoded every key into a fresh `DictEntry`: the whole struct reset, a new `std::string`, and a `Status` for every varint. - `DictBlockReader::open` fully decoded every anchor of the block, about 1 entry in 16, only to get its key. Every block open pays for this, exact term lookups included. Both now read an entry's length and front-coded key on the raw cursor. A walk rebuilds the key in one buffer it keeps across entries and a rejected key skips its body by the entry length; `open()` reads only the keys of the anchors. An entry that does not read cleanly goes to the checked decoders, which report what is wrong with it as before, and a walk still decodes the body of every entry it accepts with the checked decoder. The on-disk format and the write path do not change. `open()` no longer decodes the bodies of the anchors. The block's crc32c, verified just before, covers them, and they are checked when they are decoded, like the body of every other entry. | Query (99M rows) | Hot, new/previous | Cold, new/previous | |---|---|---| | regexp `'transi.*'` | 0.69 | 0.77 | | regexp that matches no term | 0.67 | | | regexp `'.*ing'` | 0.72 | | | regexp `'e.*'` (stops at the expansion limit) | 0.90 | | | phrase query, term lookup cache off | 0.78 | | | phrase query, term lookup cache on | 0.99 | 1.01 | | AND of two terms | 0.99 | 1.01 | | the other 22 queries | around 1.0 (reruns of those above 1.05: 0.94-1.03) | | Hot: both versions interleaved in one BE process. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. The build also had #69879 (the term lookup cache) and #69882. With that cache off, which is master today, queries open dictionary blocks more often: the phrase query gains as the 0.78 row shows, and the five other lookup-heavy queries measured that way stayed at 0.96-1.04. |
||
|
|
5d1bdea509 |
[fix](ci) Resume reviews with Astra after five capacity failures (#69953)
### What problem does this PR solve? Reviews can exhaust all seven attempts while the selected model remains at capacity. After five capacity failures, this change resumes the same review session with `gpt-6-astra` for the remaining two attempts, preserving the existing five-minute backoff, shared deadline, target checks, and final-review verification. The workflow opts in through `--capacity-fallback-model`. Account/model admission fallback remains separate. Recovery instructions preserve completed findings and the cumulative round count, and direct unfinished child-agent work to the selected model. Scope: three files; 17 production lines and 91 test lines added. |
||
|
|
62cf8c7588 |
[improvement](be) Read SNII phrase-prefix batches on the calling thread when they hit the page cache (#69882)
### What problem does this PR solve? Issue Number: None Related PR: #69648 Problem Summary: #69648 reads a SNII batch on the calling thread when the index file is local, the batch has at most `snii_inline_read_max_segments` (4) physical segments and 256 KiB, and the moving average of recent reads is under 20 us, i.e. they hit the page cache. A phrase prefix reads the postings of every term its prefix expands to, in batches of 5 to 16 segments that stay well under 256 KiB, so those still went segment by segment through the shared buffered-reader prefetch pool: on a hot 99M-row table, about a fifth of the SNII time of such a query was spent on the pool's lock and latch (`ThreadPool::do_submit`, `CountDownLatch::wait` in `read_batch`). The default of `snii_inline_read_max_segments` becomes 16, which covers those batches, and two changes keep larger inline batches safe when the data is not in the page cache: - A batch read inline hands the segments it has left to the pool as soon as one of its reads is slow. When the average is low but this batch's data is not cached, the batch pays one read on the calling thread and overlaps the rest. A single segment left is still read inline. - A read counts by how much longer it took than copying its bytes out of the page cache (at 4 bytes/ns, the low end for one thread), for both the average and the slow test. A page-cache hit of 64-256 KiB spends tens of microseconds on the copy alone, which pushed the average over 20 us and sent about 5% of a phrase query's small batches to the pool. On the 99M-row table, the phrase prefix's batches of 5-16 segments are now all read inline when hot, and the small batches of a phrase query that still went to the pool dropped by 77%. In a cold run (BE restarted, index files evicted from the page cache), the average was at least 40 us whenever such a batch was decided, so they all went to the pool, as before. | Query (99M rows) | Hot, new/previous rule | Cold, new/previous rule | |---|---|---| | phrase prefix 'retry budget dep' | 0.79 / 0.85 / 0.96 (three runs) | 0.96 | | other phrase queries | 0.87-0.95 | 1.01 (A6) | | the other 19 queries | 0.98-1.03 | | Hot: both rules interleaved in one BE process. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. |
||
|
|
5af2df3231 | [fix](fe) Lock table metadata while constructing UPDATE plans (#69880) | ||
|
|
ac15da94a2 |
[fix](fe) Load Iceberg views without engine-name metadata (#69829)
### What problem does this PR solve?
Allow missing/empty `engine-name` in Iceberg views. Preserve existing
engine-based SQL selection and only use the Doris/SDK fallback when the
field has no value.
### Release note
Support Iceberg views without `engine-name` while retaining existing SQL
selection when it is present.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [x] 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
- Behavior changed:
- [ ] No.
- [x] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
|
|
b2164ff1ba |
[fix](cloud) Avoid lock inversion in delete bitmap checks (#69892)
### What problem does this PR solve?
Issue Number: N/A
Problem Summary:
With background delete bitmap checks enabled, the checker holds the
delete bitmap read lock while `tablet_schema()` acquires the tablet
header lock. Compaction takes these locks in the opposite order, causing
a deadlock on the same tablet. The checker also runs inside
`TabletMap::traverse()`, so its outer map mutex blocks unrelated tablet
initialization, Rowsets scans, synchronization, and compaction
scheduling.
Read the sequence-column flag before traversing the delete bitmap, and
run checks outside the map mutex by reusing the existing tablet
snapshot. Preserve the active/stale rowset version checks for sequence
and non-sequence tables.
### Release note
Fix BE deadlocks and cascading write and metadata stalls when background
delete bitmap checks are enabled.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test
- [ ] No need to test
- Behavior changed:
- [ ] No.
- [x] Yes. Delete bitmap checks avoid the reversed lock order and no
longer hold the tablet map mutex while checking individual tablets.
- Does this need documentation?
- [x] No.
- [ ] Yes.
Validation: 72 ASAN unit tests passed, including five new instances
covering lock ordering, an empty compaction bitmap merge, unrelated
tablet map operations, and sequence/non-sequence version semantics.
`build-support/check-format.sh`, `build-support/check-build-hygiene.sh`,
and `git diff --check` passed.
```bash
./run-be-ut.sh --run --filter='SequenceModes/CloudTabletDeleteBitmapCheckTest.*:CloudTabletMgrTest.*:CloudTabletDeleteBitmapTest.*:PrefillModes/CloudTabletDeleteBitmapPrefillTest.*:TabletMetaTest.*:CloudCompactionTest.*' -j100
```
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
|
|
25a8c10334 |
[improvement](tablet) Source active tablet stats from BE reports (#67621)
Problem Summary: FE tracks tablet activity with per-tablet sliding windows, consuming substantial heap and retaining stale entries on followers and observers. Use BE query and load counters to report bounded sets of active tablets. Replace FE sliding windows and asynchronous recording with per-BE report snapshots, and rank query and load activity separately for cloud balancing. Also fix inflated tablet counts when cloud tablet reports retry. ### Release note Reduce FE memory usage for tablet activity tracking. Rename `WindowAccessCount` to `AccessCount`, reflecting activity over a BE report interval. Tablets outside the reported top-N show zero activity. |
||
|
|
378404eeb1 |
[fix](thirdparty) Upgrade Brotli to 1.0.10 for CVE-2026-111497 (#69906)
### What problem does this PR solve? Issue Number: None Related PR: None Doris bundles Brotli 1.0.9, which is affected by [CVE-2026-111497](https://github.com/google/brotli/releases/tag/v1.0.10). The upstream decoder can mistake an invalid block switch for insufficient input, allowing an out-of-bounds write. Upgrade the bundled dependency to the upstream 1.0.10 security release and update its archive checksum. Align Arrow 24's internal Brotli version and SHA256 pin with the overridden archive; changing the Doris manifest alone causes Arrow's ExternalProject verification to fail. Apply that metadata patch with a separate marker so previously patched Arrow source trees also receive it. The fix requires rebuilding the relevant thirdparty libraries and relinking BE because the decoder is statically linked. ### Release note Upgrade bundled Brotli to 1.0.10 to include the upstream fix for CVE-2026-111497. ### Check List (For Author) - Test: Manual validation passed: - Official source archive MD5 and SHA256 verification; the source contains the upstream block-switch error fix. - Bash syntax and `git diff --check`. - Existing thirdparty download checksum and fallback tests. - Actual Brotli/Arrow companion download and extraction; patch application to previously patched Arrow sources and repeated invocation. - Patch dry-run against pristine Arrow 24 metadata. - Arrow 24's Brotli ExternalProject verification, build, and static-library installation in an isolated CMake harness. Confirmed the old SHA256 pin fails with the new archive before the patch, then passes after it. - Brotli 1.0.10 build and all 14 available upstream round-trip/compatibility tests with AddressSanitizer and UndefinedBehaviorSanitizer. - Full Doris and Arrow builds were not run locally; CI validation is requested. - Behavior changed: Yes. Builds use the patched Brotli decoder and updated Arrow dependency metadata. - Does this need documentation: No. This updates a bundled security dependency. |
||
|
|
112ca45819 |
[fix](ci) Add review findings to one draft before publishing (#69447)
### What problem does this PR solve? Related PR: #68499 The review runner's bulk REST create-review request rejects five valid inline findings on #68499 with HTTP 422 (`Position could not be resolved`; line/side also fails). In a controlled experiment at the same head/base, all five original findings were accepted when added sequentially to **one PENDING review** through GraphQL. The experimental draft was deleted afterward. The exact GitHub internal diff limit is not established. For reviews with inline findings, this change creates one draft, adds one thread per request, verifies the complete comment set, then publishes once. It converts the existing path/position/body input to line/side using the authoritative local PR diff. Summary-only reviews keep their current submission path. The invocation marker, exclusive submission intent, frozen head/base checks, priority/convergence gate, and exact final readback remain required. The helper rechecks competing bot reviews before publication. Add failures trigger best-effort cleanup only of the exactly owned pending draft; uncertain writes are never retried. `review-draft.json` is included in failure artifacts, and GraphQL errors in HTTP 200 responses retain HTTP diagnostics. Ambiguous publication, unavailable cleanup, and hard termination boundaries are documented. Live formal publication and the GitHub Actions identity still need rollout validation. The existing experiment validates draft construction, not end-to-end CI delivery. |
||
|
|
9177f0cbd6 |
[refactor](be) Separate S3 spill file I/O from common readers and writers (#69781)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: `SpillFileWriter` and `SpillFileReader` mixed their common spill-part lifecycle and serialization/deserialization with object-storage-specific policies. The writer included S3 upload queue admission, cancellation checks, timing, and remote accounting; the reader included S3 footer prefetch, adjacent-block coalescing, and remote byte/request accounting. This change introduces `SpillS3FileWriter` and `SpillS3FileReader`, selected by the spill file factory for remote files. The common classes retain part lifecycle, footer format and validation, block serialization/deserialization, part rotation/traversal, and seek handling. Narrow virtual hooks preserve the S3 policies, failed-part reconciliation, query-buffer cap, and local/remote metric separation. The derived writer closes before base destruction so implicit close still dispatches its S3 hooks; reader cleanup remains entirely in the base class and does not require derived hooks. Public writer/reader APIs and the spill file format remain unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test / Build and static checks - `BUILD_TYPE=ASAN ./build.sh --be -j16` passed on the updated reader/writer head. - `./run-be-ut.sh -j16 --run --filter='SpillFileS3Test.*:SpillFileTest.*:SpillRepartitionerTest.*:SpillSortSourceOperatorTest.*'` passed (109 tests from 4 suites). - New coverage checks S3 reader factory selection and empty-file handling, the query-buffer cap on coalesced GET lengths, and local reader type/accounting isolation. Existing suites exercise multi-part roundtrips, seeking, footer overflow, oversized blocks, and disabled coalescing. - BE formatting/check-format, build hygiene, and `git diff --check` passed. - Changed-line clang-tidy passed for all 7 files in the reader follow-up using the UT compilation database and the Clang resource directory. - S3 unit tests use an in-memory object-storage mock. SQL regression and real-object-store E2E were not rerun for the reader follow-up; these results are not remote CI results. - Behavior changed: No; this is an internal responsibility split preserving the existing local and S3 spill behavior. - Does this need documentation: No |
||
|
|
efa20d2b83 |
[improvement](be) Accelerate relative-day weekday lookup (#69897)
### What problem does this PR solve?
`next_day` and `previous_day` copy and uppercase the weekday string and
search an `unordered_map` for every row, including repeated constant
weekday arguments.
Use a specialized, compile-time-checked 32-slot table for the 21
supported English aliases. Fold ASCII case during bounded word
comparisons rather than materializing a normalized string. Verify the
complete key after selecting a slot, so arbitrary input that collides
with an alias is rejected. Parse constant weekday arguments once per
block, preserving NULL propagation before invalid-weekday errors.
Keep the parser and the shared `FunctionRelativeDay` implementation in
the anonymous namespace of `function_date_or_datetime_computation.cpp`,
their only consumer. Both registered functions reuse the same template.
No standalone parser source/header, new dependency or compiler
adaptation remains. The SQL signatures, return types, accepted aliases
and error behavior are unchanged.
#### Performance: original implementation vs specialized implementation
**The overall before/after comparison is the original `unordered_map`
implementation with per-row parsing versus the specialized weekday
lookup and once-per-block constant parsing delivered by this PR.**
End-to-end SQL improves by **1.68–2.59x**, and the complete registered
functions improve by **1.89–2.85x** in the saved A/B measurements below.
**Measurement provenance:** these are the saved original feature-branch
A/B results, not a new run on the latest PR head. The final revision
retains the original specialized parser and function execution logic,
with the implementation now local to the existing function CPP. The SQL
A/B comparison was not rerun after the cherry-pick and final relocation.
Current-head functional/build validation is listed separately below.
Both variants were measured on an x86_64 Intel Xeon Platinum 8457C with
Clang 21.1.8, C++20, RELEASE/AVX2. The nullable scenarios have 25% NULL
dates. These are synthetic local workloads; ARM performance was not
measured.
**End-to-end SQL — milliseconds, lower is better.** One BE, one
pipeline, one bucket, 10 million materialized rows; the same FE, data
and settings for both variants, SQL/query caches disabled, two warmups
followed by seven measurements, reporting the median.
| Function | Scenario | Original (ms) | Specialized (ms) | Speedup |
|---|---|---:|---:|---:|
| `next_day` | Varying weekday | 528.7 | 285.1 | 1.85x |
| `next_day` | Constant weekday | 664.7 | 256.5 | 2.59x |
| `next_day` | Nullable date | 441.6 | 256.2 | 1.72x |
| `previous_day` | Varying weekday | 524.1 | 279.7 | 1.87x |
| `previous_day` | Constant weekday | 649.6 | 254.4 | 2.55x |
| `previous_day` | Nullable date | 435.1 | 258.6 | 1.68x |
All six SQL result checksums match.
**Complete registered-function benchmark — nanoseconds per input row,
lower is better.** Batches of 4096 rows, seven repetitions, CPU-time
medians. Includes function execution, result allocation, NULL handling
and date calculation.
| Function | Scenario | Original (ns/row) | Specialized (ns/row) |
Speedup |
|---|---|---:|---:|---:|
| `next_day` | Varying weekday | 49.06 | 23.86 | 2.06x |
| `next_day` | Constant weekday | 59.76 | 21.53 | 2.78x |
| `next_day` | Nullable date | 35.57 | 18.84 | 1.89x |
| `previous_day` | Varying weekday | 45.86 | 22.66 | 2.02x |
| `previous_day` | Constant weekday | 59.44 | 20.85 | 2.85x |
| `previous_day` | Nullable date | 35.54 | 18.22 | 1.95x |
**Isolated weekday parsing — nanoseconds per lookup, lower is better.**
Measures only string-to-weekday parsing, excluding date calculation,
result columns and query execution.
| Input | Original (ns/lookup) | Specialized (ns/lookup) | Speedup |
|---|---:|---:|---:|
| Valid aliases | 40.51 | 2.66 | 15.24x |
| Invalid names | 38.83 | 1.81 | 21.47x |
Parser-only speedups must not be interpreted as complete-function or SQL
speedups. Constant-weekday results include both faster parsing and
eliminating repeated parsing within the block.
### Release note
Improve `next_day` and `previous_day` weekday lookup and avoid repeated
parsing of constant weekday arguments while preserving SQL results and
error behavior.
### Check List (For Author)
- Test
- [x] Unit Test: seven focused ASan tests via `run-be-ut.sh`, covering
every ASCII case combination of all aliases, arbitrary high bytes,
embedded NULs, invalid names, constant/nullable arguments, NULL/error
precedence and timestamp precision. Parser tests exercise the registered
SQL functions.
- [x] Manual test: standard RELEASE BE and focused benchmark builds;
full-function benchmarks; 123210 ASan/UBSan oracle cases against an
exact extraction of the CPP-local helper, including exact-length
allocations and guard pages.
- [x] clang-format 16.0.6, production/test clang-tidy, whitespace, shell
syntax and build hygiene.
- ASan unit tests use `GLIBC_COMPATIBILITY=OFF` for existing toolchain
duplicate compatibility symbols; the production build keeps
compatibility enabled.
- Existing SQL regression cases and runner-generated output remain in
the PR. They passed in the original dedicated-parser work; the full SQL
regression suite was not rerun for this revision.
- Behavior changed:
- [x] No.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
|
|
49da8785c8 |
[refactor](scan) Remove legacy file scanner and deprecate rollout option (#69723)
### What problem does this PR solve?
File scans maintained both the legacy scanner/readers and the V2 reader
stack. Route query scans, schema inference, file loads and external
row-ID fetches through FileScannerV2, migrate standalone Push and
Iceberg delete-file consumers, and remove the retired implementation and
tests.
- Keep enable_file_scanner_v2 as a deprecated no-op, with the existing
optional Thrift field emitted/defaulted to true.
- Preserve ORC session/adaptive batch caps, stripe dictionary rejection,
lazy predicate-column reuse and sequential CSV/TEXT/JSON S3 prefetch.
- Filter matching ORC dictionary IDs before string expansion in lazy,
eager and predicate-only scans. Retain residual predicates, NULL
semantics, pruning fences, physical column identity and physical row
positions.
- Restore ORC serialized-tail caching using the existing V1
FileMetaCache key policy, including its file-size fallback when mtime is
absent. Keep V2 payload types separate. ORC footer counters have
format-qualified names and publish once across repeated close; native
Parquet cache identity remains unchanged.
- Bound standalone position-delete reads to the native Parquet batch
limit. Broker source projections follow validated src_slot_ids order,
and malformed ORC dictionary offsets return CORRUPTION.
- Retain eight active V2 position-delete system-table regressions and
the operator shared-cache/shard-limit tests in maintained suites.
- Preserve all six Hive nested STRUCT/ARRAY/MAP access-path scenarios in
V2 across ORC/Parquet and name/position mapping, including separate
predicate-column IDs and complete-versus-projected row comparisons.
- Preserve active-file error context and the upstream Arrow column/type
context without changing original validation status codes.
Related PR: #69668
The supported rolling-upgrade direction remains BE first, then FE.
Old-FE/new-BE requests without semantic COUNT arguments retain their
ordinary-read fallback. New-FE/old-BE nullable COUNT is outside that
documented upgrade direction; the deprecated scanner option does not
provide a legacy routing gate.
The branch resolves the modify/delete conflict with master at
|
||
|
|
25968c92bf |
[test](fe) Stabilize scan termination and retry profile assertions (#69894)
### What problem does this PR solve? Issue Number: N/A Problem Summary: Two FE unit tests depend on asynchronous work without owning its complete lifecycle: - `SplitProducerTest.readThatOutlivesItsStatementLoadsNothingIntoTheClosedScope` waits for the source-close signal and immediately checks the producer completion count. `SplitProducer.finish` invokes the completion callback after closing the source, so the assertion can run before that callback. Wait for the assignment termination future, which completes after both cleanup and the callback. - `StmtExecutorTest.testSuccessfulRetryRendersMergedProfileAndActualRows` registers its profile with the global background `ProfileManager` while also writing and reading the profile synchronously. Use a real manager whose daemon is not started, scoped to this test, so automatic spill, memory release and archival cannot interfere with the temporary storage lifecycle. Preserve the real persistence path and all merged-profile and actual-row assertions, and include the readback content in the failing assertion. The tests originated in #69718 and #68658 respectively. These test-only changes are independent of the Coordinator refactoring in #69830. The exact CI interleaving was not reproduced locally; both original tests also passed local repeated execution. No production behavior changes. ### Release note None ### Check List (For Author) - Test: On this independent master-based branch, `./run-fe-ut.sh --run 'SplitProducerTest,StmtExecutorTest'` passed all 81 cases (26 + 55). Before extraction, both target cases also passed 50 repetitions each (179 executions across both classes); FE Checkstyle passed with 0 violations. - Behavior changed: No - Does this need documentation: No |
||
|
|
a9253e979d |
[fix](iceberg) Decode Parquet timestamptz columns as instants by the Iceberg table type (#69845)
### What problem does this PR solve? Issue Number: None Related PR: #69711 Problem Summary: **In short.** An Iceberg `timestamptz` column stored as Parquet INT96, the encoding Spark and Hive use for timestamps, reads shifted by the session time zone's offset in any non-UTC session: in Asia/Shanghai, 2024-01-01T00:00:00Z reads as `2024-01-01 00:00:00+08:00`, eight hours early, while Iceberg itself reads `2024-01-01 08:00:00+08:00`. Equality deletes on such a key miss their rows for the same reason. This PR has the FE mark each `timestamptz` field of the scan's schema dictionary, and FileScannerV2 then decodes the matching Parquet columns of data files and equality-delete files as instants, which is how Iceberg reads them. It came up in the review of #69711. **Background.** - Parquet INT96 has no logical annotation, so a reader cannot tell from the file whether an INT96 column holds an instant or a wall clock. Spark writes its `timestamp` type (an instant) as INT96 by default, and `add_files`, `migrate` and `snapshot` attach such files to Iceberg tables unchanged, as `timestamptz` columns. - Iceberg does not look at the file annotation at all: its Parquet readers choose the timestamp reader from the table type, so a `timestamptz` field reads INT96, and INT64 with or without the UTC annotation, as an instant. - The FE sends `parquet_timestamp_semantics_version = 1` for every file scan. Parquet scans therefore always use FileScannerV2, and INT96 decodes as a UTC wall clock (`DATETIMEV2`) unless the table format supplies the semantics. Paimon has the same ambiguity (TIMESTAMP and TIMESTAMP_LTZ share INT96) and solves it with `timestamp_is_adjusted_to_utc` on each field of its schema dictionary, which `PaimonReader` applies to the file schema. The Iceberg FE never set that field. **The problem, and what it cost.** For a `timestamptz` column, the projection casts the INT96 wall clock to `TIMESTAMPTZ` in the session time zone, so the instant moves by the session offset. Reproduced on master with an INT96 file attached like `add_files` and an Iceberg-written INT64 file holding the same instants, both in one table: | Session | Rows | Iceberg Java | Before | After | |---|---|---|---|---| | UTC | INT96 and INT64 | 2024-01-01 00:00:00 UTC | correct | correct | | Asia/Shanghai | INT64 (written by Iceberg) | `2024-01-01 08:00:00+08:00` | correct | correct | | Asia/Shanghai | INT96 (written by Spark) | `2024-01-01 08:00:00+08:00` | `2024-01-01 00:00:00+08:00`, 8 hours early | `2024-01-01 08:00:00+08:00` | The result is the same with field ids in the file, with only the table's name mapping, whatever the catalog's `enable.mapping.timestamp_tz` says (master always maps `timestamptz` to TIMESTAMPTZ), and with `enable_file_scanner_v2` on or off. The same shift breaks equality deletes, which compare the projected values: | Equality delete on a `timestamptz` key, non-UTC session | Before | After | |---|---|---| | INT96 data key, Iceberg-written tombstone of the same instant | the tombstone misses the row | the row is deleted | Wrong values and missed or extra deletes come back without any error. **How this PR fixes it.** - FE: `IcebergSchemaUtils.buildField` sets `timestamp_is_adjusted_to_utc = true` on every `timestamptz` field, at every nesting level; the dictionary also carries the hidden equality-delete keys. `timestamp` fields stay unmarked: marking them `false` would make BE read the Parquet statistics of an adjusted-UTC INT64 file as UTC wall clocks while its rows still decode as session wall clocks, so row-group pruning could drop matching rows. - BE, data files: `IcebergTableReader::annotate_file_schema` copies the marker onto the Parquet file column that `TableColumnMapper` will select for each field, using the mapper's own matchers (`find_column_by_field_id` / `find_column_by_name`) after the positional fallback ids are stamped, so field ids, the table's name mapping and positional ids all work. Like the mapper, it accepts an ID-less wrapper at every depth when `allow_idless_complex_wrapper_projection` is on. It then rewrites the column type with the helper `PaimonReader` already used, moved to `schema_history_util` as `apply_file_timestamp_semantics`. The mapping, the scan request and `ParquetReader` all see `TIMESTAMPTZ`, and INT96 decodes as an instant. Only a `DATETIMEV2` or `TIMESTAMPTZ` leaf takes the marker; a `TIMESTAMP_NS` leaf keeps its nanoseconds. Columns without a marked leaf keep the types the file reader reported, and a plan from an older FE carries no marker and reads as before. - BE, equality deletes: three kinds of column are read without a `ColumnMapping`: a hidden key column, the parent struct that is read for the NULLs of a nested key the data file lacks, and the key columns of an equality-delete file. Their projections now carry the markers of the annotated file column (`equality_delete_root_projection`, built on `attach_timestamp_semantics`, which the mapper now exports). Without them Parquet would decode an INT96 leaf as `DATETIMEV2` under a `TIMESTAMPTZ` file schema, and the read would fail its type check. That holds even when the query projects the column, because reading it as a predicate column replaces the projection its mapping marked. Equality-delete files get the same annotation, so tombstones and data keys of a `timestamptz` field are both instants. The `DATETIMEV2` / `TIMESTAMPTZ` comparison rule from #69711 stays for plans from older FEs. - Refactor: `find_current_schema` and `build_schema_identity_from_external_field` (now also carrying the marker) move to `schema_history_util`; `TableReader`'s four copies of the current-schema lookup use the helper. **Results.** - FE unit test `IcebergSchemaUtilsTest#timestamptzFieldsCarryInstantMarkerAtEveryLevel`: top-level, struct, list and map `timestamptz` fields are marked; `timestamp` and `timestamp_ns` fields are not, under both values of the mapping flag. - 5 new BE unit tests in `iceberg_reader_test.cpp`, all failing without the BE change: the file-schema annotation (field ids at every level, `TIMESTAMP_NS` untouched, ORC untouched); INT96 `timestamptz` in Asia/Shanghai with field ids, name mapping and positional ids, with and without the marker; INT96 inside a struct and a list; an INT96 equality key against an Iceberg tombstone, hidden or projected; and the DST fall-back above. Leaving the delete files unannotated fails the DST case (the second tombstone deletes the first row); projecting a hidden key or a tombstone without its marker fails both equality-delete cases with a read error; marking a `TIMESTAMP_NS` leaf fails the annotation case. With neither data nor delete files annotated (the reader before this PR), the INT96 key deletes nothing and the DST case deletes both rows. The existing case for an older FE's plan (no marker) still passes. - 2 more BE unit tests from review: - INT96 under a nested ID-less wrapper struct, read and used as an equality key. It fails when the walk accepts the wrapper only at the root. - A nested equality key missing from an older file whose parent struct holds an INT96 `timestamptz`. It checks the survivors with the parent hidden and projected, that the projected timestamp reads as an instant in Asia/Shanghai, and a plan without the marker. It fails with the reader's type check when the parent is read without the marker. - End to end on a local cluster: an Iceberg table holding an INT96 Parquet file attached like `add_files` (one table with field ids in the file, one relying on `schema.name-mapping.default`) and an Iceberg-written INT64 file with the same instants, read through three catalogs (`enable.mapping.timestamp_tz` unset, `true`, `false`), with `enable_file_scanner_v2` on and off, in UTC and Asia/Shanghai: 24 combinations. Before this PR the INT96 rows were 8 hours early in every Asia/Shanghai combination (8 of the 16 measured); with it the INT96 rows equal the INT64 rows in all 24, and the INT64 rows and the UTC results are unchanged. branch-4.1 and branch-4.2 have the same code. **Classes, and how they call each other.** - `IcebergSchemaUtils` (FE): builds the schema dictionary; now marks `timestamptz` fields. - `schema_history_util`: `find_current_schema`, `build_schema_identity_from_external_field`, and `apply_file_timestamp_semantics` (moved from `PaimonReader`). - `IcebergTableReader`: `annotate_file_schema` → `_apply_positional_id_fallback` (the existing logic) and `_apply_table_timestamp_semantics`; `_append_equality_delete_predicates` (hidden keys), `_build_missing_equality_delete_key_expr` (the parent of a missing nested key) and `_load_equality_delete_file` (tombstones) project their columns with the markers through `equality_delete_root_projection`. - `column_mapper`: `attach_timestamp_semantics` is exported. ``` FE IcebergSchemaUtils.buildField --timestamptz--> TField.timestamp_is_adjusted_to_utc = true (history_schema_info, current_schema_id = -1) BE TableReader::open_reader +-- IcebergTableReader::annotate_file_schema (Parquet only) | +-- _apply_positional_id_fallback | +-- _apply_table_timestamp_semantics | +-- find_current_schema, build_schema_identity_from_external_field | +-- find_column_by_field_id | find_column_by_name (the mapper's matchers) | +-- apply_file_timestamp_semantics (shared with PaimonReader) +-- TableColumnMapper: mapping/projection take the marker from the file column +-- ParquetReader::open: apply_projection_timestamp_semantics -> INT96 decodes as TIMESTAMPTZ IcebergTableReader::_append_equality_delete_predicates +-- equality_delete_root_projection -> attach_timestamp_semantics (hidden key) +-- _build_missing_equality_delete_key_expr +-- equality_delete_root_projection (parent of a missing key) IcebergTableReader::_load_equality_delete_file -> _apply_table_timestamp_semantics + equality_delete_root_projection (tombstones) ``` ORC files are not changed: ORC's file type already separates `TIMESTAMP` from `TIMESTAMP_INSTANT`. ### Release note Fix Iceberg `timestamptz` columns stored as Parquet INT96 (written by Spark or Hive and attached with `add_files`, `migrate` or `snapshot`) reading shifted by the session time zone's offset in non-UTC sessions, and equality deletes on such keys missing their rows. ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [x] Unit Test - BE: `run-be-ut.sh --run --filter='IcebergV2ReaderTest.*:Paimon*ReaderTest.*:Hudi*Test.*:TableReaderTest.*:ColumnMapper*:FlussUnionLakeReaderTest.*:EqualityDeletePredicateTest.*:*PositionDelete*'`: 532 of 532 pass on macOS arm64 (ASAN), including the 5 new cases. After the first review round, `IcebergV2ReaderTest` passes 122 of 122, including the new nested ID-less wrapper case. It fails when the walk accepts the wrapper only at the root. After the second review round, the whole filter passes 534 of 534 and `IcebergV2ReaderTest` 123 of 123, including the new missing nested key case. It fails when the missing key's parent is read without the marker. They ran on a local base; the commit was then applied to master `2a4191c1dc9` without conflicts, and the changed sources and their test files were syntax-checked against that tree with the UT build flags. - FE: `mvn -pl :fe-connector-iceberg -am package -Dtest=IcebergSchemaUtilsTest`: 35 of 35 pass, checkstyle passes. - [x] Manual test (add detailed scripts or steps below) - The local end-to-end check under Results, with an ASAN BE and the Iceberg connector built from this change. - [ ] No need to test or manual test. Explain why: - Behavior changed: - [ ] No. - [x] Yes. Parquet columns of Iceberg `timestamptz` fields decode as instants by the table type, as Iceberg reads them: INT96 no longer shifts by the session offset, an INT64 timestamp without the UTC annotation under a `timestamptz` field reads its value as UTC, and equality deletes on `timestamptz` keys compare instants. Plans from an older FE are unchanged. - Does this need documentation? - [x] No. - [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 --> ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
c4b11b53e1 |
[fix](be) Keep BM25 statistics per index for MATCH on one column (#69901)
### What problem does this PR solve? Issue Number: None Related PR: #69790 Problem Summary: score() over MATCH clauses that read different inverted indexes of one column fails: INDEX idx_english(name) USING INVERTED PROPERTIES("parser" = "english", ...), INDEX idx_standard(name) USING INVERTED PROPERTIES("parser" = "standard", ...) SELECT id, score() FROM t WHERE name MATCH 'apple' USING ANALYZER english AND name MATCH 'apple' USING ANALYZER standard ORDER BY score() DESC LIMIT 10; -- Scoring predicates for one field select different inverted indexes BM25 statistics are keyed by field (column unique id plus suffix path), but each index of a column holds its own terms and lengths: over Apple and apple, an index that folds case has df(apple) = 2 and one that does not has df(apple) = 1. One entry cannot serve both, so the collector refuses the query. Fix: a MATCH keeps its statistics under the field and the index id. CollectInfo records the field as indexed for opening the index, since the map key no longer is one. The readers bind their source to the real field and pass the statistics name to run_leaf, so the scorers and similarities need no change: they already look statistics up by the name they are given. Each clause now scores with the statistics of its own index, and the combined score is the sum of what each clause scores alone. SEARCH keeps its own statistics names and its rejection of two indexes on one field; only MATCH changes. A MATCH and a SEARCH on one index keep separate entries with the same values. |
||
|
|
eb16c4d81f |
[improvement](be) Keep hot SNII term lookups across queries (#69879)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A SNII dictionary larger than the resident threshold (256 KB) is read block by block on demand. Every exact term lookup reads the term's DICT block (64 KB target), decompresses it with zstd, checks its CRC and parses its anchors, and the request-scoped DictBlockCache keeps nothing across queries. Every query therefore pays this again for every term in every segment. On a 99M-row AgentLogs table with 722 segments, counting one frequent term spent 68 ms of index time on these lookups against 16 ms for V3, and stack samples of postings-only and phrase queries showed 7-14% of SNII time in decompressing dictionary blocks alone. A SNII reader that the searcher cache keeps now keeps its recent exact lookups as well: a bounded LRU of up to 32 terms per reader, each at most 64 bytes long with at most 256 bytes of inline postings, which also remembers terms its dictionary turned out not to hold. `lookup()` and `lookup_batch()` check it before they read a block, so a batch reads only the blocks of the terms it does not hold. The cache's full footprint, about 25 KB, is reserved in the reader's searcher cache charge when the reader is inserted, as norms are, so the memory stays accounted. Only readers kept by the searcher cache enable it, and only when their dictionary is read on demand. The entries live inside the reader, so they go away with it and never outlive the file they describe. Caching the decoded blocks instead was not chosen: an exact lookup touches one 64 KB block per segment, so blocks would cost about 46 MB per hot term and column on that table, while a kept lookup costs a few hundred bytes. Keeping blocks in the searcher cache would also count against its fd-derived entry limit and push readers out. Prefix and pattern walks over the dictionary are not covered. Hot latency on the 99M-row table, cache off/on interleaved in one BE process: | Query | Off -> On | |---|---| | phrase 'retry budget depleted' (DS2) | 0.60 | | AND of two terms in two columns (A4) | 0.74-0.79 | | one term in two columns (A3) | 0.80 | | count of one frequent term (A2) | 0.84 | | regexp, its matched terms looked up again (DS9) | 0.89 | | phrase prefix (DS4) | 0.93 | | the other 19 queries | 0.92-1.05, within run-to-run noise | Against V3 on the same data (SNII/V3), DS2 went from 1.29 to 0.81, A4 from 1.24 to 0.95, A3 from 1.21 to 1.01 and A2 from 1.07 to 0.97. |
||
|
|
12f49adad2 |
[improvement](be) Add sorted docids to roaring bitmaps a container at a time (#69895)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: An equality filter on an indexed string column, such as `tenant = 'x'`, collects its whole posting list into a roaring bitmap, and a value can cover tens of percent of the table. `RoaringDocIdSink::append_sorted` gets those documents in sorted windows that mostly fall in one 64K container, but `Roaring::addMany` goes through the container's type for every document. On a 99M-row table that was about 20% of the SNII stack samples of a filtered phrase prefix query. `append_sorted` now splits a span at container boundaries and fills each container directly: - A bitset container gets one write per 64-bit word, with the bits gathered in a register, because neighbouring documents mostly share a word and a store per document would wait on the one before it. - An array container gets its tail appended in one loop, up to its limit of 4096. The next document turns it into a bitset, which then takes the rest. - Anything else goes through `addMany` as before: run containers, documents that fall inside an existing array, and spans that are not strictly ascending, which the sink does not rule out and a damaged posting could produce. The containers end up the same as with `addMany`. Two things to look at in review: - The sink is shared by every inverted index format, so V3 queries take the same path. - The code writes to CRoaring's container structs. `null_bitmap.cpp` already includes those headers, but only reads their sizes, so an upgrade of CRoaring has to keep this code in mind. CRoaring 2.1.2 has no public call that adds a sorted run to a container. | Query (99M rows), new/previous | SNII hot | V3 hot | SNII cold | |---|---|---|---| | phrase with tenant, app, type and status | 0.80 / 0.80 / 0.78 | 0.90 | 0.95 | | phrase with a latency filter | 0.78 / 0.77 / 0.78 | 0.78 | | | MATCH_ALL with the same four filters | 0.79 / 0.77 / 0.81 | 0.95 | 0.98 | | filters and a latency range, no text | 0.83 / 0.87 / 0.88 | 0.80 | | | phrase prefix with tenant and app | 0.93 / 0.92 / 0.92 | 0.93 | 0.98 | | top-50 with tenant and payload equalities | 0.77 | 0.82 | | | top-50 tool failures | 0.93 | 0.77 | | | the other 18 queries | 0.89-1.04 | 0.91-1.04 | | Hot: both versions interleaved in one BE process, three runs where three are shown, on a host with a load of about 100 to 140 from other builds. An earlier round at a load of 230 to 400 read up to 1.41 on some queries that do not gain; their reruns gave 0.95-1.07. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each; the change only saves CPU, so cold runs stay about level. The build also had #69879, #69882, #69887 and #69891, with `snii_inline_read_max_segments` at 16. |
||
|
|
4c1e0ddfe2 |
[improvement](be) Read and collect SNII BKD range hits in bulk (#69908)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A range filter on a numeric SNII index reads the leaves that lie wholly inside the range one at a time, gathers their doc ids, radix-sorts them and adds them to a roaring bitmap. On a 99M-row table, 71% of the SNII stack samples of a query with a wide `latency_ms` range were in this path: 15% in the sort, 9% in `addMany`, and about 27% in reading, mostly per-call overhead, since a leaf is a few KB and takes one positioned read of its own. - Whole-leaf hits lie next to each other in `bkd_data`, so one read now takes up to about 1 MiB of them. The bytes read are the same, in far fewer calls, and sequentially when they are not cached. - A wide range sets the doc ids in a flat bitmap instead, which needs no sort. It is loaded into a Roaring through the portable format, a published spec, because CRoaring 2.1.2 has no call that takes a bitset. - A flat bitmap costs a pass over all its words, so it takes doc ids only up to 256 per point, which holds it to 32 bytes per point. The doc ids decide, not the index's document count, which leaves out rows with no value: a narrow range, or a range on a column that is mostly NULL, keeps the sort. - The bitmap also stops at doc id 2^28, which is 32 MiB. The doc ids of a leaf past that take the sort as before, so a damaged leaf cannot make it allocate more. - The leaf decoder reuses its vector of value runs instead of allocating one per leaf. The on-disk format and the write path do not change. | Query (99M rows), new/previous | Hot | Cold | |---|---|---| | filters and `latency_ms BETWEEN 2000 AND 6000`, no text | 0.42 / 0.39 / 0.37 (212 ms to 85 ms) | 0.87 | | phrase with `latency_ms >= 3000` | 0.62 / 0.61 / 0.58 | 1.00 | | phrase with `latency_ms >= 8000` | 0.90 / 0.89 / 0.87 | 1.00 | | phrase with a 7-day date range | 1.01 / 0.98 / 1.00 | | Hot: both versions interleaved in one BE process on a quiet host, three runs each. The 21 queries of the bench with no range do not reach this code and read 0.86-1.02 in one run, which is the noise of that run. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each; the wide range gains and the others wait on other reads. All 25 queries return the same results with both versions. The build also had #69879, #69882, #69887, #69891 and #69895. A wide range briefly holds about three times its result in memory: the flat bitmap, the portable image and the bitmap loaded from it. That is one bit per row of the segment each, and it is freed when the range ends. The chunks of gathered doc ids it replaces took 8 MiB at most. Like those chunks, the bitmap and the image are plain vectors that the memory tracker does not see. |
||
|
|
050698f414 |
[improvement](be) Add up SNII position counts a run at a time in selective decode (#69891)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: A phrase query decodes the positions of its candidate documents a window at a time. In a PFOR window the position counts of all the documents come first, and they are decoded to find where the positions of a selected document start, however few documents are selected. `decode_selected_pfor_count_ranges` went through every count of the window, with the running checks and a test for the next selected document at each one. On a 99M-row table that loop was 16% of the SNII stack samples of a phrase prefix query, nearly all of it in the loop rather than in the PFOR decode, because such a query selects no document in most runs. A run of counts is now added up whole, with no branch per document, so the compiler can vectorize it, and then walked from one selected document to the next, adding up only the counts in between. The sum of a run is kept in 64 bits and checked against the cap before any of its documents is used, so a window above the cap is rejected with the same error. The on-disk format and the write path do not change, and neither do the positions or the window shape returned. | Query (99M rows) | Hot, new/previous | Cold, new/previous | |---|---|---| | phrase prefix `'retry budget dep'` | 0.96 / 0.94 / 0.95 / 0.95 (four runs) | 0.99 | | phrase with a latency filter | 0.97 / 0.94 / 0.97 / 0.98 | 0.99 | | phrase `'unable to open'` in input or output | 0.98 / 0.96 / 0.98 / 0.99 | 0.92 | | the other 22 queries | around 1.0, no trend (0.90-1.06) | | In stack samples of the phrase prefix, with the same binary and both versions switched at run time, `decode_selected_pfor_count_ranges` fell from 16.1% to 3.4% of the SNII samples. Hot: both versions interleaved in one BE process, on a host with a load of about 100 to 150 from other builds, so differences of a few percent in a single run are noise. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 5 runs each. The change only saves CPU, so cold runs, which mostly wait for reads, stay level as expected. The build also had #69879, #69882 and #69887, with `snii_inline_read_max_segments` at 16. |
||
|
|
1106a37488 |
[improvement](be) Cut per-document work in SNII docid decode (#69909)
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: An equality filter reads a posting list of millions of documents through `decode_window_prefix_plan`, so small costs per document, per PFOR run or per window add up. In the stack samples of a filtered phrase prefix query on a 99M-row table: - The window loop cleared its vector, so every window zero-filled it again before decoding wrote over it (4% of the SNII samples). - The sum of the docid deltas checked for a zero delta and for overflow at every document (about 7%, with the sum itself). - Each PFOR run read its exception count, and each exception its two fields, through the out-of-line varint decoders, although they are nearly always one byte (about 5%). The window loop now decodes over the vector, so a resize zero-fills only what it adds. A window's deltas are checked whole, with no branch per document, before they are summed: the same checks, and the same error code when one fails. If a window breaks more than one of them, the zero delta is reported first, whichever came first in the window. `pfor_decode` and `pfor_skip` use `get_varint32_fast`, which handles a one-byte value inline and leaves anything else to the checked decoder. The on-disk format and the write path do not change. | Query (99M rows), new/previous | Hot | Lowest run, new/previous | Cold | |---|---|---|---| | filtered phrase prefix | 0.94 / 0.96 / 0.86 | 0.95 / 0.93 / 0.93 | 0.98 | | phrase with tenant, app, type and status | 0.92 / 0.92 / 0.98 | 0.92 / 0.95 / 0.95 | 0.95 | | phrase with a latency filter | 0.95 / 0.91 / 0.99 | 0.94 / 0.92 / 0.96 | | | MATCH_ALL with the same four filters | 0.94 / 0.92 / 1.10 | 0.94 / 0.93 / 0.93 | | | top-50 with tenant and payload equalities | 0.94 / 0.94 / 0.95 | 0.95 / 0.94 / 0.99 | 1.00 | | the other queries | 0.93-1.03 | | | Hot: both versions interleaved in one BE process on a quiet host, three runs each. Cold: BE restarted and the index files evicted before every run, configuration order alternated, 8 runs for the phrase prefix, 5 for the others. The build also had #69879, #69882, #69887, #69891 and #69895. |
||
|
|
347b9a03cf |
[test](be) Set the thread counts in TimeSharingTaskExecutorTest.test_invalid_task_handle_returns_error (#69915)
### What problem does this PR solve? Issue Number: None Related PR: #65054 Problem Summary: **In short:** `TimeSharingTaskExecutorTest.test_invalid_task_handle_returns_error` builds its executor from a `ThreadConfig` whose thread counts are never set, so BE UT runs intermittently die in it with a core dump. This PR sets the two counts in the test. **Background:** `TimeSharingTaskExecutor::ThreadConfig::max_thread_num` and `min_thread_num` are plain `int` fields without a default value. The other ten `ThreadConfig`s in `time_sharing_task_executor_test.cpp` set both; the test added by #65054 sets only `thread_name` and `workload_group` before calling `init()`. **The problem, and what it cost:** `init()` sizes the thread pool from whatever the stack holds. When that value is large, it keeps creating threads until mmap fails: ``` ==38657==ERROR: AddressSanitizer failed to allocate 0xfc000 (1032192) bytes at address 0x0f63acfd7000 (errno: 12) ==38657==ERROR: AddressSanitizer: SEGV on unknown address 0x0ab44632aa9e ... ``` TeamCity then reports `Build stopped: CORE DUMPED`, and no suite scheduled after this test runs, so the run says nothing about the PR's own BE tests that come later. Between 2026-10-24 and 10-31 this happened in 4 of 11 sampled BE UT runs, on four different PRs (build 1087538 on #69581 is one); a rerun passes. **How this PR fixes it:** set `max_thread_num` and `min_thread_num` to 0, as `test_remove_task_clears_queued_task_count` in the same file does. The test only calls `init()` and checks that null and foreign task handles are rejected, so it needs no worker thread. Test-only change; the executor is unchanged. **Results:** | | Before | After | |---|---|---| | Thread counts of the test's executor | whatever the stack holds | 0 / 0 | | BE UT | aborts in this test in some runs (4 of 11 sampled) | the test no longer depends on stack contents | Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
|
8e10b235cf | [doc](regression) Translate the Iceberg write P0 coverage matrix into English (#69514) | ||
|
|
f521ebfe4e |
[test](fe) Cover format=avro table functions failing during analysis (#69849)
### What problem does this PR solve? Issue Number: None Related PR: #69601, #64699 Problem Summary: **In short.** #69601 made FE reject `format=avro` while it parses file format properties. This PR adds end-to-end coverage for that: a regression suite that runs `s3()` and `hdfs()` with `format=avro` and checks that they fail during analysis, and a unit test for the two table-function constructors. Test only, no code change. **Background.** Master no longer reads standalone Avro files: #61141 removed the BE reader, #65733 the scanner module, and #69601 made `FileFormatProperties.createFileFormatProperties` throw `Avro file reads are no longer supported`. Before #69601, FE accepted the format and the query failed later in BE with `Not supported file format in fetch table schema: 12`. branch-4.2 still reads Avro through the JNI scanner (#64699 fixed a crash in it), so users who upgrade from 4.x with Avro table functions or loads get this error. **What was missing.** #69601's unit test calls `createFileFormatProperties` directly. Nothing checks that a table function reaches that check before it validates storage properties or lists files. If a constructor validated the location first, `format=avro` against an unreachable location would report a storage error instead, and no test would notice. **What this PR adds.** - `external_table_p0/tvf/test_tvf_avro_format_rejected`: `s3()` and `hdfs()` with `format` set to `avro` and to `AVRO`, against a made-up bucket, made-up credentials and an HDFS address nothing listens on. Each call must fail with `Avro file reads are no longer supported`. - `UnsupportedExternalReadsTest#testAvroTableFunctionsAreRejectedBeforeStorageAccess`: constructs `S3TableValuedFunction` and `HdfsTableValuedFunction` with `format=avro` / `AVRO` and checks the same message. **Results.** On master `2a4191c1dc9` with this change, all four statements of the suite fail during analysis with `Can not build s3(): Avro file reads are no longer supported` / `Can not build hdfs(): ...`, and the unit test passes. Before #69601 the same `s3()` call would have gone on to validate and list the made-up location, so the suite pins the order as well as the message. |
||
|
|
a72dcaa17b |
[fix](fe) Keep an ADBC catalog's driver open until the calls still using it finish (#69874)
### What problem does this PR solve? Issue Number: None Related PR: #66331, #68353 Problem Summary: **In short:** dropping or altering an ADBC catalog can crash the FE process. Closing the catalog frees the native ADBC objects at once, including the connections and results that other threads are still reading, such as the column-statistics loads that a query on the catalog has just started. This kills the FE in about one External regression run in 75, whatever the PR under test changes. With this PR, closing a client only refuses new calls, and the last call still in flight releases the driver as it leaves. DROP and ALTER CATALOG still return without waiting. This commit was first pushed as the last commit of #68353, which is still in review. It shares nothing with the rest of that PR, so it is carved out here to land on its own. #### Background An ADBC catalog (#66331) reads its metadata through `AdbcClient`, which owns one native `AdbcDatabase` and one Arrow allocator per catalog. Every metadata call borrows a fresh connection through `AdbcClient.withConnection` and closes it afterwards. FE talks to the C driver through adbc-driver-jni 0.24. In that bridge, closing a parent closes everything still open under it, on the closing thread: `JniDatabase.close()` closes every connection still open on the database, and `JniConnection.close()` closes every result (`getObjects` and similar readers) still open on the connection. The bridge only checks a handle before a call starts, and the C driver does not check at all, so freeing an object that another thread is still using is a use-after-free in native code, which takes the whole JVM down rather than failing one call. Column statistics reach the driver too. When a query plans against an ADBC table, the STATS_FETCH pool loads column statistics in the background. A load that finds nothing in the statistics table asks the table itself: `PluginDrivenExternalTable.getColumnStatistic` → `AdbcConnectorMetadata.getTableHandle` → `withConnection` → `getObjects`. With `"meta.cache.adbc.metadata.enable" = "false"`, every such load calls into the driver. #### The problem, and what it cost - `AdbcClient.close()` closed the database and then the allocator immediately, whatever was still in flight. DROP CATALOG and ALTER CATALOG ... SET PROPERTIES close the catalog's connector this way, on their own thread (since #69665, on the `ExternalResourceReclaimer` pool; before that, synchronously on the statement thread, with the same race). - A statistics load that was reading a `getObjects` result, or was inside the driver on its connection, then read memory that had just been freed. Where it dies depends on where that thread is at that moment. - In CI, the External regression pipeline lost its FE in 11 of the 834 runs between 2026-10-22 and 2026-10-30. Each time, 54 to 56 suites then failed with `Communications link failure`, so the PR under test showed a red run it did not cause. All 11 died at the same moment: `test_adbc_sqlite_catalog_scan` drops its catalog with the metadata cache turned off right after querying it. In the same millisecond, fe.log shows the allocator closing with `Memory was leaked by query` and a statistics load failing with `Failed to read the ADBC getObjects result: ArrowArrayStream is already closed`. That is the same race, caught by a thread that came back only after the close had finished. | FE crash in `fe.out` | Runs | | --- | --- | | `SIGSEGV` in `nanoarrow::ArrayStreamFactory<VectorArrayStream>::get_next_wrapper+0x4` | 6 | | `SIGSEGV` in `PrivateArrowSchemaDeepCopy+0x46` | 2 | | glibc `double free or corruption (fasttop)` | 2 | | no message | 1 | | `SIGSEGV` in `sqlite3FindTable` (an earlier run, 2026-10-06) | 1 | - Users can hit this too, not only CI: DROP or ALTER an ADBC catalog while a statement or a statistics load is still reading it. #### How this PR fixes it - `AdbcClient` counts the calls inside `withConnection` (`enter` / `leave`, under the client's monitor, which also opens the database on first use). - `close()` marks the client closed, so new calls are refused with `AdbcClient has been closed`. It releases the database and the allocator itself only when no call is in flight. Otherwise it returns at once, and the last call to leave releases them. - A deferred release that fails is logged with the driver's path, as the catalog already logs a failed connector close. It is not thrown at that last caller, whose own work succeeded and which did not close anything. - `close()` never waits for the calls in flight, so DROP and ALTER CATALOG are not held up by a slow remote source. #### Results Unit tests (0 skipped). The first case runs against the real JNI bridge and the SQLite driver, the others against a recording database: | `AdbcClientTest` case | `close()` releasing at once, as before this PR | This PR | | --- | --- | --- | | a call still on its connection when the client is closed keeps using it | fails: `Native connection handle is closed` | passes | | a call still reading a `getObjects` result when the client is closed reads it to the end | fails: `the database was released under an open connection` | passes | | the last call to leave a closed client releases the database exactly once, after it leaves | fails: `the database was released under an open connection` | passes | | a release that fails as the last call leaves does not fail that call | fails: `the database was released under an open connection` | passes | | an idle client releases at once | passes | passes | Freeing only the allocator at once, and leaving the database to the last call, fails the `getObjects` case alone: `Attempting operation on allocator when allocator is closed`. A stress program (not committed) closes a client 200 times while four threads loop over `getObjects` and `getTableSchema` through `withConnection` against the SQLite driver: | | `AdbcClient` of master | This PR | | --- | --- | --- | | Runs | 5 | 5 | | JVM crashed | 5 of 5, 1.1 to 1.3 s after the JVM started: `SIGSEGV` in `PrivateArrowSchemaDeepCopy+0x40`, under `ArrowArrayStreamReader.readSchema` ← `AdbcObjectsReader.readNamespaces` ← `AdbcClient.withConnection` | 0 of 5 | | Closes completed per run | crashed before finishing | 200 | | Metadata calls per run | crashed before reporting | 6,019 to 7,238 | | Calls the close reached while they were using the driver | | 0 | | Failed closes, failed deferred releases | | 0, 0 (checked with WARN logging on, where the unit test's deliberate release failure shows up as one warning) | The stress runs used the macOS arm64 build of the same SQLite driver, so the offset differs from the Linux one in CI. #### Classes, and how they call each other - `AdbcClient`: `withConnection` brackets the body with `enter` / `leave`; `close` refuses new calls and releases only when idle; `release` closes the database, then the allocator; `openDatabase` is the one place the database is created, so a test can stand a recording database in for the driver. - `AdbcClientTest`: five new cases, one against the real driver and four against a recording database. The recording connection answers `getObjects` with a result built in memory in the client's allocator: reading a driver's result into Arrow loads arrow-c-data's JNI shim, which needs CXXABI_1.3.9 and does not load on the FE UT agent (see `AdbcNativeTestSupport`, #68510). - `AdbcObjectsReaderTest`: `response`, `catalog` and `schema` are package-private, so `AdbcClientTest` builds that result with them. ``` STATS_FETCH: ColumnStatisticsCacheLoader.doLoad -> PluginDrivenExternalTable.getColumnStatistic -> AdbcConnectorMetadata.getTableHandle (namespaces, table names) -> AdbcClient.withConnection enter() refused once closed; callsInFlight++ -> AdbcConnection.getObjects -> AdbcObjectsReader.readNamespaces / readTableNames leave() callsInFlight--; the last one out of a closed client -> release() DROP CATALOG / ALTER CATALOG ... SET PROPERTIES -> ExternalCatalog.onClose / resetToUninitialized -> ExternalResourceReclaimer -> AdbcConnector.close -> AdbcClient.close() closed = true; release() now only if no call is in flight release(): AdbcDatabase.close (closes its connections and their results) -> allocator.close ``` Not changed: `AdbcConnector`, the catalog lifecycle and `ExternalResourceReclaimer`, BE, and the regression suites; `test_adbc_sqlite_catalog_scan` stays as it is, since what it exposes is this bug. |
||
|
|
ba3f2c6af0 |
[fix](regression) Sync version 3 before DROP in test_clean_tablet_when_drop_force_table (#69846)
Related PR: #68449 Problem Summary: `test_clean_tablet_when_drop_force_table` fails intermittently with `File cache directories were not cleaned before timeout`. The leftover cache dirs all belong to the version 3 rowsets. #68449 set `disable_auto_compaction=true` for this case. Before that change, cloud compaction called `sync_rowsets()` and loaded version 3 into the BE tablets as a side effect. Now nothing loads version 3 between the second stream load and `DROP TABLE FORCE`, because the case only queries after the version 2 load. The cloud `drop_tablet_callback` only recycles the file cache of rowsets in `get_snapshot_rowset(true)`, so the version 3 cache stays and the cleanup assertion times out. This PR queries the table after the version 3 load, the same way the case already does after version 2. The query loads version 3 on every BE before DROP. It also asserts the row count (160000) to confirm version 3 is visible. |
||
|
|
339e087b11 |
[fix](be) Reject incompatible Arrow arrays without crashing Stream Load (#69872)
### What problem does this PR solve? Issue Number: close #69858 Related PR: None Problem Summary: The shared Arrow BOOLEAN and DECIMAL SerDes dereference unchecked `dynamic_cast` results. Passing an `Int32Array` to the BOOLEAN reader or a `StringArray` to the DECIMAL64 reader can therefore terminate the BE instead of returning a load error. MAP and STRUCT have the same unchecked input-cast pattern. Reject incompatible arrays before accessing buffers, decimal metadata, or values, independently of `enable_arrow_input_validation`. Reject mismatched STRUCT field counts before indexing children. Add the column name and source/destination types to V1/V2 Arrow load errors. The current V2 Stream Load reader already binds supported input types with safe Arrow casts. Preserve those conversions, including valid integer-to-BOOLEAN and string-to-DECIMAL inputs, and test the shared SerDes directly for incompatible arrays. |
||
|
|
fb61aeeb63 |
[fix](plugin) Allow legacy unversioned authentication plugins (#67286)
### What problem does this PR solve? #66211 Problem Summary: The authentication plugin API-version gate rejects every plugin jar that does not contain `Doris-Authentication-Plugin-Api-Version`. Authentication plugins were distributed before this manifest contract existed, so upgrading FE breaks those legacy artifacts even when they implement the current v1 API. This change adds an opt-in legacy mapping to the generic API gate and configures only the authentication family to treat a genuinely absent declaration as API 1.0: - unversioned legacy authentication plugins load with an operator warning; - empty, malformed, and explicitly incompatible declarations remain rejected; - connector, filesystem, lineage, and other strict gates keep their existing fail-closed behavior; - when the FE authentication API moves to another major, legacy v1 plugins are rejected automatically. |
||
|
|
374d87d81d |
[perf](csv reader) Reduce V2 CSV parsing overhead (#69875)
### What problem does this PR solve? Issue Number: None Related PR: #69716 Problem Summary: After restoring sequential prefetch, large V2 CSV loads still spend substantial CPU time in row parsing and string materialization. The reader compares a general separator at every byte, updates atomic profile counters per cell, starts three stage timers per row, and repeats checked string-column casts per cell. It also ignores the row cap supplied by TableReader. This change combines four optimizations on master: - Compare the separator byte directly for plain single-character separators; retain enclosed, multi-character, Hive/OpenCSV and PROTO parsing paths. - Accumulate exact work counters locally and publish at get_block/count/schema return, including errors. Sample stage timings every 1024 physical read_line calls, starting with the first, and expose unscaled SampledReadLineTime, SampledSplitLineTime and SampledDeserializeTime with sample counts. DelimitedTextReader remains full elapsed time; split-prefix alignment has its own full timer. - Validate nullable string column types and reserve offsets/null maps once per mutable batch, then insert strings directly with the existing NULL, quoting and escape semantics. - Honor FileReader::set_batch_size, retaining the existing byte budget and runtime default when no explicit cap is supplied. The caller still controls the maximum batch size; this does not force 16352-row blocks. Shared counters and batch sizing also apply to the Hive text reader. ### Performance benchmark On TPC-DS SF100, elapsed time decreased from **1,944 seconds to 1,375 seconds**: | Benchmark | Before | After | Time reduction | Speedup | | --- | ---: | ---: | ---: | ---: | | TPC-DS SF100 | 1,944 s | 1,375 s | 29.3% (569 s) | 1.41× | ### Release note Reduce V2 CSV parsing overhead and honor caller-provided batch sizes. Delimited-text per-stage profile timers are now explicitly named Sampled* and contain unscaled sampled durations; use the full DelimitedTextReader timer for elapsed-time comparisons. ### Check List (For Author) - Test - [x] Unit Test: added cases for dynamic/zero batch caps, byte limits, filtered blocks, cross-batch sampling and exact counters, single/multi-character separators, NULL and escape behavior, missing fields, reordered projection and copy-on-write. Extended error, rejected-row and COUNT counter coverage. - Execution: run-be-ut.sh was attempted with CsvV2ReaderTest.*, TextV2ReaderTest.* and LegacyAndV2/PlainCsvSplitTest.*. It exits before C++ compilation because the local JDK 17 fails to initialize (Failed setting boot class path), including outside the sandbox. Full build/UT remain pending. Performance benchmark results are reported above. - Passed: repository clang-format 16 formatting/check scripts, build-support/check-build-hygiene.sh and git diff --check. - Behavior changed: Yes. Existing batch-size contract is honored, and detailed stage timers are explicitly sampled. CSV value semantics are unchanged. - Does this need documentation: No. The profile includes a timing explanation; no new configuration or SQL syntax. For further benchmark comparisons, use identical hardware, input, compression, cache conditions and load batch/byte-budget settings for the parent and this commit. Compare total load time, Scanner CPU time, DelimitedTextReader time and RowsRead/BlockNumber. Do not directly compare Sampled* timers with the previous full stage timers. |
||
|
|
45d1598da2 |
[improvement](be) Accelerate phmap control scans with ARM NEON (#69688)
AArch64 currently uses portable scalar phmap control-byte scans. The subtraction-based H2 mask can include adjacent false-positive candidates: control bytes `0x12, 0x13` can both be returned when looking for `0x12`. Key comparison preserves correctness, but the extra candidates cost work. Add exact NEON control comparisons on little-endian AArch64 while keeping eight-byte groups, the portable fallback and the existing ARM dump/load layout. x86 keeps its existing SSE implementation. `PHMAP_HAVE_ARM_NEON=0` disables the new implementation at compile time. The normal third-party downloader applies this patch alongside the existing AArch64 int128 hash-mixer patch, with separate markers and a subshell that restores the caller's working directory. The NEON patch now includes context so a replay cannot silently insert the same code again. The downloader checks the actual source with a direction-locked reverse dry-run, recovers a missing marker when all hunks are already applied, and preflights every hunk before a fresh application. Partial or duplicated sources fail without further edits and do not retain a success marker. The standalone tests cover control masks, unaligned reads, special slots, conversion bounds, map operations, rehash and dump/load. The standalone C++ benchmark can be compiled directly. For an incremental comparison, retain the existing int128 patch in both the baseline and optimized dependency sources, adding NEON only to the optimized copy. ### Supplied ARM measurements The following measurements come from the supplied report on original Doris baseline `70f7dcbe11c8e142b81536c1dfaaf505ec05437b`, not a new ARM run of this rebased PR. That baseline predates master's AArch64 int128 patch. These numbers therefore do **not** establish the incremental gains of the final combined patch stack; a new ARM run with the combined patch stack is still needed. Environment: Neoverse-N2, 16 cores, nominal 3 GHz; Clang 20.1.8; C++17, `-O3 -DNDEBUG -march=armv8-a+crc -pthread`; one process pinned to CPU 15. Nine alternating baseline/NEON rounds, each measurement at least 0.2 seconds of thread CPU time. Values are median thread CPU ns/op. Reported intervals use 10,000 paired bootstrap resamples. Positive time reduction means faster; negative means slower. | Scenario | Baseline ns/op | NEON ns/op | Time reduction | Reported 95% interval | |---|---:|---:|---:|---:| | Hit lookup, 4,096 keys | 4.0773 | 3.8350 | +5.94% | [+5.84%, +6.11%] | | Hit lookup, 100,000 keys | 10.6120 | 9.6412 | +9.15% | [+8.99%, +12.44%] | | Hit lookup, 114,000 keys | 11.4606 | 10.5508 | +7.94% | [+7.73%, +11.42%] | | Hit lookup, 1,000,000 keys | 16.3554 | 14.4228 | +11.82% | [+10.20%, +12.74%] | | Miss lookup, 4,096 keys | 3.4818 | 3.3069 | +5.02% | [+4.81%, +5.08%] | | Miss lookup, 100,000 keys | 9.7027 | 9.9144 | -2.18% | [-2.91%, -1.27%] | | Miss lookup, 114,000 keys | 16.8860 | 17.5147 | -3.72% | [-3.93%, -2.89%] | | Miss lookup, 1,000,000 keys | 8.6927 | 8.1035 | +6.78% | [+4.93%, +9.01%] | | Insert, 100,000 keys, including allocation/destruction | 9.8116 | 9.2758 | +5.46% | [+3.94%, +6.25%] | | Erase/reinsert, 20,000 operations per iteration | 16.3032 | 16.3810 | -0.48% | [-1.19%, +0.97%] | | Synthetic adjacent-H2 scan, 32,768 groups | 2.7578 | 1.7641 | +36.03% | [+35.75%, +36.09%] | Load factors differ: approximately 50.0%, 76.3%, 87.0% and 47.7% at the four lookup sizes. Two miss-lookup workloads regress reproducibly. The erase/reinsert interval crosses zero. The adjacent-H2 case measures group scans and candidate-key comparisons, not complete table lookups. These are library microbenchmarks, not end-to-end SQL gains. ### Validation **Reported in the supplied ARM bundle:** randomized scalar-oracle checks, baseline/NEON/forced-portable paths, bidirectional dump/load and ASAN/UBSAN passed. The archive contains the patch and report; these ARM executions were not repeated during this submission. **Executed locally after rebasing onto master `3d2d9aac54148056529c13ef4b9fbdbcb65fab77` (x86_64, Clang 16.0.6):** - Standalone correctness tests passed for baseline, patched and explicitly NEON-disabled builds; x86 selected the existing 16-byte SSE groups in all three. Tests also exercise the portable group implementation directly. - Baseline-to-patched and patched-to-baseline dump/load passed; patched ASAN/UBSAN passed. - Benchmark source compiled with `-Wall -Wextra -Werror`; all 11 scenarios passed a short checksum smoke run. No x86 performance conclusion is drawn from it. - Standard third-party download/patch script passed twice with `OLDPWD` unset. The standard downloader produces byte-identical headers to those used by the standalone tests. - clang-format 16, repository clang-tidy entry point for both standalone C++ files, shell syntax and `git diff --check` passed. **Downloader recovery follow-up:** `bash thirdparty/test/parallel-hashmap-patch-test.sh` passed using the pinned cached archive and the real downloader; no dependency downloads were needed. It covers fresh extraction, normal reruns, an applied patch with a missing marker, a stale marker over pristine sources, partially applied sources with/without a marker, and duplicate NEON definitions. Successful reruns compare both complete headers; failing cases verify unchanged source and absence of the success marker. The same test detects the original replay bug. The regenerated patch inserts exactly the same 67 C++ lines, and its applied header bytes match the original optimized source. Bash syntax and shell whitespace checks passed; generated patch context preserves the pinned dependency's existing whitespace. Full Doris BE compilation, SQL regression/performance tests and new ARM measurements against the combined int128+NEON patch stack have not been run. The C++ test and benchmark have no Python dependency and can be compiled directly: ```bash bash thirdparty/download-thirdparty.sh parallel_hashmap clang++ -std=c++17 -O2 -pthread -Wall -Wextra -Werror \ -isystem thirdparty/src/parallel-hashmap-1.3.8 \ thirdparty/tests/parallel_hashmap_neon_test.cpp -o /tmp/phmap-neon-test /tmp/phmap-neon-test clang++ -std=c++17 -O3 -DNDEBUG -pthread -Wall -Wextra -Werror \ -isystem thirdparty/src/parallel-hashmap-1.3.8 \ thirdparty/benchmarks/parallel_hashmap_bench.cpp -o /tmp/phmap-neon-bench taskset -c 15 /tmp/phmap-neon-bench 0.2 ``` Select an available isolated CPU instead of CPU 15 when necessary. For the reported ARM compiler settings, add `-march=armv8-a+crc`. The benchmark emits one JSON line per scenario with CPU/wall timings and checksums. To compare against baseline on current master, extract the verified source archive into another directory, apply only `parallel-hashmap-1.3.8-aarch64-int128.patch`, compile the same C++ benchmark against that directory with identical flags, and alternate baseline/optimized execution order. ### Release note Accelerate phmap control-byte scanning on little-endian AArch64 while preserving eight-byte ARM groups and the portable fallback. |
||
|
|
96f2558e16 |
[feature](function) Add MIN_TOPK/MAX_TOPK and ARRAY_SORTED_MERGE functions (#69456)
### What problem does this PR solve? Issue Number: N/A (tracked in https://github.com/apache/doris/issues/65418) Related PR: #69286 Problem Summary: Adds the generic building blocks that incremental MIN/MAX maintenance builds on; the IVM integration itself is a separate PR. - `min_topk(x, k)` / `max_topk(x, k)`: an aggregate returning `ARRAY<STRUCT<value T, count BIGINT>>` — the k smallest (largest) distinct non-NULL values with their exact row counts, ordered by value. The array carries no truncation flag: a caller that needs one infers it from the length, treating a list of exactly k entries as possibly truncated. With `k = 0` the array holds at most the single `(extreme, count)` pair. `k` is a constant `BIGINT` with `0 <= k <= 65536`. - `array_sorted_merge(merge_fn, arr1, arr2, is_asc)`: a generic higher-order function that merges two arrays already sorted by the elements' natural order. `merge_fn` receives the two current candidates and returns the combined element, or NULL when the two elements must be kept separately. `is_asc` states the sort direction, so one function covers ascending and descending inputs, and calls can be chained to merge more than two arrays. The aggregates keep a sorted `(value, count)` state with checked int64 counting, so partial aggregation and merge stay exact (`topK(A ∪ B) ⊆ topK(A) ∪ topK(B)`). FLOAT/DOUBLE order NaN as greater than every number so the distinct list keeps a strict order. Returning the array alone keeps the aggregate in the same shape as the other array aggregates and leaves the truncation flag to the caller that actually persists a bound: incremental MIN/MAX maintenance still stores one, but as its own state column rather than inside the aggregate result. `array_sorted_merge` keeps its non-lambda `is_asc` argument through the higher-order binding: the existing pipeline passed only the lambda arguments to the bound function, so the binding contract gained a way to return extra inputs. ### Release note Add the `min_topk`/`max_topk` aggregate functions and the `array_sorted_merge` scalar function. |
||
|
|
9c8f7b2d2e |
[improvement](be) Compact selected row indices with mask lookups (#69818)
`SegmentIterator` expands partially selected SIMD masks one set bit at a
time, creating a serial dependency for every selected row. Expand x86
masks with more than two selected rows using a constexpr eight-row
lookup table and fixed-width stores. Keep the existing bit iteration for
sparse masks and other architectures. Empty/full masks, scalar tails,
row order and downstream column materialization retain their existing
behavior.
The output selector already has one slot per input row. At a complete
SIMD block, the number of emitted indices is at most the input position,
so the fixed-width stores fit within that allocation; only the returned
prefix is consumed. The lookup table has constant initialization and
introduces no dynamic allocation or mutable shared state.
Add unit coverage and a standalone benchmark that measures the original
loop and the actual production helper. On an Intel Xeon Platinum 8457C,
with 4096-row batches, random 50% selection and Clang 20.1.8 `-O3
-march=native`:
| Benchmark | Before (ns/input row) | After (ns/input row) | Speedup |
| --- | ---: | ---: | ---: |
| Row-index generation | 0.693280 | 0.119248 | 5.81x |
| Predicate comparison + selection + 8-byte copy | 1.170464 | 0.643518 |
1.82x |
Performance tradeoff: Clang 16.0.6 with `-march=x86-64-v3` regresses
from 0.104319 to 0.119946 ns/input row at random 1% selection (about 15%
slower). The Clang 20 native 1% case is approximately unchanged. These
are standalone measurements; complete SQL query speedup has not been
measured.
### Release note
None
### Check List (For Author)
- Test
- [ ] Regression test
- [ ] Unit Test
- [x] Manual test (details below)
Validation completed:
- Standalone benchmark with native and AVX2 compiler settings.
- ASan + UBSan verification of all 256 byte masks, 65536 mixed masks,
scalar tails, row order, output bounds, near-uint16 row limits and
payload copies.
- clang-format 16.0.6, repository build-hygiene checks and focused
clang-tidy for the helper, test source and standalone benchmark.
- Prepared this PR as one commit on the latest master; the helper, test
and benchmark source blobs are identical to the verified versions.
Unit tests added: empty/full/single-bit masks, exhaustive byte and mixed
masks, large row bases and consecutive appends. They have **not executed
through the standard BE test runner**. Both `./run-be-ut.sh -j 48 --run
--filter=SelectionTest.*:BitsTest.*` and `BUILD_TYPE=ASAN ./build.sh
--be -j 48` were attempted in the development workspace, but compilation
failed in unrelated existing local `ColumnVector` edits at
`column_vector.h:95` before the tests could execute. Full BE build and
full `SegmentIterator` static analysis remain incomplete. No SQL or ARM
execution.
Standalone reproduction (x86, from the repository root):
```bash
c++ -O3 -DNDEBUG -std=c++20 -march=native -Ibe/src \
be/benchmark/selection_compaction_bench.cpp -o /tmp/selection_compaction_bench
/tmp/selection_compaction_bench --verify-only
/tmp/selection_compaction_bench --pattern random --rate 50 --mode select \
--repeats 9 --min-ms 60
/tmp/selection_compaction_bench --pattern random --rate 50 --input values \
--mode copy8 --repeats 9 --min-ms 80
`
|
||
|
|
4aab9c4a44 |
[fix](iceberg) Honor storage Kerberos settings and retain REST credentials (#69598)
### What problem does this PR solve? Issue Number: N/A Iceberg catalogs configured with `hdfs.authentication.*` can fail against secured HDFS with `Client cannot authenticate via:[TOKEN, KERBEROS]`. Storage binding already translates these properties into Hadoop configuration, but plugin-side authenticator selection checks only the raw legacy `hadoop.security.authentication` key and skips the Kerberos identity. Resolve authentication from the merged Hadoop configuration used by FileIO and create a plugin authenticator only when it resolves to Kerberos with a principal/keytab pair. XML resources can contain a Kerberos mode without credentials; the shared resolver then falls back to SIMPLE. Preserve the existing engine/storage authentication path in that case so FileIO does not switch to the default `hadoop` user. Keep HMS authentication separate. Complete Kerberos credentials can also arrive through XML for a REST catalog that vends temporary S3 credentials. Scan planning wraps the table FileIO for authenticated manifest access, but that wrapper does not expose Iceberg's optional `SupportsStorageCredentials` interface. Iceberg 1.12 stores the REST response's `storage-credentials` separately from `FileIO.properties()`, so inspecting the wrapper alone loses the temporary keys forwarded to BE. Unwrap the authentication decorator only while reading credential metadata; keep file operations authenticated and preserve the existing optional capability surface. Add a canonical-property unit test and an HMS/storage identity separation check, and switch the existing Kerberos regression suite to canonical storage properties so its read/write, rollback, and snapshot-expiration coverage exercises this path. ### Release note Iceberg catalogs honor canonical HDFS Kerberos authentication properties for plugin-side file access and retain REST-vended storage credentials when Kerberos is also enabled. ### Check List (For Author) - Test - [x] Unit Test: canonical credential selection and XML identity fallback both have red/green verification; all 14 authenticator tests pass, including incomplete principal/keytab pairs and the original storage/HMS separation checks. Two additional tests use the real Iceberg 1.12 `S3FileIO` to cover XML-resolved Kerberos with separate REST credentials, nested authentication decorators, credential refresh, the disabled-vended gate, and BE access-key/secret-key/session-token forwarding; both fail before the credential extraction fix and pass afterward. - [x] Unit Test: the complete Iceberg module reports 1743 tests, 0 failures, 0 errors, and 14 skipped (1729 passed), run through `run-fe-ut.sh` with all 106 module test classes selected explicitly. - [ ] Regression test: updated `test_iceberg_hadoop_catalog_kerberos`; KDC/S3/HDFS end-to-end suites were not run locally. - [x] FE Checkstyle: all 88 reactor modules pass `mvn -f fe/pom.xml checkstyle:check`. - Behavior changed: - [x] Yes. Canonical HDFS Kerberos properties now select the plugin-side storage authenticator. - Does this need documentation? - [x] No. This restores the existing properties' documented behavior. |
||
|
|
81cbb5b420 | [fix](fe) Disable localhost authentication bypass by default (#69740) | ||
|
|
6a4284e5ce |
[fix](mtmv): Re-prune Iceberg scans after partition compensation (#69678)
### What problem does this PR solve? Problem Summary: Iceberg materialized-view partition compensation retained the stale partition-pruning state when deep-copying a file scan. The compensation branch therefore scanned all old partitions, causing nested UNIONs, Memo expansion, and fallback to a full Iceberg scan. Invalidate the copied scan's pruning state so the missing-partition predicate is applied again |
||
|
|
6104204b40 |
[improvement](be) Reduce map and string array deduplication overhead (#69690)
`map(k, v)` serializes its keys and allocates deduplication buffers even though each output row has only one pair. String `array_distinct` grows its character buffer and copies each retained value separately. - Skip `deduplicate_keys()` for a single key/value pair. Multiple-pair construction keeps its existing last-value-wins deduplication. - Collect retained positions and call `insert_indices_from()` once for string batches without NULL elements. Use the existing SIMD NULL-map scan to select the scalar path when any NULL exists; this avoids copying hidden string payloads beneath NULLs. Nullable inputs with an all-zero NULL map still use batch copying. - Keep scalar copying when the nested element count exceeds the UInt32 selector limit, and use `insert(...).second` to avoid separate hash lookups. First-occurrence ordering and SQL NULL semantics are unchanged. Tests cover NULL keys/values, constant keys, multiple-pair deduplication, string ordering, hidden NULL payloads, a NULL at the end of the input, empty inputs, long strings and selector growth. Paired Release microbenchmarks used the complete original and updated `execute_impl` entry points in one process with identical inputs. Results below are median CPU milliseconds per 4,096 rows/arrays on a pinned Xeon Platinum 8457C CPU, seven repetitions (selected earlier MAP cases had nine). These are function-level measurements, not end-to-end SQL speedups. | Case | Before (ms) | After (ms) | Before / after | | --- | ---: | ---: | ---: | | Single-pair MAP, BIGINT key/value | 0.0804 | 0.0629 | 1.28x | | Single-pair MAP, 16-byte string key, BIGINT value | 0.0986 | 0.0753 | 1.31x | | Single-pair MAP, 128-byte string key, BIGINT value | 0.1665 | 0.1033 | 1.61x | | ARRAY, 32 distinct 16-byte strings | 2.063 | 1.664 | 1.24x | | ARRAY, 32 distinct 128-byte strings | 22.993 | 10.550 | 2.18x | | ARRAY, 128 distinct 128-byte strings | 116.683 | 57.871 | 2.02x | | ARRAY, 128 x 128-byte strings, 25% retained | 31.968 | 20.596 | 1.55x | | ARRAY, 128 x 128-byte strings, one distinct value | 10.792 | 11.104 | 0.97x | | ARRAY, 32 NULL elements with empty payloads | 0.1173 | 0.1219 | 0.96x | | ARRAY, 8 NULL elements with 4KiB hidden payloads | 0.0400 | 0.0413 | 0.97x | Highly duplicated and all-NULL inputs showed little benefit; small differences in these controls were within the observed sample variability. NULL batches keep scalar copying. Temporary frozen-baseline benchmark code and build-entry changes are excluded from this PR. ### Release note Improve execution efficiency of single-pair MAP construction and string ARRAY_DISTINCT batches without NULL elements, preserving SQL results. |
||
|
|
9e61225e3e |
[feature](file cache) Add PageIO read-ahead, I/O coalescing, and cache block hole filling (#67292)
### What problem does this PR solve? Issue Number: None Related PR: #67579 (validation and follow-ups) Problem Summary: Doris data pages are commonly much smaller than a 1 MiB File Cache block. On a cache miss, the existing query path expands a page read to the block boundary. This behaves like a fixed 1 MiB read-ahead for continuous scans, but the size and shape cannot follow the actual scan. Sparse point-query data-page reads can therefore download a full block for a small page, while sequential scans still expose one remote page dependency at a time through `FileColumnIterator`. This PR adds an opt-in cloud query path that prepares each eligible top-level scalar scan column's compressed data pages once after pruning, advances its window on physical data-page reads, merges nearby pages into bounded file ranges, and serves the original exact page slices to the existing PageIO and decoder. Only a range that PageIO actually consumes becomes eligible for File Cache writeback. Complete cache blocks use the existing asynchronous cache writer; partial blocks enter a bounded background pipeline that fills their holes and then hands a complete block to the same writer. The implementation covers top-level scalar columns in regular segment scans and short-circuit point queries, including scalar row-store reads. ARRAY/MAP/STRUCT and their offsets, null maps and children retain the original readers and legacy segment prefetchers; variant paths remain on the original path. TopN/global delayed-materialization second-stage read-ahead is outside the current PRs; that path stays identical to master. The feature is disabled by default and requires both `enable_query_read_ahead=true` and `enable_async_file_cache_write=true`. Planning, admission, asynchronous read, checksum, decode, and best-effort writeback failures preserve the original query result path. #### Entry paths and component boundaries ```mermaid flowchart TB SCAN["Regular SegmentIterator scan"] POINT["PointQueryExecutor<br/>row store or missing columns"] SCAN --> COLUMNS["After pruning: initialize scalar column windows once"] POINT --> LOOKUP["Collect exact row IDs and required scalar columns"] COLUMNS --> SEGMENT["SegmentReadAhead"] LOOKUP --> SEGMENT SCAN --> COMPLEX["Complex columns: original reader and prefetch"] POINT --> COMPLEX SEGMENT --> PLAN["FileRangePlanner"] PLAN --> SCHED["FileRangeReadScheduler"] SCHED --> POOL["Existing SegmentPrefetchThreadPool"] POOL --> READER["SegmentReadAheadFileReader"] READER --> PAGEIO["Existing PageIO and decoder"] READER --> WRITEBACK["Consumed-range writeback"] ``` | Component | Responsibility | |---|---| | `ColumnReadAhead` / `ColumnReadAheadPlan` | Prepare eligible physical pages once and advance byte windows from page reads; the plan owns reset and timing aggregation. | | `SegmentReadAhead` | Combine all column plans for a segment, probe Page Cache, plan ranges, submit one batch, and register exact page locations. | | `FileRangePlanner` | Merge sorted page intervals and optionally complete high-coverage boundary cache blocks. It performs no I/O. | | `FileRangeReadScheduler` | Admit ranges that fit query/BE budgets in one batch pass; provide cancellation and concurrent execution for already planned file ranges. | | `SegmentReadAheadFileReader` | Serve an exact page slice from a ready range, immediately retire that page, and fall back to the original reader when needed. | | `RangeWritebackDispatcher` / `RangeCacheWriteback` | Split a consumed range at 1 MiB cache-block boundaries and route complete blocks and partial fragments. | | `PartialBlockWritebackManager` | Bound and deduplicate partial-block work, fill single ranges on block workers or multiple ranges on a shared remote-read pool, and submit complete owned buffers to the existing cache writer. | #### Read-ahead window Columns are classified by dependency: eager columns have no earlier filtering dependency; lazy columns depend on an earlier filter or expression result. After segment pruning, initialize all currently resolvable eager and lazy windows together. Build each column's candidate-page indexes once from the pruned selection. Later data-page reads advance the window using the already resolved physical page index, before Page Cache lookup. Small scanner batches no longer rebuild rowid plans for every column. Each window selects eligible pages until their compressed bytes reach the configured size; its final page can cross that size. The second selected page triggers the next window, or the only page for a one-page window. First access and a jump beyond planned coverage start at the current page. Reverse scans retain predictions while reading an ascending batch and retire them when the next lower batch begins. Scalar point reads keep exact-row preparation, grouping sorted row IDs by distinct pages. Complex columns keep the original dependent-child reading path. Page completion updates ownership/accounting; the page-read position drives scan-window extension. Initial planning remains on the scanner thread; no separate planning pool is introduced. | Configuration | Default | Reason for the default | |---|---:|---| | `read_ahead_eager_window_bytes` | 4 MiB | An early trigger exposes the next window while the current one is read, bridging small scanner batches. | | `read_ahead_lazy_window_bytes` | 1 MiB | Expose more dependent-column I/O while retaining a smaller window than eager columns. | ```mermaid flowchart TD Init["After pruning: select candidate pages once<br/>initialize eager and lazy windows together"] --> Read Read["Column reader resolves a physical data page"] --> Advance["Retire passed predictions<br/>include the current page"] Advance --> Outside{"At or beyond planned coverage?"} Outside -- Yes --> Start["Start a byte window at this page<br/>mark its second selected page as the trigger"] Outside -- No --> Trigger Start --> Trigger{"Current page reaches the trigger?"} Trigger -- Yes --> Append["Append the next byte window<br/>move the trigger into it"] Trigger -- No --> Apply Append --> Apply["Submit newly planned pages<br/>reuse existing buffered pages"] Apply --> PageIO["Existing Page Cache / PageIO / decoder"] PageIO --> Read ``` The window is a planning unit, not a resident-memory limit; query and BE budgets bound retained range buffers. `ReadAheadColumnInitTime` covers initialization, `ReadAheadPageAdvanceTime` covers page-driven maintenance, and `ReadAheadCurrentBatchPlanTime` covers the retained exact-row path. Window discard and extension remain sub-timers under `ReadAheadColumnPlanTime`. #### Range planning and concurrent I/O The range merge uses a left-to-right algorithm inspired by Velox. Here, “merge” means combining nearby requested page intervals into one physical object-store GET. `SegmentReadAhead` supplies sorted, disjoint, non-empty page intervals; one linear pass adds the next interval only when every configured limit remains satisfied. | Configuration | Default | Meaning | |---|---:|---| | `read_ahead_max_gap_bytes` | 64 KiB | Largest unrequested gap that one foreground GET may cross. | | `read_ahead_max_range_bytes` | 2 MiB | Largest range produced by merging or foreground block completion. A single indivisible input page may exceed it and remains a standalone read. | | `read_ahead_max_read_amplification_ratio` | 1.0 | Largest ratio of physical range bytes to requested page bytes. | After the base ranges are formed, `FileRangePlanner` examines their boundary File Cache blocks. `read_ahead_block_fill_min_coverage=1.0` requires full requested-page coverage, so the default adds no foreground bytes to complete a partial block. Lowering this threshold retains the existing optional completion algorithm within the 2 MiB range limit. ```mermaid flowchart LR PAGES["Sorted unique data-page intervals"] --> MERGE["Linear three-limit merge"] MERGE --> BASE["Base ranges"] BASE --> COVER{"Boundary block coverage<br/>at least 50 percent?"} COVER -->|"Yes and final range at most 2 MiB"| COMPLETE["Extend to block boundary"] COVER -->|No| KEEP["Keep base range"] COMPLETE --> BATCH["One atomic submission batch"] KEEP --> BATCH BATCH -->|Accepted| WORKERS["Concurrent range reads"] BATCH -->|Rejected| FALLBACK["Original synchronous path"] ``` `FileRangeReadScheduler` walks the planned ranges once under both query and BE budget locks, admitting each range that fits and continuing past skipped ranges so later smaller ranges can still proceed. Returned handles preserve the original range indices, with empty slots for budget skips; only admitted ranges receive buffers and tasks. It adds no range-count or concurrency quota. Accepted tasks run on the existing segment-prefetch thread pool, whose configured thread count controls physical read concurrency; the scheduler does not own or stop that shared pool. A byte reservation follows the query-owned buffer lifetime and is released with the last reference. Scheduled reads suppress direct cache writes. A ready range is indexed back to every exact page it contains, so PageIO still receives the original offset and size and keeps the existing checksum and decoder behavior. ```mermaid sequenceDiagram participant Scan as Scanner participant Column as ColumnReadAhead participant Segment as SegmentReadAhead participant Scheduler as FileRangeReadScheduler participant PageIO Scan->>Column: initialize candidates and first windows after pruning Column->>Segment: eager and lazy initial plans Segment->>Scheduler: page-cache probe, merge, complete blocks, submit ranges loop Physical data-page reads Scan->>Column: resolved page index Column->>Segment: new and released pages when the window changes Segment->>Scheduler: submit newly planned ranges Scan->>PageIO: exact page offset and size PageIO->>Segment: read buffered page Segment-->>PageIO: ready slice or original-reader fallback end ``` Point queries create one request-scoped range-read context and plan either the row-store column or all missing column-store values before decoding; existing row-cache and result-padding behavior remains unchanged. The point-query component owns the shared exact-row page helpers and their tests. #### Consumed-range writeback A ready range that read remote bytes becomes eligible for writeback when PageIO successfully reads its first registered page, and it is published at most once. Pure File Cache/inflight hits skip writeback. A prefetched range whose pages are all passed without use is released without a cache write. ```mermaid flowchart TD CONSUMED["Consumed ready range"] --> ORIGIN{"Range read remote bytes?"} ORIGIN -- No --> SKIP["Skip duplicate writeback"] ORIGIN -- Yes --> SPLIT["Split at 1 MiB File Cache boundaries"] SPLIT --> FULL{"Fragment covers the complete valid block?"} FULL -->|Yes| ASYNC["Existing AsyncCacheWriteManager"] FULL -->|No| PARTIAL["PartialBlockWritebackManager queue"] PARTIAL --> DISPATCH["HoleFillDispatch: ordered deadline and capacity checks"] DISPATCH --> ELIGIBLE{"Aggregation deadline elapsed<br/>and cache writer has capacity?"} ELIGIBLE -- No --> WAIT["Dispatcher waits on condition variable<br/>task remains queued and mergeable"] WAIT --> ELIGIBLE ELIGIBLE -- Yes --> READY["Bounded worker handoff"] READY --> WORKER["Block worker: recheck cache and plan block-local holes"] WORKER --> COUNT{"Number of merged hole ranges?"} COUNT -- Zero --> OWNED["Complete owned block buffer"] COUNT -- One --> INLINE["Read directly on the block worker"] COUNT -- Multiple --> REMOTE["HoleFillRemoteReadPool: one task per range"] REMOTE --> JOIN["Block worker joins all accepted reads"] INLINE --> RESULT{"All reads succeeded?"} JOIN --> RESULT RESULT -- Yes --> OWNED RESULT -- No --> DROP["Drop best-effort writeback"] OWNED --> ASYNC ASYNC --> CACHE["Existing File Cache write path"] ``` Complete 1 MiB fragments, including a shorter physical EOF block, enter the existing asynchronous cache writer directly. Partial fragments are copied into a BE-level queue. Fragments for the same queued block merge under that task's fragment lock; an already active fill is deduplicated. A combined inflight-buffer and File Cache probe skips cached/downloading blocks before admission and once when a worker starts the task, outside the queue mutex. The task then executes its planned GETs and retains the final cache-writer checks. Its source pointer reads directly from the inner remote reader while retaining the cached wrapper as the shared owner, keeping the source open through delayed hole filling. Queued partial blocks retain a mutable minimum aggregation interval, `hole_fill_merge_delay_ms=2000` (milliseconds), to keep queued fragments mergeable for at least two seconds before dispatch. Timing starts at first admission; merging later fragments preserves the timestamp. The queue remains admission-time ordered, with stale replacements moving to the tail. One `HoleFillDispatch` thread checks due entries and stops at the first future deadline. It can bypass a due task whose writer lacks capacity, then waits until the earliest deadline or capacity retry. Runtime delay updates wake this dispatcher; tail arrivals that cannot advance its wait leave the timer alone. Waiting releases the mutex and preserves fragment merging. Complete blocks still enter the asynchronous cache writer directly. The queue accounts one full cache-block buffer per queued or active entry and is bounded by `hole_fill_max_pending_bytes_per_be=4 GiB`. When full, the oldest queued task is evicted. If every retained task is already active, new cache-only work is rejected. The query read result is unaffected in either case. Block workers, dynamically resized by `hole_fill_workers_per_be=32`, consume ready tasks. The total of dispatched and executing tasks is bounded by that worker count, so excess entries stay in the mergeable queue. Dispatch moves existing list nodes into the handoff queue. Shrinking lets already-dispatched work drain before admitting more. Writer-capacity accounting includes the handoff, and final write submission still uses spare-capacity-only admission. Active-task metrics include both dispatched and executing work. #### Hole-fill algorithm For one partial cache block, `HoleFillPlanner` unions the covered fragments, takes their complement inside that block, and applies the same three-limit merge algorithm to the missing intervals. Hole ranges never cross a 1 MiB block boundary. A single merged range is read directly on the block worker, avoiding task submission and wakeup overhead. Multiple ranges are submitted concurrently to the BE-wide `HoleFillRemoteReadPool`, controlled by mutable `hole_fill_remote_read_threads_per_be=64`. This pool limit applies only to multi-range reads; single-range reads also run on the block workers. Each block worker creates and reuses one concurrent token for its lifetime. Before writeback or failure cleanup it joins all accepted GETs, including when a submission or read fails. Shutdown stops admission, joins the dispatcher and discards queued/undelivered work without its GETs, then joins executing block workers and releases their tokens before shutting down the remote-read pool. Block-worker and asynchronous cache-writer pools start with their configured worker counts; the remote-read pool retains at least one thread. The hole-fill limits are tighter because crossing already covered bytes consumes background bandwidth without reducing the foreground query latency. | Configuration | Default | Meaning | |---|---:|---| | `hole_fill_max_gap_bytes` | 32 KiB | Largest already-covered gap crossed while combining adjacent holes. | | `hole_fill_max_range_bytes` | 1 MiB | Largest block-local hole-fill GET. | | `hole_fill_max_read_amplification_ratio` | 1.0 | Largest ratio of physical GET bytes to missing bytes; the default forbids rereading covered gaps. | Example: all offsets below are KiB within one 1 MiB block. - Covered fragments: `[0, 256)`, `[384, 416)`, `[544, 1024)` - Complement: hole1 `[256, 384)`, hole2 `[416, 544)` - The holes are separated by 32 KiB of covered data. Merging would read 288 KiB for 256 KiB of missing data (1.125 amplification), exceeding the default limit of 1.0. Submit two parallel GETs, `[256, 384)` and `[416, 544)`: 256 KiB total, with no covered bytes reread. ```mermaid flowchart LR C1["covered 0 to 256 KiB"] --> H1["hole1 256 to 384 KiB"] H1 --> GAP["covered gap 384 to 416 KiB"] GAP --> H2["hole2 416 to 544 KiB"] H2 --> C2["covered 544 to 1024 KiB"] H1 --> GET1["GET1: 256 to 384 KiB"] H2 --> GET2["GET2: 416 to 544 KiB"] GET1 --> JOIN["Join both GETs"] GET2 --> JOIN JOIN --> BLOCK["complete block buffer"] BLOCK --> WRITER["existing async cache writer"] ``` #### Failure and compatibility boundaries - The scheduler and hole-fill manager follow the Cloud BE lifetime. New query read-ahead paths require both `enable_query_read_ahead=true` and `enable_async_file_cache_write=true`; runtime updates affect newly initialized reads. Disabling asynchronous cache writes retains the original query reader and cache-filling path. - The legacy segment File Cache prefetcher is skipped only after the new context is successfully created, avoiding duplicate prefetch work. - Planning failure or a completely rejected submission releases the planned pages for the original reader. Partial budget admission registers only accepted ranges and completes skipped pages for synchronous fallback; only admitted, consumed buffers trigger writeback. - A range read failure or cancellation retires its window entries and falls back when PageIO requests the page. - If either enable switch closes during row-store point-read initialization, reuse the loaded segment with the original synchronous iterator. - A checksum or decode failure on prefetched bytes evicts the suspect Page Cache entry and retries through the original reader. - File Cache epoch changes, queue pressure, hole-read failure, and cache-writer rejection discard only best-effort writeback work. - Variant-specific read-ahead forwarding remains outside this PR; existing Variant behavior is preserved. #### Configuration defaults | Configuration | Default | Meaning | |---|---:|---| | `enable_query_read_ahead` | `false` | Dynamically enable cloud query range read-ahead and consumed-range writeback for newly initialized reads; also requires `enable_async_file_cache_write=true`. | | `read_ahead_max_bytes_per_query` | `256 MiB` | Maximum resident range-buffer bytes retained by one query. | | `read_ahead_max_bytes_per_be` | `1 GiB` | Maximum resident range-buffer bytes retained across the BE. | | `read_ahead_eager_window_bytes` | `4 MiB` | Compressed-page bytes selected for each eager-column window; its second selected page triggers the next window. | | `read_ahead_lazy_window_bytes` | `1 MiB` | Compressed-page bytes selected for each dependency-delayed column window, using the same trigger rule. | | `read_ahead_max_gap_bytes` | `64 KiB` | Foreground range-merge gap limit. | | `read_ahead_max_range_bytes` | `2 MiB` | Foreground merged/block-completed range limit. | | `read_ahead_max_read_amplification_ratio` | `1.0` | Foreground merge amplification limit. | | `read_ahead_block_fill_min_coverage` | `1.0` | Minimum requested-page coverage for foreground cache-block completion. | | `hole_fill_max_gap_bytes` | `32 KiB` | Background block-local hole-merge gap limit. | | `hole_fill_max_range_bytes` | `1 MiB` | Background block-local GET limit. | | `hole_fill_max_read_amplification_ratio` | `1.0` | Background hole-fill amplification limit; covered gaps remain unread by default. | | `hole_fill_max_pending_bytes_per_be` | `4 GiB` | BE-wide memory bound for queued and active partial blocks. | | `hole_fill_merge_delay_ms` | `2000 ms` | Mutable minimum time from successful admission before a queued partial block can activate; `0` disables it. Merges keep the original timestamp. | | `hole_fill_workers_per_be` | `32` | Dynamically adjustable block workers for planning, single-range reads, joining multi-range reads, and writeback. | | `hole_fill_remote_read_threads_per_be` | `64` | Dynamically adjustable shared pool size for parallel multi-range GETs; single-range reads execute on block workers outside this limit. | #### Read IO diagnostics `enable_read_io_trace=false` dynamically enables actual S3 GET intervals/outcomes, range/task provenance, read-only cache/inflight coverage snapshots and typed timing events. Records are batched in memory and flushed by a dedicated writer to standalone JSONL files; they do not go to `be.INFO` and have no cumulative event cap. Normal and fast shutdown drain submitted records. | Configuration | Default | Meaning | | --- | --- | --- | | `enable_read_io_trace` | false | Mutable diagnostic switch; leave disabled for performance measurements | | `read_io_trace_dir` | empty | BE log directory's `read_io_trace/` subdirectory, unless explicitly set | | `read_io_trace_flush_bytes` | 1 MiB | Memory-batch flush threshold | | `read_io_trace_flush_interval_ms` | 1000 | Periodic flush interval | `doris_read_io_trace_record_time_ns` exposes diagnostic overhead. The implementation records consumed-range writeback, queued/active fragments and dispatcher scan timing. Profile separates complete/partial foreground submission even with tracing disabled. The offline analyzer under `tools/read_io_trace` is no longer included. S3 upload-buffer configuration updates now enforce the existing File Cache startup alignment constraint: when File Cache is enabled, the buffer must cover at least one cache block and be an exact multiple of its size. Invalid updates return `INVALID_ARGUMENT` and retain the previous value. Cache probing, read-ahead and hole-fill mechanisms are preserved. Writeback deduplication also handles historical non-aligned cache blocks restored from disk. It checks actual contiguous `DOWNLOADED`/`DOWNLOADING` intervals, retaining the inflight shortcut; the existing grid-only `probe()`, read-ahead windows and hole-fill scheduling are unchanged. #### Commit structure The branch is based on upstream master [`fc1b82283cb`](https://github.com/apache/doris/commit/fc1b82283cbbab0ef4e23a8d9249dc3c83d71411) and contains **21 commits: 18 components, one unity-build fix, one S3 configuration validation fix, and one restored-cache coverage fix**. Shared BE diagnostics enter once as component 6; commit 7 scopes filesystem error-code imports for no-PCH builds. The RLE reverse-reach baseline increases only by the feature's six translation-unit users (828 to 834; measured reach: 910 to 916 TUs); existing global expression/reader header dependencies are retained. Hole-fill and consumed-range follow-ups remain in components 15, 17 and 18. | # | Component | Commit | | --- | --- | --- | | 1 | Range coalescer | [ |
||
|
|
ea3558d1ed |
[fix](fe) Stop split enumeration when queries finish or terminate (#69835)
### What problem does this PR solve?
External scans can continue listing files and retain split queues after
a LIMIT query finishes, a query is cancelled, or its deadline expires.
This change makes split enumeration follow the query's lifetime and adds
a configurable first-split deadline with progress in the error message.
| Area | What changes | How it works |
|---|---|---|
| Query lifetime | Stop split work on LIMIT completion, cancellation,
timeout, and failed/replaced planning attempts. | Query- and
attempt-owned scan scopes propagate cancellation; statement resources
remain owned until the associated tasks actually exit. |
| Producer scheduling | A stopped producer cannot be restarted by a late
wakeup or left in the executor queue. | Explicit closing/closed states
prevent resubmission; cancellation removes queued tasks and handles
submission races. Idle/queued sources close on a separate cleanup pool;
a running worker closes its own source before completing its termination
future. |
| Hive/HMS and file listing | Stop requesting more partition metadata,
files, and listing pages after cancellation. | Check the scan context at
metadata RPC/retry, partition, file, and S3 pagination boundaries. Share
completed cache entries while isolating cancellable in-flight loads from
healthy callers. |
| Iceberg SDK planning | Stop owned manifest/data/delete planning jobs
and wait for their actual exit. | A source-owned executor view and
cancellable FileIO reuse the existing SDK pool. Queued jobs settle
independently; SDK bodies exit before iterators and statement resources
close. |
| First split wait | Make the initial wait configurable and report
enumeration progress on timeout. | Add `first_split_wait_timeout_ms`
(default `30000`, minimum `1`, supports `SET_VAR`). The error includes
processed/total partitions, enumerated files, generated splits, and
active producers. This is separate from the existing subsequent
split-fetch timeout. |
| Bounded work | Keep backpressure and allow an explicit producer cap
per scan. | Preserve upstream query-owned fair schedulers and queues
bounded by individual splits. Schedule partition-batch producers as
capacity becomes available. Add `split_producer_parallelism`: `0`
retains pool-derived parallelism; whole-scan streams use one producer. |
Cancellation is cooperative. An already-running SDK operation that
cannot be interrupted may still need to return before its owner can
finish cleanup.
### Release note
External scan split enumeration now stops when its query finishes or
terminates. Add the `first_split_wait_timeout_ms` and
`split_producer_parallelism` session variables. Connector/filesystem
plugin API versions become `26.0`/`7.0`; deploy plugins built with the
matching FE. Native streaming Iceberg planning uses request-local FileIO
without the SDK content cache; catalog FileIO and the Doris manifest
cache are retained.
### Check List (For Author)
- Test:
- [x] Unit Test
- [x] Manual test
- [ ] Regression test (deterministic session-variable suite retained; no
claim of a complete regression run on the latest master)
- Behavior changed:
- [ ] No.
- [x] Yes: query termination stops split scheduling/enumeration and
releases owned resources; first-split waits have an explicit deadline.
- Does this need documentation?
- [ ] No.
- [x] Yes: connector cancellation contract and variables are documented
in `fe/fe-connector/README.md`; user-facing variable reference follow-up
is pending. Manual acceptance/performance tools and evidence remain
local and are excluded from this PR.
| Validation | Result |
|---|---|
| FE compilation and packaging (`00dfe97a`) | Complete `build.sh --fe`
exited 0 with Java style validation. Maven build cache was disabled.
Source/class/package provenance passed for all 36 changed production
sources, 143 classes including nested classes, 24 plugin assemblies,
matching API resources, and the actual Version marker. A separate frozen
package was reverified before benchmark use. |
| Related FE tests (`bfe19cde`) | All 761 selected tests across 61
classes completed with zero failures, errors, or skips through
`run-fe-ut.sh`. The runner subsequently exited 137 while shading the
unrelated BE Java `paimon-s3-shade` module; the complete runner did not
pass. Coverage includes queue/admission races, owner-thread source
closure and actual termination on read/close/completion errors,
query/attempt ownership, cache isolation, and native Iceberg SDK work. |
| Functional observer (`bfe19cde`) | Observer compiled with JDK 17; all
16 report-analysis tests passed. It reads the current split-count queue
structure and rejects the obsolete CSV schema. |
| HMS cancellation fix (`00dfe97a`) | All 246 related HMS/Hive tests
across 19 classes passed with zero failures, errors, or skips; targeted
`run-fe-ut.sh` invocation exited 0 using a per-invocation Maven
module-selection wrapper. Negative control reproduced both pool-wait
cancellation failures before the fix. Coverage includes exhausted-pool
waiters, healthy connection reuse, original total wait budget, creation
failure propagation, authenticated dispatch, names-cache isolation, and
the whole-stream entry path. |
| Independent review (`00dfe97a`) | APPROVE: Blocker/Major/Minor/Nit all
zero. The prior HMS issue is fixed. Main and independent full reviewer
covered all 81 files and 17 checkpoints; all 53 anchors in each language
passed validation. Source review does not substitute for current
package, end-to-end, or performance validation. |
| Baseline package (`2a4191c1`) | Complete `build.sh --fe` exited 0 with
Java style validation and Maven build cache disabled. Independently
verified source/class/package provenance and the actual Version marker;
all five PR-added production classes are absent from all 2,260 deployed
baseline jars. Both comparison packages were frozen with materialized
dependencies before either timed run. |
| Real OSS query validation (`00dfe97a` vs `2a4191c1`) | All 1,440
scheduled queries succeeded: 800 warmups and 640 measured queries, 40
measured samples per version/scenario. No failed or outlier samples
excluded. Aggregations returned the expected COUNT/SUM and each read
exactly 10,000 physical files according to native BE reader counters. |
| Real OSS LIMIT/cancel/timeout cleanup (`00dfe97a`) | 9/9 passed on
10,000 partitions / 1,000,000 files, three repetitions per trigger.
Maximum observed actual exit: LIMIT 90ms, cancellation 135ms, query
timeout 475ms. Queues, producers, source resources, listeners and
callback registrations were released. A separate observer at the OSS SDK
HTTP dispatch boundary saw no new dispatch after 10s during each 12s
observation window. Already-entered transport calls/internal retries are
not separate dispatches in this observer. No instrumentation was loaded
in the performance runs. |
| Real OSS first-split deadline (`00dfe97a`) | Both 1000ms and 2500ms
budgets produced the expected deadline and partition/file/split/producer
progress in the error. End-to-end SQL errors took 1167ms / 2761ms,
including planning before the wait. A deliberately delayed
already-entered SDK operation returned before source closure; cleanup
finished 4133ms / 2608ms after the respective error. These are separate
acceptance cases, not performance measurements. |
| PR scope cleanup (`0e882462`) | Remove all 15 local acceptance-tool
and dependent-suite files (2,003 added lines). Both fixture-dependent
suites and their generated outputs are removed together; no tracked
references remain. All FE implementation and unit-test files are
byte-identical to the reviewed and runtime-tested `00dfe97a`. Retain
only the deterministic session-variable regression (SET, SET_VAR,
restoration and invalid values), which has no storage access or
elapsed-time assertions. |
| Unit test scope cleanup (`c3b37ae5`) | Reduce another 144 lines:
remove the entire JDK CompletableFuture callback test class, a test that
manually constructed two attempt scopes, duplicate failure/queue cases
and constant fake-executor assertions. Combine batching cases with both
partition-batch parameters and all 300 split identities checked. Keep
the actual query-retry path, HMS/SDK cancellation races, resource
lifetime, late-fetch EOF and timeout lower-bound oracles. Independent
test-quality review completed. Focused `run-fe-ut.sh`: all 47 tests
across four classes passed, zero failures/errors/skips, runner exit 0,
Checkstyle explicitly enabled and passed, build cache disabled. The
Iceberg class with only two constant assertions deleted compiled and
passed style; its five tests were not rerun in this focused core
selection. Current PR scope: 64 files, +4,075/-276; production Java is
+1,743/-245 across 36 files. |
| Latest independent skill review (`c3b37ae5`) | APPROVE:
Blocker/Major/Minor/Nit all zero, one round converged. Main and new
independent helper verified 64/64 paths and all 17 checkpoints. Reuse
the completed full review for 60 byte-identical paths; freshly review
four retained test changes and both paths leaving the PR. Preserve 45
prior dismissals and two superseded historical dispositions; retire
three removed-tool dispositions. Both language reports passed 53 anchors
each. Author test evidence was verified; reviewers ran no builds, tests
or cluster operations. The disclosed OSS performance slowdown remains
unresolved. |
| HDFS query validation (historical `60351f1`) | All 488 scans
succeeded: 240 measured queries, 240 warmups, four million-file SUM
validations, and four separate profile queries. Full scans were checked
against native reader counts and expected SUM results. |
| HDFS LIMIT/cancel/timeout cleanup (historical `60351f1`) | 9/9 cases
passed, three repetitions per trigger. Maximum observed exit was 871ms,
measured from LIMIT return, KILL send, or the configured query deadline.
Producers, queues, sources, listeners, and callback registrations were
released; a direct HDFS request observer saw no new listing requests
after 10s. Instrumentation was used for this separate resource test, not
performance timing. |
| Configurable first-split timeout (historical `60351f1`) | Manual waits
of 1000ms and 2500ms failed near their configured deadlines with
progress in the error; invalid nonpositive values are rejected. |
The latest built and runtime-tested revision is
`00dfe97a935d12c268044171c98103be28372edd`, merged with master
`2a4191c1dc94e789e1725d6cb345f46c31a17272`. Current head
`c3b37ae5084ebcbeb16a9387551f7612eeb9ffa6` removes local tools and
redundant tests; all production sources and API-version POMs are
unchanged. Both complete packages, the zero-finding independent source
review at `00dfe97a`, fresh OSS acceptance (11/11), and the four-round
OSS comparison are complete. The PR remains a draft: the OSS
cold-catalog 10,000-partition LIMIT is 3.77% slower by median and 4.05%
by mean in the formal run; attribution remains open. Shared-host load
differs materially, and a separate profiled replay reverses that result
without establishing causality. Historical MinIO slowdowns are also not
resolved. The HDFS figures below refer to `60351f1` on baseline
`d30058a`, before this merge; they are not evidence for the merged head
or object-storage performance.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
### HDFS performance measurements
| Setting | Value |
|---|---|
| Data | 10,000 actual HMS partitions × 100 physical Parquet files =
1,000,000 files; 481 bytes and one row (`id = 1`) per file. |
| Storage | Dedicated HDFS 3.2.1 NameNode/DataNode, with the same data
and server settings throughout. |
| FE comparison | Before: `d30058a80337e56f0f4930d7a75c0df50aa39ffc`;
after: `60351f116ece1427a0a7e04a58869a52dc22a57e`. |
| Common BE | `4.1.3-rc02-7126cf65d96` for both versions. |
| Execution | One online cluster, one query and one producer at a time.
Fresh FE/BE runtimes for each round; order: before → after → after →
before. |
| Repetitions | Fixed 30 warmup cycles, then 20 measured queries per
scenario per round: 40 measured samples per version/scenario. |
| Configuration | Native query/catalog cache and timeout defaults.
Single-producer setting on both sides;
`experimental_enable_local_shuffle_planner=false` and
`be_exec_version=10` on both for BE 4.1.3 compatibility. |
| Cold definition | Fresh FE catalog; HDFS/OS caches were not forcibly
flushed. Timing used no proxy or Java agent. |
In the following tables, `t` is the million-file HDFS table and `p` is
its integer partition key. The 10,000-file aggregation uses `WHERE p <
100`; it is not a separate 100-partition table.
These are observed shared-host measurements, not a guaranteed speedup.
Baseline median latency for the 10,000-file aggregation fell 13.5%
between its two rounds, and CPU affinity was not exclusive. All
successful samples are retained; both medians and means are shown.
Negative changes mean lower elapsed time.
| Scenario / SQL | Samples per version | Before median (ms) | After
median (ms) | Median change | Before mean (ms) | After mean (ms) | Mean
change |
|---|---:|---:|---:|---:|---:|---:|---:|
| Fresh-catalog `SELECT * FROM t LIMIT 10` | 40 | 863.56 | 774.17 |
-10.35% | 905.38 | 813.43 | -10.16% |
| Warm-catalog `SELECT * FROM t LIMIT 10` | 40 | 114.20 | 109.90 |
-3.77% | 124.91 | 115.60 | -7.45% |
| `SELECT COUNT(*), SUM(id) FROM t WHERE p < 100` (10,000 files) | 40 |
1167.90 | 1052.15 | -9.91% | 1404.32 | 1145.69 | -18.42% |
Per-round results expose drift. Million-file SUMs are separate
correctness/workload validations, one per runtime, rather than repeated
steady-state performance samples.
| Round | Version | Fresh-catalog LIMIT median (ms) | Warm-catalog LIMIT
median (ms) | 10,000-file aggregation median (ms) | Million-file SUM
validation (s; result = 1,000,000) |
|---|---|---:|---:|---:|---:|
| 1 | Before | 989.08 | 116.32 | 1273.19 | 282.615 |
| 2 | After | 753.10 | 99.75 | 1052.15 | 232.395 |
| 3 | After | 785.57 | 115.46 | 1061.70 | 232.503 |
| 4 | Before | 781.45 | 102.26 | 1101.28 | 238.465 |
### Real OSS performance measurements (current merged head)
| Setting | Value |
|---|---|
| FE comparison | Before: master
`2a4191c1dc94e789e1725d6cb345f46c31a17272`; after:
`00dfe97a935d12c268044171c98103be28372edd`. Both complete FE builds and
independently verified frozen packages. |
| Common BE | `4.1.3-rc02-7126cf65d96`, identical binary on both sides.
|
| Data/storage | Direct real OSS endpoint, 1,000,000 verified physical
Parquet objects, 481 bytes and one row (`id=1`) each. Three HMS tables
expose 100 / 1,000 / 10,000 partitions × 100 files. Smaller tables reuse
the corresponding subset of the same object paths. Local Docker HMS. |
| Execution | One online FE + two BEs, one SQL query and one split
producer at a time. Fresh runtime each round; after → before → before →
after. Each process pinned to its own four CPUs; FE heap 4GiB, each BE
memory 3GiB. CPU affinity is not exclusive on this shared host. |
| Repetitions | Fixed before formal measurement: 30 LIMIT warmup cycles
plus 10 full-scan warmups per aggregation, then 20 measured queries per
scenario per round. 800 warmups + 640 measured, all successful; 40
measured samples per version/scenario. |
| Split/cache configuration | Current defaults on both sides: batch 8,
queue 1024 splits, initial batch 64, partition batch 16, batch
thresholds 1, fetch wait 1000ms. One split/metadata worker; PR producer
cap 1. Native BE file cache is disabled (default); FE session file-cache
variable is enabled (default). SQL cache enabled, Hive SQL cache
disabled. |
| Compatibility settings |
`experimental_enable_local_shuffle_planner=false`, `be_exec_version=10`
on both sides for the common BE 4.1.3. |
| Timing boundary | SQL client wall time includes query execution.
Fresh-catalog cold cases recreate the FE catalog; HMS/OS caches are not
flushed. Required native scheduler/reclaimer queues and BE readers drain
before the next query, outside the SQL timer. No Java agent, JFR,
profiler, replay or proxy in these timed runs. |
| Correctness | Full and filtered-full queries each returned COUNT/SUM
10,000 and each incremented the native S3 physical file-reader counter
by exactly 10,000. |
| Qualification | All attempts retained. Shared host average load was
approximately 132 for PR samples and 162 for baseline samples.
Consequently this is an observed comparison, not proof of a causal
improvement or absence of regression. |
The 10,000-partition cold LIMIT slowdown appears in both adjacent formal
round pairs (+5.45% and +3.04% median). Its FE CPU usage is nearly
unchanged (237.5ms before / 238.0ms after, mean process CPU per query).
A separate post-timing JFR replay (20 samples/version/scene) instead
observed cold LIMIT 481.51ms before / 430.32ms after, with average host
load about 398 / 131. That replay is instrumented, unbalanced, and
excluded from the table below; it does not explain away the formal
slowdown. Sampled common costs include partition-view construction,
range construction, URI parsing, HMS reads and HTTP reads; the available
evidence does not identify a causal extra 13ms in this PR. Further
attribution requires controlled host contention. The 100-partition warm
LIMIT median is also reported as slower, while its mean is lower;
neither statistic is hidden.
Native FE audit logs reconcile all 1,440 query markers exactly once,
including phase timings for all 640 formal queries. For the
10,000-partition cold LIMIT, mean client time increases 14.11ms; mean
native planning time increases 12.53ms (216.60 → 229.12ms), including an
8.23ms increase in analysis (96.30 → 104.53ms). Mean
`finalize_scan_node` time decreases 2.17ms (58.50 → 56.33ms). These
nested stage times localize the observed difference mostly to planning;
they must not be added together or treated as a causal explanation of
the shared-host difference.
Negative changes mean lower elapsed time. `t100`, `t1000`, and `t10000`
denote the corresponding OSS table scales.
| Partitions | Scenario / SQL | Samples per version | Before median (ms)
| After median (ms) | Median change | Before mean (ms) | After mean (ms)
| Mean change |
|---:|---|---:|---:|---:|---:|---:|---:|---:|
| 100 | Fresh-catalog `SELECT * FROM t100 LIMIT 10` | 40 | 172.70 |
156.51 | -9.37% | 179.45 | 167.35 | -6.74% |
| 100 | `SELECT COUNT(*), SUM(id) FROM t100` (10,000 files) | 40 |
1537.61 | 1477.29 | -3.92% | 1553.59 | 1535.61 | -1.16% |
| 100 | Warm-catalog `SELECT * FROM t100 LIMIT 10` | 40 | 40.58 | 42.50
| +4.71% | 46.99 | 45.80 | -2.54% |
| 1000 | Fresh-catalog `SELECT * FROM t1000 LIMIT 10` | 40 | 266.82 |
266.11 | -0.26% | 268.92 | 273.72 | +1.78% |
| 1000 | Warm-catalog `SELECT * FROM t1000 LIMIT 10` | 40 | 125.00 |
122.20 | -2.24% | 125.96 | 124.64 | -1.04% |
| 10000 | Fresh-catalog `SELECT * FROM t10000 LIMIT 10` | 40 | 347.26 |
360.37 | +3.77% | 347.94 | 362.05 | +4.05% |
| 10000 | `SELECT COUNT(*), SUM(id) FROM t10000 WHERE p < 100` (10,000
files) | 40 | 1563.96 | 1541.16 | -1.46% | 1599.73 | 1569.72 | -1.88% |
| 10000 | Warm-catalog `SELECT * FROM t10000 LIMIT 10` | 40 | 133.35 |
123.73 | -7.21% | 132.67 | 130.09 | -1.95% |
|