mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
cb0b56d408a76a59e5b7d248071907537796e47e
32971
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Jamie
|
cb0b56d408 |
[fix](cloud) Fix deadlock between tablet encryption check and meta lock writers (#69950)
### What problem does this PR solve? Issue Number: None Related PR: #64574 Problem Summary: `is_tablet_encrypted()`, which serves `/api/check_tablet_encryption`, holds the tablet `_meta_lock` read lock in `traverse_rowsets()` and takes it again through `tablet_schema()` in the visitor. Since #64574 `_meta_lock` is the writer-preferring `BthreadSharedMutex`, so a writer that arrives between the two reads deadlocks with the check: - The check holds the first read lock and waits for the second, which is blocked by the pending writer. - The writer waits for the first read lock to be released. The window is wide because each rowset's files are read from remote storage under the lock, and compaction and rowset sync take the write lock often. Once this happens the tablet stays stuck until the BE restarts: - The HTTP request never returns. - The compaction keeps renewing its lease but never finishes, so other BEs get `JOB_TABLET_BUSY` for this tablet. - Every later reader of this tablet's meta blocks behind the pending writer, including the periodic tablet sync. We hit this in a regression test: the compaction stopped right after its rowset sync, and the check timed out on all 10 retries over about 2h47m. This PR reads the schema once before the check and checks a snapshot of the rowsets (`get_snapshot_rowset()`) with no lock held, which also stops holding the meta lock across remote file reads. It adds a test-only sync point so a unit test can observe the lock at that moment. ### Release note Fix a deadlock between the tablet encryption check API and tablet meta writers such as compaction, which could block a tablet until the BE restarts. ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] 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 <!-- Add your reason? --> Unit test `CheckEncryptionActionTest.RowsetsAreCheckedWithoutHoldingMetaLock` checks that `_meta_lock` is free while a rowset is being checked. It fails on the code before this PR and passes with it. Manual test: a temporary unit test (not committed) started a writer from inside the rowset check, then waited for the check to return. On the code before this PR the check had not returned after 20s. With this PR it returned at once. - Behavior changed: - [x] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [x] No. - [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 |
||
 daidai
|
61906ce9a1 |
[fix](be) Preserve signed decimal bounds in Iceberg ORC writes (#69888)
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
ORC `Int128` statistics already contain two's-complement bits, but the
Iceberg ORC writer negates negative values again before encoding them. A
file containing `[-100, -1]` consequently reports bounds `[100, 1]`, so
a negative predicate can prune a file that contains matching rows.
ORC footer statistics also trim trailing fractional zeros. For example,
`-1.00` is returned as `value=-1, scale=0`; a `DECIMAL(...,2)` manifest
needs the unscaled value `-100`. Preserving the sign alone therefore
leaves incorrect bounds.
Restore each bound to the declared column scale with the existing ORC
scaling utility, then serialize its original signed bits using minimal
big-endian encoding. Preserve necessary sign prefixes such as `128 -> 00
80` and `-129 -> FF 7F`.
The fix applies to newly collected bounds. Existing incorrect metrics
require recomputation or data-file rewriting.
### Release note
Correct decimal bounds emitted by Iceberg ORC writes to prevent
incorrect predicate pruning for negative values and values with trailing
fractional zeros.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [x] 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
Added BE tests cover 29 encoding cases, real ORC statistics at
precisions 1/9/18/38, scale restoration through scale 38, nullable
columns, and all-null columns. The external regression checks manifest
pruning for negative and mixed decimal data across decimal32/64/128.
Local validation:
- An isolated codec harness built from the production function passed
all 29 encoding cases.
- Five real ORC files passed independent Iceberg 1.12.0 manifest
write/read and predicate-pruning probes.
- clang-format 16, build hygiene, changed-line clang-tidy, and Groovy
4.0.19 syntax checks passed. Static analysis used real ASAN UT
compilation commands with PCH expanded as headers.
- The full local BE-UT build was stopped during the monolithic
dependency build before tests executed.
- Regenerated `test_iceberg_write_stats2.out` with the standard
regression runner after restoring decimal scales. Only `sql_4/5/7/9`
changed, and the decimal bounds match both stored rows and the Parquet
metrics.
- Re-ran `test_iceberg_write_stats2` and
`test_iceberg_orc_decimal_bounds` locally using this PR's CI FE/BE
artifacts: 2 suites passed, 0 failed, 0 skipped.
- Behavior changed:
- [ ] No.
- [x] Yes. Newly written ORC decimal bounds preserve the sign and the
column scale.
- 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
|
||
 wudi
|
deb079478d |
[fix](fe) Report missing backend during stream load planning (#69989)
### What problem does this PR solve? Issue Number: None Problem Summary: A cloud Stream Load request can still arrive from a BE after scaling has removed that BE from FE metadata. Resolving the compute group then obtains a null backend and throws a NullPointerException, which reaches the caller as a generic INTERNAL_ERROR. Return a descriptive UserException containing the missing backend ID instead. The existing RPC exception handler converts it to ANALYSIS_ERROR. The request fails before plan generation, allowing the existing CDC task failure and automatic rescheduling flow to handle recovery. Add an RPC-level test for a missing backend with token authentication and a compute-group header. It checks the returned status and backend ID, absence of an execution plan, and ConnectContext cleanup. |
||
 lihangyu
|
a37fc36a0f | [opt](variant) Walk only non-null materialized slots per row in VariantAssembler (#69914) | ||
 minghong
|
156dbe71dc |
[fix](MV) Accept constant query outputs when matching a materialized view (#69787)
### What problem does this PR solve? Issue Number: None Problem Summary: The materialized view rewrite keeps a query output which is a **constant** as it is, because the view does not have to output that column, but it only recognized a bare `Literal`. A constant alias such as `1 AS o_orderkey#97` (the shape constant propagation materializes above an aggregate) or a constant function such as `bin(1)` did not reach that path, so the rewrite looked for a counterpart in the view, did not find it, and rejected the candidate. The explain then reports `... not chose` and the query does not use the view even though the columns are pinned by predicates. Accept every constant expression there (`isConstant()` covers `Literal` and constant aliases/functions) so a constant query output needs no counterpart in the view. Aggregate functions over literals (`count(1)`) are not constant by definition and keep the previous behavior. ### Release note None ### Check List (For Author) - Test: - Unit Test: the materialized view unit tests (`MappingTest`, `MaterializedViewUtilsTest`, `MvExplorationSuiteTest`, `AsyncMaterializationContextTest`, `PartitionColumnTraceTest`) pass - Regression test: the whole `nereids_rules_p0/mv` directory. `Explain and check failed` does not appear: no materialized view rewrite case regressed. 10 suites of that directory fail in this local environment for unrelated reasons: 5 stream load failures (empty response body), 4 external table suites which cannot reach a Hive metastore, and 1 outfile suite without S3 credentials. - Behavior changed: No - Does this need documentation: No |
||
 Jerry Hu
|
4088359429 |
[fix](be) Reset quantile union state for empty window frames (#69964)
### What problem does this PR solve? Issue Number: N/A Related PR: N/A Problem Summary: A bounded ROWS window can become empty after a nonempty frame. The `quantile_union` reset previously restored only the first-input flag and retained the preceding `QuantileState`. Without a subsequent value to merge, an empty or NULL-only frame returned the preceding frame's quantiles. Reset both the first-input flag and the stored samples before each frame. The logical reset retains explicit sample-vector capacity for reuse, while `clear()` preserves its storage-release behavior. ### Release note Fix `quantile_union` over empty window frames: `quantile_percent` now returns NaN instead of values from the preceding frame. ### Check List (For Author) - Test: ASAN BE unit tests (47 passed); ASAN BE+FE build (including FE Checkstyle); three isolated SQL regression suites (1 suite passed each); clang-format, format check, build hygiene, changed-line clang-tidy (no warnings), and diff check. No performance benchmark was run. - Behavior changed: Yes (empty window frames no longer inherit the preceding quantile state). - Does this need documentation: No. |
||
|
daidai
|
952ed6904f |
[fix](be) Preserve Unicode across JNI string conversions (#69889)
### What problem does this PR solve?
Issue Number: N/A
Related PR: N/A
Problem Summary:
JNI `NewStringUTF` and `GetStringUTFChars` use modified UTF-8, while
native scanner/writer parameters and paths use standard UTF-8. Passing a
SQL predicate containing an emoji to `NewStringUTF` can corrupt or
truncate it before the JDBC driver receives it. Returned maps also
encode supplementary characters differently and cannot preserve a
standard UTF-8 round trip.
- Add validated standard UTF-8 ↔ UTF-16 conversion helpers and use them
for both directions of parameter/statistics maps. Preserve supplementary
characters and embedded NULs; reject malformed UTF-8 and unpaired UTF-16
surrogates with an input offset.
- Migrate Paimon writer text, column/directory arrays, spill-path
callbacks, and exception messages to these helpers. Reuse one array
builder and preserve native spill-path matching and accounting.
- Use standard UTF-8 for dynamic UDF cache signatures and plugin status
JSON as well.
- Use native temporary conversion buffers that do not require a Doris
`ThreadContext`, and return native allocation failures as `Status`.
Regressions exercise JNI-attached threads without a Doris context,
including spill callbacks and accounting.
- Fix the existing `String<Global>::new_string` template instantiation
and keep the legacy modified UTF-8 APIs' encoding contract.
### Release note
Fix Unicode corruption in JNI parameters, returned maps, and Paimon
metadata and spill paths. Invalid text encodings now fail explicitly.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason
Validation: Focused ASAN_UT build and execution via `./run-be-ut.sh
--run
--filter='JniUtf8DeathTest.*:JniUtf8Test.*:JniPaimonUtf8DeathTest.*:JniPaimonUtf8Test.*:JniPaimonWriteBackendTest.*:PaimonWriteBackendFactoryTest.*:PaimonWriteBackendTest.*:PaimonTableSinkOperatorTest.*:PaimonTableWriterTest.*'
-j 6`: **23 passed, 0 failed, 0 skipped**. This includes 13 real-JVM
subprocess tests, 3 JNI allocation-failure tests, and 7 existing Paimon
backend tests. The local test source list was temporarily narrowed;
production libraries and ASAN configuration were retained, and the CMake
selection was restored afterward. Full BE/regression suites, complete
Paimon writes, and the customer-specific JDBC environment were not run.
Style/static checks: clang-format 16, build hygiene, and clang-tidy
checks passed.
- Behavior changed:
- [ ] No.
- [x] Yes. JNI text boundaries preserve standard UTF-8; malformed input
is rejected instead of being silently corrupted. Existing modified UTF-8
APIs retain their contract.
- 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
|
||
 hui lai
|
31a036fbcd |
[fix](load) Avoid warning spam when filtering non-local adaptive bucket tablets (#69979)
### What problem does this PR solve? During adaptive random bucket loading, `VNodeChannel::_set_adaptive_random_bucket_open_request()` normally skips selected tablets that do not belong to the current node channel. Logging a WARNING for every skipped tablet can flood both INFO and WARNING logs when many partitions and parallel writers are involved, increasing disk I/O and evicting useful diagnostic history. Change this expected filtering message to `VLOG_DEBUG`, which is disabled by default. Keep the existing diagnostic fields, tablet selection logic, and warnings for unknown partitions, unknown indexes, and invalid bucket sequences unchanged. ### Release note Avoid excessive BE INFO/WARNING logging when adaptive random bucket loading skips non-local tablets. |
||
 Gabriel
|
156bae0793 |
[fix](be) Read dictionary-encoded Parquet BOOLEAN columns (#69967)
### What problem does this PR solve?
Issue Number: N/A
Problem Summary:
The native Parquet reader rejects every BOOLEAN dictionary page,
although PLAIN dictionaries with RLE_DICTIONARY or legacy
PLAIN_DICTIONARY data pages are valid. Reading a nullable BOOLEAN
interoperability fixture fails with `Boolean type cannot have a
dictionary page`.
Create the existing fixed-length dictionary decoder for BOOLEAN and
expand its LSB-first packed entries into tracked, contiguous one-byte
values for SerDe. Validate the packed extent before allocation and
preserve the common dictionary-index checks for reads, selections, and
skips.
The 257-byte interoperability fixture and synthetic tests cover byte
boundaries, invalid extents, dictionary replacement, both dictionary
encoding enums, V1/V2 pages, skipped/filtered values, and
dictionary-to-PLAIN transitions.
### Release note
Support reading dictionary-encoded BOOLEAN columns from Parquet files.
### Check List (For Author)
- Test
- [x] Regression test: S3 TVF reads compare against SQL literal rows,
covering predicates, aggregates, NULL alignment, and lazy
materialization on/off. Groovy compilation and a suite control-flow
check using the real FE variable manager pass, including settings
restoration after success or an injected failure; full SQL execution is
pending CI.
- [x] Unit Test: ASAN BE tests; all 199 tests in
`ParquetV2NativeDecoderTest.*:NewParquetReaderTest.*` pass. The five new
tests fail before the fix and pass after it.
- [ ] Manual test
- Behavior changed:
- [x] Yes. Valid BOOLEAN dictionaries are readable instead of being
rejected.
- Does this need documentation?
- [x] No. Compatibility fix for existing Parquet reads.
|
||
|
Gabriel
|
8ebf20dc3a |
[feature](fe) Support MongoDB ObjectId and Trino JSON types (#69969)
### What problem does this PR solve?
Issue Number: N/A
MongoDB exposes ObjectId as a plugin-defined Trino type. Doris did not
register these types in either plugin loader, rejected ObjectId during
schema mapping, and discarded the connector's default hidden `_id`
column. Register plugin-defined types in FE and BE, map ObjectId
recursively to `VARBINARY(12)`, and expose the default MongoDB `_id`
while retaining the hiding of other columns. Existing byte readers
preserve all twelve bytes without UTF-8 conversion.
Trino JSON columns also lacked a Doris type mapping. Map the built-in
Trino JSON type recursively to native Doris JSON, and parse its UTF-8
JNI carrier into JSONB. Preserve SQL NULL independently of the JSON
value `null`, including empty and repeated batches. Reuse the existing
JSON parser and return malformed-input errors. Correct nested JSON text
serialization so arrays, maps and structs contain JSON values with their
original quoting; previously an object could become invalid text such as
`["{"a":1}"]`. Coordinate typed ARRAY/MAP/STRUCT text parsing with that
formatter, retaining JSON string quotes and using uppercase `NULL` for
SQL-null JSON children versus lowercase JSON `null`. Both strict and
non-strict casts retain the original JSON type and null semantics.
Delegate complete child tokens to `from_string_as_nested`: scalar SerDes
remove outer quotes, JSONB retains JSON quotes and accepts legacy SQL
apostrophe wrappers, and Nullable handles SQL nulls and conversion
errors using the child's paired null-token policy. Generic complex
parsing no longer queries the concrete JSONB type.
Require the fixture's final network-facing MongoDB process before
marking it ready, and use bounded tmpfs mounts for both image-declared
data directories to avoid orphaned anonymous volumes.
Add an opt-in MongoDB 8.0.20 Docker fixture and five regression suites
with **138 result checks and ten expected-error checks**. Coverage
includes primitive and nested types, BSON documents and Extended JSON,
all JSON value shapes, null and missing fields, numeric/date boundaries,
Unicode and escapes, schema discovery/permissions, scalar and JSON
predicates, joins, aggregates, subqueries, windows, views,
internal-table joins, scalar JSON materialization, multi-page reads and
repeated early termination. Golden files are generated and compared with
the standard regression runner. Source-side MongoDB queries
independently confirm critical counts and null semantics.
MongoDB remains an operator-installed Trino 435 plugin; no MongoDB
runtime dependency is added to Doris. Use the complete matching plugin
ZIP on FE and BE.
ObjectId retains existing VARBINARY operator/storage restrictions; use
`from_binary(_id)` for hexadecimal filtering, joining, grouping or
materialization. Nested JSON is readable through catalogs, while
internal-table creation retains existing complex-type restrictions,
including rejecting `ARRAY<JSON>`. BSON documents inferred automatically
remain ROW/STRUCT; use an explicit `json` schema declaration to read a
field as a whole JSON value.
### Validation
- FE package and native ASAN BE built through `build.sh --fe` /
`build.sh --be`. UI, Hive UDF, OBS/COS dependencies and unrelated Java
plugins were excluded locally; the native build covers the initial
connector and JSON changes. The final SerDe interface refactor was
compiled and linked through the ASAN BE unit-test build; the standalone
BE package and external integration suites were not rerun for that
refactor. Final executable passed the GLIBC_2.17 compatibility gate.
- `run-be-ut.sh --run`: **33 related BE unit tests passed**, including
existing JSON serde, general complex-type parsing and formatting tests.
Typed JSON text covers 66 combinations of container, value shape and
strictness, plus 48 legacy apostrophe-wrapper combinations, deeper
nesting and malformed children in both modes. Reverting JNI support
reproduces three failures; the typed complex JSON test fails before the
reader fix and passes after it.
- `run-regression-test.sh --run -d
external_table_p0/trino_connector/mongodb`: **five suites passed**, with
**138 executed result checks and ten expected errors**, real MongoDB and
the complete Trino 435 plugin ZIP.
- Checkstyle/validate, clang-format 16 for all affected C++ files,
header hygiene and clang-tidy passed.
- **17 related regression suites passed** across the final comparison
and an Elasticsearch rerun after adjusting only local fixture
CPU/direct-memory limits. This includes the array-function
documentation, JSON/array/aggregate coverage, all four Elasticsearch
fixture versions, spill-enabled bucket-shuffle plans, HTTP TVF and FE/BE
version metrics.
- The generated-version test verifies refreshed numbers without
recompiling a cached consumer. Warmed FE builds verify changed and
restored release metadata after cache hits; bytecode inspection confirms
numeric fields are read at runtime.
- Gated MongoDB startup reproduces and fixes the initialization-server
readiness race; repeated start/stop cycles leave no anonymous data
volumes.
- Independent MongoDB queries confirmed source-side null semantics and
multi-page counts/sums.
Refresh the affected array-function documentation and Elasticsearch
golden results through the regression runner. Keep the bucket-shuffle
regression focused on physical INTERSECT/EXCEPT nodes when spill is
enabled by disabling their two spill rewrite rules. Pin the HTTP TVF
INT96 fixture timezone to UTC to preserve its stored wall-clock values
and existing golden results.
Refresh the FE build-version class after Maven cache restoration so
packaged FE/BE version metrics report the current revision. Keep numeric
version fields runtime-initialized so cached consumers observe refreshed
metadata. A standalone generator test covers replacing Version.class
without recompiling its consumer and invokes the actual packaging
function with a PATH-only JDK to verify both tool selection and
deployed-jar metadata.
The fixture README documents setup, JSON schema behavior, typed SQL text
null spelling, coverage and current limitations. MongoDB integration
suites are disabled by default and require the matching plugin on FE and
BE.
### Release note
Support MongoDB ObjectId columns, including the default `_id` and nested
fields, as `VARBINARY(12)` through Trino connector catalogs. Support
Trino JSON columns as native Doris JSON, including nested JSON fields,
and preserve JSON values and SQL-null distinctions through typed
complex-text round trips.
### Check List (For Author)
- Test
- [x] Regression test
- [x] Unit Test
- [x] Manual test: real MongoDB fixture, complete connector ZIP, exact
bytes and independent source-side JSON checks
- Behavior changed:
- [x] Yes. ObjectId and JSON fields become queryable with their native
Doris types, the default hidden MongoDB `_id` is exposed, and nested
JSON values retain their own quotes and null semantics.
- Does this need documentation?
- [x] Yes. Setup, JSON schema selection and limitations are documented
in `docker/thirdparties/docker-compose/mongodb/README.md`.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
|
||
|
daidai
|
3763f09897 |
[fix](fe) Skip table snapshot resolution for external views (#69836)
### What problem does this PR solve?
External views currently resolve a table handle when loading the
statement snapshot, before their SQL is expanded. Skip that table-only
path for known views; their base tables retain normal snapshot pinning.
Reject unsupported view scan parameters such as `@branch` and `@tag`
instead of silently ignoring the requested selector.
### Release note
Avoid table metadata requests for external views during snapshot
binding, and reject unsupported view scan parameters.
### Check List (For Author)
For this update: Checkstyle passed for both changed Java files.
Selector-rejection unit tests were added but not run locally.
- Test
- [x] 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
|
||
  zhaorongshengandzhaorongsheng
|
bdb90856e6 |
[improvement](hive) Push partition filters to HMS (#67725)
## What problem does this PR solve? Push eligible Hive partition equality/IN predicates into bounded HMS filtering without eagerly materializing every partition. Preserve conservative fallback for unsupported predicates, types, identifiers and unavailable views. This update addresses the current review findings: - Reuse filtered handles without losing stronger backend residual predicates. - Prepare complete partition views for MV creation and eligible refresh/rewrite consumers. - Distinguish unavailable coverage from native/all coverage during MV compensation. - Fence ROOT/ADMIN selective subsets against schema and connector changes. - Restrict filtered, unfiltered and empty refresh reads to their materialized pin. - Preserve connector-retained schema through deferred pin hydration. - Keep INSERT preloading outside the outer target lock; retry schema or target replacement before binding with refreshed table caches. - Avoid pre-binding constant folding of unbound CAST predicates. - Handle reserved Hive partition keys `in` and `between` through local fallback. - Display unknown full partition totals as `?` without enumerating solely for EXPLAIN. - Resolve the latest target integration conflict by preserving both partition-pruning and write-only-default capabilities and advancing the connector API to 28.0. ## Release note Improve selective Hive partition planning while preserving schema, snapshot and materialized-view consistency. Selective EXPLAIN output may show an unknown full partition total as `?`. ## Validation - Published head: `ae2f21cbb30469408970cfb81709d8bbe27a8998`. - Target snapshot integrated without rewriting published history: `0976b2cd5020607205969bf7986dcb551b6a7325`. - Before the final target integration, canonical `./run-fe-ut.sh --coverage --run ...` runs verify 39 selected classes: 945 tests, zero failures/errors, one existing disabled test. - After the final target-replacement correction, a fresh targeted canonical run passes 33 tests, zero failures/errors/skips, and 89 actual zero-violation Maven Checkstyle executions. - After the final target integration, fresh canonical reports verify 36 classes and 642 tests, zero failures/errors/skips. This includes SPI's own full suite, Hive/Iceberg, view/MVCC, planning and partition compensation, INSERT/defaults, MTMV task fixtures and SQL-cache version tests. - The first integration run passes 641 tests but exits 1 when the final mock-FE class hits an occupied ephemeral edit-log port before executing its test. The isolated single-method rerun passes the remaining test, exits 0 with `BUILD SUCCESS`, and performs 89 actual zero-violation Checkstyle executions. These results are aggregated from fresh reports, not a claim that the all-class reactor succeeded in one attempt. - Coverage includes connector/HMS/SPI, MVCC schema and partition views, sync/stream/batch handoffs, MV compensation, policy/replan boundaries, INSERT lock windows and real target replacement in both execution and planning-only modes. Previously failing CAST/shape classes are included. - Connector-import, metadata-funnel, build-hygiene and authored whitespace checks pass. - Local mock-FE port conflicts required targeted reruns. A temporary test-only non-ephemeral port selection was used and restored before committing. - Standard local `./build.sh --fe` packaging remains blocked before Maven by missing genuine native thirdparty prerequisites. No successful full packaging, live HMS/BE regression or complete refresh-scheduler run is claimed. - Extended review rounds were stopped at the author's request. All independently confirmed remaining findings are repaired and tested; no pipeline PASS receipt or new remote `/review` request is issued by this update. --------- Co-authored-by: zhaorongsheng <zhaorongsheng@corp.netease.com> |
||
 Jack
|
7cca01bc6c |
[fix](be) Score MATCH on keyword inverted indexes like SEARCH (#69790)
### What problem does this PR solve? Issue Number: None Related PR: #69051 Problem Summary: score() of a MATCH on an untokenized (keyword) inverted index is always 0, while SEARCH on the same field scores BM25: INDEX idx_redirect (redirect) USING INVERTED -- one row ('BP') SELECT score() FROM t WHERE redirect MATCH 'BP'; -- 0 SELECT score() FROM t WHERE SEARCH('redirect:BP'); -- 0.2876821 Elasticsearch scores a match, term or match_phrase query on a keyword field with BM25 in query context: no norms and no frequencies, so freq = dl = avgdl = 1 and the score is the idf of the whole value (0.2876821 above, 0.6931471 for N=4, n=2). Only a bool filter scores 0. SEARCH already gives exactly these values. MATCH on keyword behaved like a filter, so `body MATCH 'apple' AND tag MATCH 'x'` ranked as if the tag clause were not there. Root cause: #69051 made the engine score keyword postings (unit frequency and norm) but kept MATCH out on purpose: can_score_match required an analyzed index, and MatchPredicateCollector returned early for indexes without an analyzer, so no statistics were collected for them. Fix: a keyword (non-analyzed) index is eligible for MATCH scoring regardless of support_phrase, and MatchPredicateCollector collects its whole value as one term (case and spaces kept), the same term the reader looks up and the same as SearchPredicateCollector collects. Analyzed indexes keep the existing support_phrase / lowercase-normalizer rule. Equality and IN stay pure filters. A keyword MATCH_PHRASE_PREFIX scores the constant 1 of a single-term prefix; MATCH_REGEXP is unchanged. |
||
 Mryange
|
4bc7f42682 |
[fix](be) Fix TIMESTAMPTZ schema change defaults (#69611)
### What problem does this PR solve?
Issue Number: N/A
Problem Summary:
Adding a TIMESTAMPTZ column with DEFAULT CURRENT_TIMESTAMP could finish
schema change but leave historical rows unreadable. Root cause: the
backfill default was materialized without timezone information, and the
batch ADD COLUMN and Protobuf paths did not consistently use the
materialized value.
Materialize TIMESTAMPTZ backfill defaults as UTC literals, use the
schema-change conversion only for TIMESTAMPTZ/TIMESTAMP_NS in batch ADD
COLUMN, preserving other types' existing conversion, and preserve the
default expression in both Protobuf and Thrift. Partial updates evaluate
the preserved expression for new rows instead of reusing the fixed
backfill timestamp.
### Release note
Fix unreadable historical TIMESTAMPTZ values after adding a column with
DEFAULT CURRENT_TIMESTAMP.
### Check List (For Author)
- Test:
- [x] Regression test: test_timestamptz_default_schema_change passed
with -forceGenOut and again normally (1 suite, 0 failures each),
including TIMESTAMP_NS backfill and TIMESTAMP_NS/DATETIME partial-insert
checks.
- [ ] Unit Test: FE coverage added and compiled, not executed
separately.
- BE ASAN build passed for the BE changes. FE rebuilt successfully after
the follow-up. `git diff --check` passed. C++ formatting checks passed.
Clang-tidy reported existing diagnostics outside modified lines; the
full scan was not clean.
- Behavior changed:
- [x] Yes. Historical rows receive readable TIMESTAMPTZ defaults;
subsequent writes retain dynamic CURRENT_TIMESTAMP evaluation.
- 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
|
||
 yujun
|
c9b73d2947 |
[fix](ivm) Plan EXPLAIN REFRESH COMPLETE with the read mode a refresh reads (#69851)
`EXPLAIN REFRESH MATERIALIZED VIEW mv COMPLETE` for an IVM materialized view did not show the plan the refresh actually runs. The explain path installed `IvmRewriteContext.fullExplain`, which carried no stream read mode, so `hasFullRefreshStreamScans()` was false and `IvmFullRefreshMTMV` never rewrote the base-table scans. The statement therefore printed the normalized MV query over ordinary base-table scans, while a real COMPLETE refresh reads its base tables through IVM table streams in RESET mode (the first batch of a whole-MV refresh, and every table it does not partition by). That rewrite reshapes the plan, so the explained plan and the executed plan differed in exactly the part EXPLAIN is used to debug. Problem Summary: - Root cause: `IvmRewriteContext.fullExplain` passed an empty reset partition map and an empty non-PCT read mode, so the full-refresh stream rewrite rule was gated off during EXPLAIN. - Fix: the explain context now carries `StreamReadMode.RESET`, and the full-refresh rewrite reads every stream-scanned base table — including the tables the MV partitions by — in that mode under EXPLAIN, over the partitions the plan selects. - A base table without a usable IVM stream now fails the statement instead of being explained as a plain scan. EXPLAIN plans the same read the refresh runs, and the refresh fails the same way (a real COMPLETE refresh reconciles the stream before planning; EXPLAIN must not create one). Notes: - A complete refresh is not one plan: it may be split into batches, and the later batches read non-PCT tables at the stream offset (SNAPSHOT) and read tables the MV partitions by as ordinary scans. One EXPLAIN statement stands for the refresh as a whole, so it shows the read the refresh starts with — RESET for every stream read — and no reset partition scope is resolved for the explain plan. - No execution state is touched. EXPLAIN never builds an insert transaction, so no stream offset advances; the only added work at plan time is reading stream metadata. |
||
 Calvin Kirs
|
0976b2cd50 |
[opt](sql-cache) Preserve SQL cache across unrelated partition writes (#69883)
### What problem does this PR solve? Issue Number: None Problem Summary: SQL cache currently invalidates an OLAP query whenever the table visible version changes. For example, a cached query reading historical partition p1 is discarded after an insert into unrelated partition p2, even though its result is unchanged. Record the partition versions actually assigned to the scan, including selected empty partitions removed during planning. When the table version advances, reuse the cache only if those dependencies and the schema/partition topology still match. Use the same stable dependency identity for the BE cache key. Keep table-wide validation for metadata aggregates, variable-dependent keys and materialized-view rewrites whose physical scans can hide source dependencies. The change is FE-only and preserves the existing BE cache protocol. Regression cases use only 3–5 source rows and cover unrelated/relevant writes, empty partitions and branches, self joins, partition DDL, partial MV rewrites and eliminated empty MVs. |
||
 shuke
|
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. |
||
 Mingyu Chen (Rayner)
|
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'`. |
||
|
Mingyu Chen (Rayner)
|
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. |
||
|
Mingyu Chen (Rayner)
|
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. |
||
|
Mingyu Chen (Rayner)
|
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. |
||
|
Mryange
|
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 |
||
|
Mingyu Chen (Rayner)
|
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. |
||
 morrySnow
|
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. |
||
|
Calvin Kirs
|
f81660bf5f | [chore](github)Update the list of collaborators (#69972) | ||
 Xin Liao
|
c36a40c67b | [fix](regression) Fix test_ttl_lru_evict and test_stale_rowset in cloud file cache (#69847) | ||
|
Gabriel
|
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. |
||
|
shuke
|
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. |
||
|
shuke
|
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. |
||
|
Jack
|
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. |
||
|
shuke
|
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. |
||
|
Jack
|
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. |
||
|
lihangyu
|
5af2df3231 | [fix](fe) Lock table metadata while constructing UPDATE plans (#69880) | ||
|
daidai
|
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
|
||
 bobhan1
|
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
|
||
 deardeng
|
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. |
||
|
Gabriel
|
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. |
||
|
shuke
|
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. |
||
|
Jerry Hu
|
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 |
||
 HappenLee
|
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
|
||
|
Gabriel
|
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
|
||
 924060929
|
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 |
||
 Mingyu Chen (Rayner)andClaude Opus 5.5
|
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> |
||
|
Jack
|
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. |
||
|
Jack
|
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. |
||
|
Jack
|
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. |
||
|
Jack
|
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. |
||
|
Jack
|
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. |
||
|
Jack
|
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. |
||
|
Mingyu Chen (Rayner)andClaude Opus 5.5
|
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> |