github-actions[bot] commented on code in PR #69192:
URL: https://github.com/apache/doris/pull/69192#discussion_r4294055830
##########
be/src/storage/index/inverted/snii/compaction/eligibility.cpp:
##########
@@ -74,6 +74,22 @@ Status validate_source_shape(const
reader::LogicalIndexReader& source, size_t so
return Status::OK();
}
+Status validate_source_shape(const reader::LogicalIndexReader& source, size_t
source_ordinal,
+ bool has_positions) {
+ if (source.has_positions() != has_positions ||
+ source.tier() != (has_positions ? format::IndexTier::kT2 :
format::IndexTier::kT1)) {
Review Comment:
[P1] Use physical destination IDs before enabling cloud T1 streaming
For a cloud DUP_KEYS table with a docs-only SNII index and
`enable_cloud_random_segment_id=true`, accepting T1 here changes compaction
from raw rebuild to the streamed owner.
`CloudCompactionMixin::construct_output_rowset_writer()` starts output segment
IDs above zero, and the index-writer map is keyed by those physical IDs.
However, `do_inverted_index_compaction()` selects the SNII destination with
`inverted_index_file_writers[destination_ordinal]`; the first lookup at 0
inserts a null writer, which `add_snii_index_streamed()` dereferences and
crashes the BE. This docs-only workload previously rebuilt successfully.
Resolve the writer using `dest_segment_ids[destination_ordinal]`, as the
CLucene path does, and cover nonzero destination IDs before enabling this path
for T1.
##########
be/src/storage/index/inverted/snii/compaction/eligibility.cpp:
##########
@@ -238,18 +261,18 @@ Status validate_snii_compaction_eligibility(
}
for (size_t source_ordinal = 0; source_ordinal < sources.size();
++source_ordinal) {
- if (sources[source_ordinal].reader.get().tier() !=
format::IndexTier::kT2) {
- return reject(fmt::format("source {} is not a positional (T2)
index", source_ordinal));
- }
-
RETURN_IF_ERROR(validate_plain_t2_source_eligibility(sources[source_ordinal].reader.get(),
- source_ordinal));
+
RETURN_IF_ERROR(validate_snii_source_eligibility(sources[source_ordinal].reader.get(),
Review Comment:
[P1] Keep gram-family T1 indexes on the rebuild path
A gram analyzer with `support_phrase=false` (or omitted) also produces T1
sources, so this newly accepts more than ordinary docs-only postings. On
normal-sized gram segments, the writer deliberately sets `posting_dropped` for
common terms. The merge later calls `SniiPostingCursor::prepare_flat_ranges()`
-> `LogicalIndexReader::resolve_frq_window()`, which returns
`INVERTED_INDEX_EVALUATE_SKIPPED` for those entries. Raw indexing has already
been skipped, and the owner does not mark this status for a rebuild on retry,
so the same rowsets repeatedly fail compaction. Even sources without dropped
postings lose `gram_scheme` in `add_snii_index_streamed()`, disabling
subsequent gram index use. Keep gram-family sources on raw rebuild until the
merge supports their persisted schemes and dropped postings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
