This is an automated email from the ASF dual-hosted git repository.
bobhan1 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 e4d328e23b8 [fix](mow) Skip delete bitmap cache prefill for
non-running tablets (#68436)
e4d328e23b8 is described below
commit e4d328e23b888ea9698f7e58c99dcf460580e122
Author: bobhan1 <[email protected]>
AuthorDate: Thu Sep 24 14:51:49 2026 +0800
[fix](mow) Skip delete bitmap cache prefill for non-running tablets (#68436)
### What problem does this PR solve?
Cloud schema change can allow cumulative compaction on a
`TABLET_NOTREADY` tablet before its delete bitmap is complete.
Post-compaction prefill then caches empty or incomplete aggregate
bitmaps at the tablet's maximum version. Schema change later installs
the completed bitmap and marks the tablet `TABLET_RUNNING`, but the
cache keys are unchanged. Queries can reuse the stale bitmaps and return
overwritten rows.
Require `TABLET_RUNNING` under the existing tablet header read lock
before either post-compaction prefill mode runs. Normal prefill on
running tablets remains enabled.
Add a parameterized BE test that keeps the same rowsets and read version
while installing the schema-change bitmap and transitioning the tablet
to RUNNING. It covers all four prefill switch combinations, output and
non-output rowsets, and non-contiguous segment IDs. Each test instance
uses unique rowset IDs to isolate the global aggregate cache.
### Release note
Fix duplicate rows returned by Unique Key Merge-on-Write queries when
schema change overlaps compaction.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test
- [ ] No need to test or manual test.
Validation:
- Before the fix: three combinations with prefill enabled fail with
stale empty bitmaps; disabling both switches passes.
- After the fix: 56 ASAN BE unit tests pass, including all four new
parameter combinations, `CloudTabletDeleteBitmapTest`, `TabletMetaTest`,
and `CloudCompactionTest`.
- Command: `./run-be-ut.sh --run
--filter='PrefillModes/CloudTabletDeleteBitmapPrefillTest.*:CloudTabletDeleteBitmapTest.*:TabletMetaTest.*:CloudCompactionTest.*'
-j100`
- Changed C++ files pass clang-format 16, build hygiene checks, and `git
diff --check`.
- The existing Docker regression cases
`test_schema_change_with_compaction9` and
`test_schema_change_with_compaction10` cover the original interleaving
but were not run locally. Final-change clang-tidy was not run.
- Behavior changed:
- [ ] No.
- [x] Yes. Post-compaction aggregate delete bitmap cache prefill is
skipped until the tablet is RUNNING.
- 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
---
be/src/storage/tablet/base_tablet.cpp | 5 +++
be/test/cloud/cloud_tablet_test.cpp | 78 +++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+)
diff --git a/be/src/storage/tablet/base_tablet.cpp
b/be/src/storage/tablet/base_tablet.cpp
index c7ad7a6c639..71a5cd6c641 100644
--- a/be/src/storage/tablet/base_tablet.cpp
+++ b/be/src/storage/tablet/base_tablet.cpp
@@ -2390,6 +2390,11 @@ void
BaseTablet::prefill_dbm_agg_cache_after_compaction(const RowsetSharedPtr& o
int64_t cur_max_version {-1};
{
std::shared_lock rlock(get_header_lock());
+ // Schema change may still be rebuilding the delete bitmap of a
NOTREADY tablet.
+ // Prefilling now can cache incomplete bitmaps that remain stale
after it becomes RUNNING.
+ if (tablet_state() != TABLET_RUNNING) {
+ return;
+ }
cur_max_version = max_version_unlocked();
}
if (config::enable_prefill_all_dbm_agg_cache_after_compaction) {
diff --git a/be/test/cloud/cloud_tablet_test.cpp
b/be/test/cloud/cloud_tablet_test.cpp
index 0480c0a2510..3707815c268 100644
--- a/be/test/cloud/cloud_tablet_test.cpp
+++ b/be/test/cloud/cloud_tablet_test.cpp
@@ -26,6 +26,7 @@
#include <cstdint>
#include <future>
#include <mutex>
+#include <tuple>
#include "cloud/cloud_meta_mgr.h"
#include "cloud/cloud_storage_engine.h"
@@ -35,7 +36,9 @@
#include "storage/rowset/rowset.h"
#include "storage/rowset/rowset_factory.h"
#include "storage/rowset/rowset_meta.h"
+#include "storage/rowset/unique_rowset_id_generator.h"
#include "storage/tablet/tablet_meta.h"
+#include "util/defer_op.h"
#include "util/uid_util.h"
namespace doris {
@@ -149,6 +152,81 @@ TEST_F(CloudTabletDeleteBitmapTest,
AggDeleteBitmapForCompactionReturnsPreRowset
EXPECT_EQ(rowset_versions_without_stats, pre_rowset_to_versions);
}
+class CloudTabletDeleteBitmapPrefillTest
+ : public CloudTabletWarmUpStateTest,
+ public testing::WithParamInterface<std::tuple<bool, bool>> {
+public:
+ void SetUp() override {
+ CloudTabletWarmUpStateTest::SetUp();
+ // The aggregate cache is global, so rowset IDs must be unique across
test instances.
+ _engine._rowset_id_generator =
+ std::make_unique<UniqueRowsetIdGenerator>(UniqueId::gen_uid());
+ }
+};
+
+TEST_P(CloudTabletDeleteBitmapPrefillTest, WaitForSchemaChangeDeleteBitmap) {
+ const auto old_prefill_output =
config::enable_prefill_output_dbm_agg_cache_after_compaction;
+ const auto old_prefill_all =
config::enable_prefill_all_dbm_agg_cache_after_compaction;
+ Defer restore_config {[&] {
+ config::enable_prefill_output_dbm_agg_cache_after_compaction =
old_prefill_output;
+ config::enable_prefill_all_dbm_agg_cache_after_compaction =
old_prefill_all;
+ }};
+ const auto [prefill_output, prefill_all] = GetParam();
+ config::enable_prefill_output_dbm_agg_cache_after_compaction =
prefill_output;
+ config::enable_prefill_all_dbm_agg_cache_after_compaction = prefill_all;
+
+ _tablet->tablet_meta()->set_enable_unique_key_merge_on_write(true);
+ _tablet->tablet_meta()->mutable_tablet_schema()->_keys_type = UNIQUE_KEYS;
+ auto output_rowset = create_rowset(Version(2, 3), 2);
+ auto other_rowset = create_rowset(Version(4, 4), 2);
+ ASSERT_NE(output_rowset, nullptr);
+ ASSERT_NE(other_rowset, nullptr);
+ output_rowset->rowset_meta()->set_segment_ids({3, 7});
+ other_rowset->rowset_meta()->set_segment_ids({5, 9});
+ const std::vector<RowsetSharedPtr> rowsets {output_rowset, other_rowset};
+ {
+ std::unique_lock lock(_tablet->get_header_lock());
+ ASSERT_TRUE(_tablet->set_tablet_state(TABLET_NOTREADY).ok());
+ _tablet->add_rowsets(rowsets, false, lock, false);
+ }
+
+ // Compaction finishes before schema change installs the complete delete
bitmap.
+ _tablet->prefill_dbm_agg_cache_after_compaction(output_rowset);
+ auto cache_before =
DeleteBitmapAggCache::instance()->snapshot(_tablet->tablet_id());
+ DeleteBitmap complete_delete_bitmap(_tablet->tablet_id());
+ for (const auto& rowset : rowsets) {
+ for (auto segment : rowset->segments()) {
+ auto key = segment.delete_bitmap_key(4);
+ EXPECT_EQ(cache_before.get(key), nullptr);
+ complete_delete_bitmap.add(key, 1);
+ }
+ }
+ {
+ std::unique_lock lock(_tablet->get_header_lock());
+ _tablet->tablet_meta()->delete_bitmap() = complete_delete_bitmap;
+ ASSERT_TRUE(_tablet->set_tablet_state(TABLET_RUNNING).ok());
+ }
+
+ // Prefill must still work for RUNNING tablets, honoring both
configuration switches.
+ _tablet->prefill_dbm_agg_cache_after_compaction(output_rowset);
+ auto cache_after =
DeleteBitmapAggCache::instance()->snapshot(_tablet->tablet_id());
+ for (const auto& rowset : rowsets) {
+ const bool should_prefill = prefill_all || (prefill_output && rowset
== output_rowset);
+ for (auto segment : rowset->segments()) {
+ auto key = segment.delete_bitmap_key(4);
+ EXPECT_EQ(cache_after.get(key) != nullptr, should_prefill);
+ // The read version and rowset IDs are unchanged after schema
change. A premature
+ // prefill would return the old empty bitmap and expose
overwritten rows here.
+ auto bitmap = _tablet->tablet_meta()->delete_bitmap().get_agg(key);
+ EXPECT_EQ(bitmap->cardinality(), 1);
+ EXPECT_TRUE(bitmap->contains(1));
+ }
+ }
+}
+
+INSTANTIATE_TEST_SUITE_P(PrefillModes, CloudTabletDeleteBitmapPrefillTest,
+ testing::Combine(testing::Bool(), testing::Bool()));
+
// Test get_rowset_warmup_state for non-existent rowset
TEST_F(CloudTabletWarmUpStateTest, TestGetRowsetWarmupStateNonExistent) {
auto rowset = create_rowset(Version(1, 1));
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
