This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new 3ff2aa081a3 branch-4.2: [fix](compaction) Seal index files after index
compaction in non-vertical compaction (#69936)
3ff2aa081a3 is described below
commit 3ff2aa081a32b4a0172be5657f5fdbaf0f36d5c1
Author: Jack <[email protected]>
AuthorDate: Tue Nov 3 09:54:31 2026 +0800
branch-4.2: [fix](compaction) Seal index files after index compaction in
non-vertical compaction (#69936)
### What problem does this PR solve?
Issue Number: None
Related PR: #65858
Problem Summary:
With `enable_vertical_compaction=false` and
`inverted_index_compaction_enable=true`, a DUP KEY or MoW table
compacted through the non-vertical path loses the inverted indexes of
its string columns.
- Columns in `columns_to_do_index_compaction` are skipped by the segment
writer and merged after all segments are flushed.
- Non-vertical compaction flushes through `SegmentFlusher`, which called
`close_inverted_index()` at flush time. With no logical index present,
`begin_close()` sealed an empty container and uploaded a zero byte
`.idx`.
- Index compaction then wrote into temporary directories of the closed
writer, and `SegmentFlusher::close()` only ran `finish_close()`, so the
merged index was never packed. `succeed to do index compaction` is still
logged.
Result: `INDEX_DISK_SIZE=0`, and each query logs `[E-6004] inverted
index file ... is empty` and scans the whole segment. Query results stay
correct.
With a numeric index on the same table, the file is sealed with the
numeric index alone, so `INDEX_DISK_SIZE` stays above zero while the
string index is gone.
master fixed this as part of the row TTL feature #65858. This PR carries
only that part, without the feature: when the rowset has index
compaction, the flush no longer closes the index files, and
`BaseBetaRowsetWriter::_close_file_writers()` calls the existing
`_close_inverted_index_file_writers()`, the same close the vertical
writer uses. All segments are already flushed at that point, because the
merger flushes the rowset writer when it finishes. This branch still has
`SegmentWriter`, so both `_flush_segment_writer` overloads are changed.
A `DCHECK` in `BlockReader::_direct_next_block` is relaxed as well. At
EOF the block is empty and the iterator still reports the previous
block's locations, so ASAN and debug builds abort on this path. `Merger`
only reads the locations when the block has rows, so release builds were
not affected.
Existing rowsets are not repaired by this change.
### Branch adaptations
- `has_index_compaction()` also checks `snii_indexes_to_do_compaction`,
same as master.
- master seals in `SegmentFlusher::close()`. This PR reuses the rowset
level close instead, so the result is the same but the code differs from
master.
- master covers this path with a BE unit test, not with a regression
test. `test_index_compaction_non_vertical` is new here, and #69976 adds
it to master with the same close.
### Release note
Fix inverted indexes of string columns being lost by non-vertical
compaction when inverted index compaction is enabled.
### Check List (For Author)
- Test
- [x] Regression test: in non-cloud mode on this branch, new
`test_index_compaction_non_vertical` and
`test_index_compaction_dup_keys`, `_unique_keys`, `_null` and
`_empty_segments` pass.
- [ ] Unit Test
- [x] Manual test: full compaction with vertical off on V2, V3 and SNII,
with string indexes only and with numeric plus string indexes. Index
size is above zero and no index downgrade happens.
- [ ] No need to test or manual test.
The failure without the fix was checked on branch-4.1, not on this
branch. Cloud mode and BE unit tests were not run on this branch. The
local build disabled one lance call because the installed lance-c is
older than this branch needs; that change is not in this PR.
- Behavior changed:
- [ ] No.
- [x] Yes. `INDEX_DISK_SIZE` of such compaction output is now the real
size and the output keeps its inverted indexes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: martin.foo838 <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
be/src/storage/iterator/block_reader.cpp | 3 +-
be/src/storage/rowset/beta_rowset_writer.cpp | 4 +
be/src/storage/rowset/beta_rowset_writer.h | 2 +-
be/src/storage/rowset/rowset_writer_context.h | 5 +
be/src/storage/rowset/segment_creator.cpp | 12 ++-
.../test_index_compaction_non_vertical.out | 27 +++++
.../test_index_compaction_non_vertical.groovy | 117 +++++++++++++++++++++
7 files changed, 166 insertions(+), 4 deletions(-)
diff --git a/be/src/storage/iterator/block_reader.cpp
b/be/src/storage/iterator/block_reader.cpp
index 1bb23c7194d..8119eeefcde 100644
--- a/be/src/storage/iterator/block_reader.cpp
+++ b/be/src/storage/iterator/block_reader.cpp
@@ -278,7 +278,8 @@ Status BlockReader::_direct_next_block(Block* block, bool*
eof) {
if (UNLIKELY(!res.ok() && res != Status::Error<END_OF_FILE>(""))) {
return res;
}
- DCHECK_EQ(_block_row_locations.size(), block->rows());
+ // At EOF the block is empty and the iterator still reports the
previous block's locations.
+ DCHECK(*eof || _block_row_locations.size() == block->rows());
}
return Status::OK();
}
diff --git a/be/src/storage/rowset/beta_rowset_writer.cpp
b/be/src/storage/rowset/beta_rowset_writer.cpp
index 676b258bcaa..90bb771ff5d 100644
--- a/be/src/storage/rowset/beta_rowset_writer.cpp
+++ b/be/src/storage/rowset/beta_rowset_writer.cpp
@@ -947,6 +947,10 @@ RowsetSharedPtr BaseBetaRowsetWriter::manual_build(const
RowsetMetaSharedPtr& sp
}
Status BaseBetaRowsetWriter::_close_file_writers() {
+ // Index compaction runs after every segment is flushed, so its index
files are still open.
+ if (_context.has_index_compaction()) {
+ RETURN_IF_ERROR(_close_inverted_index_file_writers());
+ }
// Flush and close segment files
RETURN_NOT_OK_STATUS_WITH_WARN(_segment_creator.close(),
"failed to close segment creator when build
new rowset");
diff --git a/be/src/storage/rowset/beta_rowset_writer.h
b/be/src/storage/rowset/beta_rowset_writer.h
index e937d7a9cbb..a31b278ea69 100644
--- a/be/src/storage/rowset/beta_rowset_writer.h
+++ b/be/src/storage/rowset/beta_rowset_writer.h
@@ -213,7 +213,7 @@ protected:
std::lock_guard l(_segid_statistics_map_mutex);
return std::accumulate(_segment_num_rows.begin(),
_segment_num_rows.end(), uint64_t(0));
}
- // Only during vertical compaction is this method called
+ // Only during compaction is this method called
// Some index files are written during normal compaction and some files
are written during index compaction.
// After all index writes are completed, call this method to write the
final compound index file.
Status _close_inverted_index_file_writers() {
diff --git a/be/src/storage/rowset/rowset_writer_context.h
b/be/src/storage/rowset/rowset_writer_context.h
index 5e0eba14474..43525ce136f 100644
--- a/be/src/storage/rowset/rowset_writer_context.h
+++ b/be/src/storage/rowset/rowset_writer_context.h
@@ -101,6 +101,11 @@ struct RowsetWriterContext {
// coexist in a single pass. V2/V3 keep columns_to_do_index_compaction:
// their per-column CLucene directories cannot split an index off a column.
std::set<std::pair<int32_t, int64_t>> snii_indexes_to_do_compaction;
+
+ bool has_index_compaction() const {
+ return !columns_to_do_index_compaction.empty() ||
!snii_indexes_to_do_compaction.empty();
+ }
+
DataWriteType write_type = DataWriteType::TYPE_DEFAULT;
// need to figure out the sub type of compaction
ReaderType compaction_type = ReaderType::UNKNOWN;
diff --git a/be/src/storage/rowset/segment_creator.cpp
b/be/src/storage/rowset/segment_creator.cpp
index 98d6d418bbf..ab63a7db166 100644
--- a/be/src/storage/rowset/segment_creator.cpp
+++ b/be/src/storage/rowset/segment_creator.cpp
@@ -232,7 +232,11 @@ Status SegmentFlusher::_flush_segment_writer(
MonotonicStopWatch inverted_index_timer;
inverted_index_timer.start();
int64_t inverted_index_file_size = 0;
- RETURN_IF_ERROR(writer->close_inverted_index(&inverted_index_file_size));
+ // A horizontal compaction can write a numeric index here and merge a
string index later.
+ // Closing now would persist only the numeric index; defer both sealing
and size accounting.
+ if (!_context.has_index_compaction()) {
+
RETURN_IF_ERROR(writer->close_inverted_index(&inverted_index_file_size));
+ }
inverted_index_timer.stop();
VLOG_DEBUG << "tablet_id:" << _context.tablet_id
@@ -313,7 +317,11 @@ Status
SegmentFlusher::_flush_segment_writer(std::unique_ptr<segment_v2::Segment
MonotonicStopWatch inverted_index_timer;
inverted_index_timer.start();
int64_t inverted_index_file_size = 0;
- RETURN_IF_ERROR(writer->close_inverted_index(&inverted_index_file_size));
+ // A horizontal compaction can write a numeric index here and merge a
string index later.
+ // Closing now would persist only the numeric index; defer both sealing
and size accounting.
+ if (!_context.has_index_compaction()) {
+
RETURN_IF_ERROR(writer->close_inverted_index(&inverted_index_file_size));
+ }
inverted_index_timer.stop();
VLOG_DEBUG << "tablet_id:" << _context.tablet_id
diff --git
a/regression-test/data/inverted_index_p0/index_compaction/test_index_compaction_non_vertical.out
b/regression-test/data/inverted_index_p0/index_compaction/test_index_compaction_non_vertical.out
new file mode 100644
index 00000000000..b36b73699fd
--- /dev/null
+++
b/regression-test/data/inverted_index_p0/index_compaction/test_index_compaction_non_vertical.out
@@ -0,0 +1,27 @@
+-- This file is automatically generated. You should know what you did if you
want to edit this
+-- !string_before --
+2026-01-01T00:00:03 trace-3 request number 3 finished
+
+-- !string_exact --
+2026-01-01T00:00:03 trace-3 request number 3 finished
+
+-- !string_token --
+2026-01-01T00:00:01 trace-1 request number 1 finished
+2026-01-01T00:00:02 trace-2 request number 2 finished
+2026-01-01T00:00:03 trace-3 request number 3 finished
+2026-01-01T00:00:04 trace-4 request number 4 finished
+2026-01-01T00:00:05 trace-5 request number 5 finished
+2026-01-01T00:00:06 trace-6 request number 6 finished
+
+-- !string_equal --
+2026-01-01T00:00:05 trace-5 request number 5 finished
+
+-- !mixed_string --
+2026-01-01T00:00:03 3 trace-3
+
+-- !mixed_numeric --
+2026-01-01T00:00:04 4 trace-4
+
+-- !mixed_second_round --
+2026-01-01T00:00:08 8 trace-8
+
diff --git
a/regression-test/suites/inverted_index_p0/index_compaction/test_index_compaction_non_vertical.groovy
b/regression-test/suites/inverted_index_p0/index_compaction/test_index_compaction_non_vertical.groovy
new file mode 100644
index 00000000000..51f87e8344a
--- /dev/null
+++
b/regression-test/suites/inverted_index_p0/index_compaction/test_index_compaction_non_vertical.groovy
@@ -0,0 +1,117 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+suite("test_index_compaction_non_vertical", "nonConcurrent") {
+ def isCloudMode = isCloudMode()
+
+ def get_index_sizes = { String tableName ->
+ def tablets = sql_return_maparray """ show tablets from ${tableName};
"""
+ assertEquals(1, tablets.size())
+ return sql_return_maparray("""
+ select INDEX_DISK_SIZE, DATA_DISK_SIZE from
information_schema.rowsets
+ where TABLET_ID = ${tablets[0].TabletId} and ROWSET_NUM_ROWS > 0;
+ """)
+ }
+
+ // The index file is sealed only after index compaction merged the string
postings, so every
+ // MATCH must still be answered by the index. With this variable off, a
lost index is an error
+ // rather than a silent fallback to reading the column.
+ sql """ set enable_match_without_inverted_index = false """
+ sql """ set enable_common_expr_pushdown = true """
+ sql """ set enable_sql_cache = false """
+
+ setBeConfigTemporary([
+ "enable_vertical_compaction": "false",
+ "inverted_index_compaction_enable": "true"
+ ]) {
+ // Only string indexes: before the fix the compound file was closed
empty at flush time.
+ sql """ DROP TABLE IF EXISTS test_index_compaction_non_vertical_string
"""
+ sql """
+ CREATE TABLE test_index_compaction_non_vertical_string (
+ `ts` datetime NOT NULL,
+ `trace_id` text NULL,
+ `msg` text NULL,
+ INDEX idx_trace_id (`trace_id`) USING INVERTED,
+ INDEX idx_msg (`msg`) USING INVERTED PROPERTIES("parser" =
"unicode", "support_phrase" = "true")
+ ) ENGINE=OLAP
+ DUPLICATE KEY(`ts`)
+ DISTRIBUTED BY HASH(`ts`) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "disable_auto_compaction" = "true",
+ "inverted_index_storage_format" = "V2"
+ );
+ """
+ for (int i = 1; i <= 6; i++) {
+ sql """ INSERT INTO test_index_compaction_non_vertical_string
VALUES
+ ('2026-01-01 00:00:0${i}', 'trace-${i}', 'request number
${i} finished'); """
+ }
+ order_qt_string_before """ select * from
test_index_compaction_non_vertical_string where trace_id match 'trace-3' """
+
+
trigger_and_wait_compaction("test_index_compaction_non_vertical_string", "full")
+
+ def stringRowsets =
get_index_sizes("test_index_compaction_non_vertical_string")
+ assertEquals(1, stringRowsets.size())
+ assertTrue(stringRowsets[0].INDEX_DISK_SIZE as long > 0)
+
+ order_qt_string_exact """ select * from
test_index_compaction_non_vertical_string where trace_id match 'trace-3' """
+ order_qt_string_token """ select * from
test_index_compaction_non_vertical_string where msg match 'finished' """
+ order_qt_string_equal """ select * from
test_index_compaction_non_vertical_string where trace_id = 'trace-5' """
+
+ // A numeric index is built while the segment is flushed and the
string one is merged
+ // afterwards. Before the fix the file was sealed with the numeric
index alone, so its size
+ // stayed above zero while the string index was gone.
+ sql """ DROP TABLE IF EXISTS test_index_compaction_non_vertical_mixed
"""
+ sql """
+ CREATE TABLE test_index_compaction_non_vertical_mixed (
+ `ts` datetime NOT NULL,
+ `code` int NULL,
+ `trace_id` text NULL,
+ INDEX idx_code (`code`) USING INVERTED,
+ INDEX idx_trace_id (`trace_id`) USING INVERTED
+ ) ENGINE=OLAP
+ DUPLICATE KEY(`ts`)
+ DISTRIBUTED BY HASH(`ts`) BUCKETS 1
+ PROPERTIES (
+ "replication_num" = "1",
+ "disable_auto_compaction" = "true",
+ "inverted_index_storage_format" = "V2"
+ );
+ """
+ for (int i = 1; i <= 6; i++) {
+ sql """ INSERT INTO test_index_compaction_non_vertical_mixed VALUES
+ ('2026-01-01 00:00:0${i}', ${i}, 'trace-${i}'); """
+ }
+
+
trigger_and_wait_compaction("test_index_compaction_non_vertical_mixed", "full")
+
+ def mixedRowsets =
get_index_sizes("test_index_compaction_non_vertical_mixed")
+ assertEquals(1, mixedRowsets.size())
+ assertTrue(mixedRowsets[0].INDEX_DISK_SIZE as long > 0)
+
+ order_qt_mixed_string """ select * from
test_index_compaction_non_vertical_mixed where trace_id match 'trace-3' """
+ order_qt_mixed_numeric """ select * from
test_index_compaction_non_vertical_mixed where code = 4 """
+
+ // The merged indexes must survive the next compaction of the same
rowset.
+ for (int i = 7; i <= 9; i++) {
+ sql """ INSERT INTO test_index_compaction_non_vertical_mixed VALUES
+ ('2026-01-01 00:00:0${i}', ${i}, 'trace-${i}'); """
+ }
+
trigger_and_wait_compaction("test_index_compaction_non_vertical_mixed", "full")
+ order_qt_mixed_second_round """ select * from
test_index_compaction_non_vertical_mixed where trace_id match 'trace-8' """
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
