This is an automated email from the ASF dual-hosted git repository.
airborne12 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 24dc7762f1a [improvement](be) Account the NULL docids and point buffer
of the SNII BKD writer (#69728)
24dc7762f1a is described below
commit 24dc7762f1a5575b59a160593f05bee39a9c7d87
Author: Jack <[email protected]>
AuthorDate: Thu Oct 29 09:08:38 2026 +0800
[improvement](be) Account the NULL docids and point buffer of the SNII BKD
writer (#69728)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: `SniiBkdIndexColumnWriter`, the numeric, date and IP
index writer, installed no memory reporter. Its point buffer, and its
NULL docid vector, which grows with the row count, were invisible to the
process-wide SNII build tracker (the `SniiIndexBuild` MemTracker and the
`snii_index_build_mem_consumption` gauge) that the text writer feeds.
The writer now creates a reporter in the unregistered population, since
a forced spill cannot reclaim a BKD build, and hands it to the
BkdBuilder. It charges the NULL docids through the growth routine the
text writer already used, which is extracted to
`reserve_docids_for_append` next to `MemoryReporter` so that the two
writers share it. It gives the vector and its charge back as soon as
`finish()` has moved the docids into the bitmap, because IndexBuilder
keeps finished writers alive until the whole rowset is closed.
This only adds accounting. The reporter has no cap, so it cannot fail a
build, and its callback charges a plain MemTracker, not a limiter.
### Release note
The SNII index build memory metric now also counts the point buffers and
NULL rows of numeric, date and IP indexes.
### Compatibility
- No on-disk format change and no reader change: the BKD writer writes
the same bytes as before.
- The reporter only mirrors byte counts into the `SniiIndexBuild`
MemTracker and the `snii_index_build_mem_consumption` gauge through a
callback. It has no cap, so a reservation cannot fail (only an int64
overflow of the counter could), and the callback charges a plain
MemTracker, never a MemTrackerLimiter, which would count the same bytes
twice because the allocator hook already attributes them. So a build
cannot fail where it passed before.
- Nothing is exchanged between nodes, so mixed versions are not
affected. The text writer uses the extracted helper with the same
behavior: the larger capacity is charged before the allocation and the
old charge is returned after the move.
### Performance
- Only the capacity check of `reserve_docids_for_append` is inline in
its callers. The growth step is out of line (`NO_INLINE`), since it runs
O(log n) times per writer. Compiled with `-O2` and `-O3`, the text
writer's `_reserve_null_docids_for_append` goes from about 200
instructions, with a six-register prologue on every call, to about 10
instructions with no prologue when the capacity is enough. An earlier
version of the helper, with the growth step inline, was not inlined into
its callers and cost the text writer one extra call frame per call,
about 2% in the first row of the benchmark below.
- Appending NULL docids in runs of random length, 100 million rows, best
of 5 repetitions per process, interleaved, one pinned core. `plain` is
the BKD writer before this change, `before` the text writer's routine
before, `after` the shared helper:
| docids per call | plain | before | after | after / before |
|---|---|---|---|---|
| 1 to 20 | 60.3 ms | 51.8 ms | 50.5 ms | 0.976 |
| 1 to 200 | 217.9 ms | 244.7 ms | 251.0 ms | 1.026 |
| 1 to 20000 | 363.2 ms | 325.9 ms | 320.1 ms | 0.982 |
The text writer is unchanged within the noise of this machine (medians
vary by up to 7%). The BKD writer pays for the accounting only in the
middle row, 33 ms for 50 million NULL docids, about 0.7 ns each; in the
other two rows it is faster than before.
---
.../index/inverted/snii/snii_bkd_index_writer.cpp | 24 +++++++++++++
.../index/inverted/snii/snii_bkd_index_writer.h | 11 ++++++
.../index/inverted/snii/snii_index_writer.cpp | 18 ++--------
.../index/inverted/snii/writer/memory_reporter.h | 35 ++++++++++++++++++
.../index/inverted/snii/snii_bkd_adapter_test.cpp | 41 ++++++++++++++++++++++
5 files changed, 113 insertions(+), 16 deletions(-)
diff --git a/be/src/storage/index/inverted/snii/snii_bkd_index_writer.cpp
b/be/src/storage/index/inverted/snii/snii_bkd_index_writer.cpp
index 20d71e1143f..cd71f8172e6 100644
--- a/be/src/storage/index/inverted/snii/snii_bkd_index_writer.cpp
+++ b/be/src/storage/index/inverted/snii/snii_bkd_index_writer.cpp
@@ -17,6 +17,7 @@
#include "storage/index/inverted/snii/snii_bkd_index_writer.h"
+#include <algorithm>
#include <cstring>
#include <utility>
@@ -25,6 +26,7 @@
#include "storage/index/inverted/snii/bkd/bkd_types.h"
#include "storage/index/inverted/snii/format/metadata_directory.h"
#include "storage/index/inverted/snii/format/null_bitmap.h"
+#include "storage/index/inverted/snii/writer/snii_build_memory_tracker.h"
#include "storage/index/inverted/snii/writer/snii_compound_writer.h"
#include "storage/key_coder.h"
#include "storage/tablet/tablet_schema.h"
@@ -84,12 +86,26 @@ Status SniiBkdIndexColumnWriter::init() {
}
_value_size = cast_set<uint32_t>(field_type_size(_value_type));
+ // The same accounting the text writer has: the builder's point buffer and
the NULL docids are
+ // mirrored into the process-wide SNII build tracker. Unregistered,
because a forced spill cannot
+ // reclaim them -- the builder spills on its own buffer bound only.
+ _memory_reporter = std::make_unique<::doris::snii::writer::MemoryReporter>(
+ ::doris::snii::writer::snii_build_consume_release(
+
::doris::snii::writer::BuildMemoryPopulation::kUnregistered));
+ _null_docids_reservation = _memory_reporter->make_reservation();
+
::doris::snii::bkd::BkdBuilderOptions options;
options.bytes_per_dim = _value_size;
options.field_type = _value_type;
+ options.reporter = _memory_reporter.get();
return ::doris::snii::bkd::BkdBuilder::create(options, &_builder);
}
+Status SniiBkdIndexColumnWriter::_reserve_null_docids_for_append(size_t count)
{
+ return ::doris::snii::writer::reserve_docids_for_append(
+ _memory_reporter.get(), &_null_docids_reservation, &_null_docids,
count);
+}
+
Status SniiBkdIndexColumnWriter::_add_value(const uint8_t* value, uint32_t
docid) {
DORIS_CHECK(_builder != nullptr);
std::string encoded;
@@ -153,6 +169,7 @@ Status SniiBkdIndexColumnWriter::add_array_values(size_t
field_size, const uint8
}
Status SniiBkdIndexColumnWriter::add_nulls(uint32_t count) {
+ RETURN_IF_ERROR(_reserve_null_docids_for_append(count));
for (uint32_t i = 0; i < count; ++i) {
_null_docids.push_back(_rid);
++_rid;
@@ -167,6 +184,9 @@ Status SniiBkdIndexColumnWriter::add_array_nulls(const
uint8_t* null_map, size_t
// array level.
DORIS_CHECK_GE(_rid, num_rows);
const uint32_t first_rid = _rid - cast_set<uint32_t>(num_rows);
+ const auto null_count =
+ static_cast<size_t>(std::count(null_map, null_map + num_rows,
static_cast<uint8_t>(1)));
+ RETURN_IF_ERROR(_reserve_null_docids_for_append(null_count));
for (size_t i = 0; i < num_rows; ++i) {
if (null_map[i] == 1) {
_null_docids.push_back(first_rid + cast_set<uint32_t>(i));
@@ -191,6 +211,10 @@ Status SniiBkdIndexColumnWriter::finish() {
::doris::snii::format::NullBitmapWriter null_writer;
null_writer.add_many(_null_docids);
+ // The docids live in the bitmap now. This writer can be kept alive long
after finish() returns,
+ // so give the vector and its charge back instead of holding them until
destruction.
+ std::vector<uint32_t>().swap(_null_docids);
+ _null_docids_reservation.reset();
::doris::snii::ByteSink null_sink;
RETURN_IF_ERROR(null_writer.finish(_rid, &null_sink));
diff --git a/be/src/storage/index/inverted/snii/snii_bkd_index_writer.h
b/be/src/storage/index/inverted/snii/snii_bkd_index_writer.h
index 8dfa648f32d..9c837084e90 100644
--- a/be/src/storage/index/inverted/snii/snii_bkd_index_writer.h
+++ b/be/src/storage/index/inverted/snii/snii_bkd_index_writer.h
@@ -27,6 +27,7 @@
#include "storage/index/inverted/snii/bkd/bkd_builder.h"
#include "storage/index/inverted/snii/bkd/staged_blob_file.h"
#include "storage/index/inverted/snii/encoding/byte_sink.h"
+#include "storage/index/inverted/snii/writer/memory_reporter.h"
#include "storage/olap_common.h"
namespace doris {
@@ -84,9 +85,19 @@ private:
uint32_t _value_size = 0;
const KeyCoder* _value_key_coder = nullptr;
+ // Reserves room for `count` more NULL docids, charging the larger
capacity to the reporter first.
+ Status _reserve_null_docids_for_append(size_t count);
+
+ // Build-time RAM accounting, shared with the builder: the NULL docids
below and the builder's
+ // point buffer show up in the process-wide SNII build tracker while they
are held. Declared
+ // before everything that holds a Reservation on it, so it is destroyed
last.
+ std::unique_ptr<::doris::snii::writer::MemoryReporter> _memory_reporter;
+
// Segment-local row id of the NEXT row to arrive. Advanced by both value
// runs and null runs, so it is the row count once the column is exhausted.
uint32_t _rid = 0;
+ // Declared before the vector it charges for, so the vector is freed
before its charge is returned.
+ ::doris::snii::writer::MemoryReporter::Reservation
_null_docids_reservation;
std::vector<uint32_t> _null_docids;
std::unique_ptr<::doris::snii::bkd::BkdBuilder> _builder;
diff --git a/be/src/storage/index/inverted/snii/snii_index_writer.cpp
b/be/src/storage/index/inverted/snii/snii_index_writer.cpp
index 94766c5ade6..e92b61fbb3e 100644
--- a/be/src/storage/index/inverted/snii/snii_index_writer.cpp
+++ b/be/src/storage/index/inverted/snii/snii_index_writer.cpp
@@ -354,23 +354,9 @@ Status SniiIndexColumnWriter::add_array_values(size_t
field_size, const uint8_t*
}
Status SniiIndexColumnWriter::_reserve_null_docids_for_append(size_t count) {
- const size_t need = _null_docids.size() + count;
- // Repeated NULL runs reuse capacity until geometric growth is needed.
- if (need <= _null_docids.capacity()) {
- return Status::OK();
- }
- const size_t target_capacity = std::max(need, _null_docids.capacity() * 2);
// Uninitialized writers in unit tests have no reporter.
- if (_memory_reporter == nullptr) {
- _null_docids.reserve(target_capacity);
- return Status::OK();
- }
- ::doris::snii::writer::MemoryReporter::Reservation replacement;
- RETURN_IF_ERROR(_null_docids_reservation.prepare_replacement(
- static_cast<uint64_t>(target_capacity) * sizeof(uint32_t),
&replacement));
- _null_docids.reserve(target_capacity);
- _null_docids_reservation = std::move(replacement);
- return Status::OK();
+ return ::doris::snii::writer::reserve_docids_for_append(
+ _memory_reporter.get(), &_null_docids_reservation, &_null_docids,
count);
}
// Hand back the token stream for one row. The two ways to get one differ in
ownership AND in
diff --git a/be/src/storage/index/inverted/snii/writer/memory_reporter.h
b/be/src/storage/index/inverted/snii/writer/memory_reporter.h
index 0f81821927c..68bc43385df 100644
--- a/be/src/storage/index/inverted/snii/writer/memory_reporter.h
+++ b/be/src/storage/index/inverted/snii/writer/memory_reporter.h
@@ -23,8 +23,10 @@
#include <functional>
#include <limits>
#include <utility>
+#include <vector>
#include "common/check.h"
+#include "common/compiler_util.h"
#include "common/logging.h"
#include "common/status.h"
@@ -327,4 +329,37 @@ inline void MemoryReporter::release(uint64_t bytes, bool
postings) {
}
}
+// Grows the vector for reserve_docids_for_append. Out of line: growth is
geometric, so it runs
+// O(log n) times, and keeping it out of the callers leaves them only the
capacity check.
+NO_INLINE inline Status grow_docids_for_append(MemoryReporter* reporter,
+ MemoryReporter::Reservation*
reservation,
+ std::vector<uint32_t>* docids,
size_t need) {
+ const size_t target_capacity = std::max(need, docids->capacity() * 2);
+ if (reporter == nullptr) {
+ docids->reserve(target_capacity);
+ return Status::OK();
+ }
+ MemoryReporter::Reservation replacement;
+ RETURN_IF_ERROR(reservation->prepare_replacement(
+ static_cast<uint64_t>(target_capacity) * sizeof(uint32_t),
&replacement));
+ docids->reserve(target_capacity);
+ *reservation = std::move(replacement);
+ return Status::OK();
+}
+
+// Makes room for `count` more docids in a vector whose capacity is charged to
`reservation`; a null
+// `reporter` (unit tests) charges nothing. Capacity grows geometrically, so a
long sequence of small
+// NULL runs does not reallocate and copy the whole array on every call. The
larger capacity is
+// charged BEFORE it is allocated and the old charge is returned only after
the move, which is the
+// transient double residency of a vector growth.
+inline Status reserve_docids_for_append(MemoryReporter* reporter,
+ MemoryReporter::Reservation*
reservation,
+ std::vector<uint32_t>* docids, size_t
count) {
+ const size_t need = docids->size() + count;
+ if (need <= docids->capacity()) {
+ return Status::OK();
+ }
+ return grow_docids_for_append(reporter, reservation, docids, need);
+}
+
} // namespace doris::snii::writer
diff --git a/be/test/storage/index/inverted/snii/snii_bkd_adapter_test.cpp
b/be/test/storage/index/inverted/snii/snii_bkd_adapter_test.cpp
index 3f5296bf38e..6b8cbaac9b7 100644
--- a/be/test/storage/index/inverted/snii/snii_bkd_adapter_test.cpp
+++ b/be/test/storage/index/inverted/snii/snii_bkd_adapter_test.cpp
@@ -21,6 +21,7 @@
#include <cstdint>
#include <memory>
+#include <numeric>
#include <optional>
#include <set>
#include <string>
@@ -29,6 +30,7 @@
#include "common/status.h"
#include "io/fs/local_file_system.h"
#include "runtime/exec_env.h"
+#include "runtime/memory/mem_tracker.h"
#include "runtime/runtime_state.h"
#include "storage/index/index_file_reader.h"
#include "storage/index/index_file_writer.h"
@@ -46,6 +48,7 @@
#include "storage/index/inverted/snii/snii_bkd_index_reader.h"
#include "storage/index/inverted/snii/snii_bkd_index_writer.h"
#include "storage/index/inverted/snii/staged_file_probe.h"
+#include "storage/index/inverted/snii/writer/snii_build_memory_tracker.h"
#include "storage/key_coder.h"
#include "storage/tablet/tablet_schema.h"
@@ -317,6 +320,44 @@ TEST_F(SniiBkdAdapterTest,
SealingDrainsTheStagedDataWhileTheProducerIsStillAliv
assert_ok(index_file_writer.finish_close());
}
+// The writer's build memory is visible in the process-wide SNII build tracker
while it is held, as
+// the text writer's is: the NULL docids until finish() moves them into the
bitmap, the builder's
+// point buffer until the build is done. None of it may stay charged once
finish() returns, even
+// though the producer is still alive (IndexBuilder keeps it until the rowset
is closed).
+TEST_F(SniiBkdAdapterTest, BuildMemoryIsChargedAndGivenBackAtFinish) {
+ ::doris::MemTracker* tracker =
::doris::snii::writer::snii_build_mem_tracker();
+ const int64_t baseline = tracker->consumption();
+ const int64_t registered_baseline =
::doris::snii::writer::snii_registered_build_bytes();
+
+ const std::string prefix = test_path("accounted_producer");
+ io::FileWriterPtr file_writer;
+ assert_ok(io::global_local_filesystem()->create_file(
+ InvertedIndexDescriptor::get_index_file_path_v2(prefix),
&file_writer));
+ IndexFileWriter index_file_writer(io::global_local_filesystem(), prefix,
"accounted_rowset",
+ /*seg_id=*/0,
InvertedIndexStorageFormatPB::SNII,
+ std::move(file_writer),
/*can_use_ram_dir=*/true,
+ /*tablet_id=*/303);
+ SniiBkdIndexColumnWriter writer(&index_file_writer, &_meta, kFieldType);
+ assert_ok(writer.init());
+
+ constexpr uint32_t kNulls = 200000;
+ assert_ok(writer.add_nulls(kNulls));
+ std::vector<int64_t> values(1000);
+ std::iota(values.begin(), values.end(), 0);
+ assert_ok(writer.add_values(reinterpret_cast<const
uint8_t*>(values.data()), values.size()));
+ // At least the 4 bytes of every NULL docid, and the builder's point
buffer on top.
+ EXPECT_GE(tracker->consumption() - baseline, static_cast<int64_t>(kNulls)
* 4);
+ // A BKD build cannot be reclaimed by a forced spill, so it is not offered
to the spill decision.
+ EXPECT_EQ(::doris::snii::writer::snii_registered_build_bytes(),
registered_baseline);
+
+ assert_ok(writer.finish());
+ EXPECT_EQ(tracker->consumption(), baseline)
+ << "finish() left build memory charged to a producer that is still
alive";
+
+ assert_ok(index_file_writer.begin_close());
+ assert_ok(index_file_writer.finish_close());
+}
+
// NULL rows are carried by the SNII null-bitmap POD, not by the point set: a
// NULL that leaked in as a point would answer `col > x` for a row that has no
// value at all.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
