Skip to content

Commit 3debacf

Browse files
bobhan1bobhan1
committed
[improvement](cloud) Skip initial rowset sync for MoW loads by default
### Background Cloud MoW load initialization can synchronize rowsets from Meta Service before capturing its local rowset snapshot. The existing mutable cloud_mow_sync_rowsets_when_load_txn_begin setting controls this synchronization. ### What problem does this PR solve? Issue Number: None Related PR: #59754 (introduced the setting), #70218 (enables notifications and fixes cleanup), #70209 (temporary BE UT build prerequisite) PR Type: Enhancement Problem Summary: Make rowset synchronization at the start of a cloud MoW load opt-in by changing cloud_mow_sync_rowsets_when_load_txn_begin from true to false. Adjust the related CloudGroupRowsetBuilderWriterTest fixture to restore the incoming configuration value instead of hard-coding true during teardown. ### Details - **Before and after:** Cloud MoW loads no longer force sync_rowsets before constructing their initial local rowset snapshot by default. - **Configs and defaults:** cloud_mow_sync_rowsets_when_load_txn_begin changes from true to false and remains dynamically mutable. - **Impact on existing users:** An explicit true setting retains synchronization at load initialization. Notification delivery and subsequent metadata synchronization remain relevant to local rowset freshness. - **Behavior change:** Yes; skip the initial synchronization unless the setting is enabled. The test fixture preserves whichever configuration value it received. ### Testing - ./run-be-ut.sh --run --filter='CloudGroupRowsetBuilderWriterTest.*:*DeltaWriterCancelIntegrationTest.*' -j100 passed all 12 ASAN tests after this change. - The unit-test build temporarily applied #70209 to fix the unrelated vparquet_writer_test.cpp compilation failure. That prerequisite was removed afterward and is not included in this commit. - clang-format v16/check-format on both changed files and git diff --check passed. - The earlier 36 BE and 56 FE passing tests belong to commit 1b4d168, before this default update; they were not rerun for this commit. - clang-tidy was omitted. The stale-cache partial-update scenario below was not exercised in a running cloud cluster. ### Notes for Reviewers The stale-rowset partial-update issue identified during source review remains unresolved: if a BE has not received or synchronized a prior committed rowset, constructing the MoW context from its local snapshot can classify an existing key as new and fail a partial update under the ERROR policy or when an omitted column is non-nullable and has no default. This commit changes the default; it does not repair that path. ### Release note The BE setting cloud_mow_sync_rowsets_when_load_txn_begin now defaults to false. Cloud MoW loads skip forced rowset synchronization at initialization unless this setting is explicitly enabled. ### Check List (For Author) - Test: Unit Test; 12 BE ASAN tests passed with the prerequisite described above. - Behavior changed: Yes (see Details). - Does this need documentation: Yes; include this third default-value change in the configuration reference and release notes alongside the two notification settings. - Pre-submission status: Partially verified: focused tests passed; the known stale-cache partial-update risk remains unresolved and cloud end-to-end validation is pending. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: Code, test fixture adjustment and this description. - Human review: Pending.
1 parent 1b4d168 commit 3debacf

2 files changed

Lines changed: 4 additions & 2 deletions

File tree

‎be/src/common/config.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2148,7 +2148,7 @@ DEFINE_mInt32(concurrency_stats_dump_interval_ms, "100");
21482148
DEFINE_Validator(concurrency_stats_dump_interval_ms,
21492149
[](const int32_t config) -> bool { return config >= 10; });
21502150

2151-
DEFINE_mBool(cloud_mow_sync_rowsets_when_load_txn_begin, "true");
2151+
DEFINE_mBool(cloud_mow_sync_rowsets_when_load_txn_begin, "false");
21522152

21532153
DEFINE_mBool(enable_cloud_make_rs_visible_on_be, "true");
21542154
DEFINE_mBool(enable_cloud_random_segment_id, "false");

‎be/test/olap/rowset/cloud_group_rowset_builder_writer_test.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ void add_initial_rowset(const CloudTabletSPtr& tablet, int64_t version) {
161161
class CloudGroupRowsetBuilderWriterTest : public testing::Test {
162162
protected:
163163
void SetUp() override {
164+
_saved_sync_rowsets = config::cloud_mow_sync_rowsets_when_load_txn_begin;
164165
config::cloud_mow_sync_rowsets_when_load_txn_begin = false;
165166

166167
char buffer[MAX_PATH_LEN];
@@ -205,7 +206,7 @@ class CloudGroupRowsetBuilderWriterTest : public testing::Test {
205206
_changelog_tablet.reset();
206207
_engine.reset();
207208
EXPECT_TRUE(io::global_local_filesystem()->delete_directory(_storage_root_path).ok());
208-
config::cloud_mow_sync_rowsets_when_load_txn_begin = true;
209+
config::cloud_mow_sync_rowsets_when_load_txn_begin = _saved_sync_rowsets;
209210
}
210211

211212
void create_remote_tablet_dir(int64_t tablet_id) {
@@ -336,6 +337,7 @@ class CloudGroupRowsetBuilderWriterTest : public testing::Test {
336337
0);
337338
}
338339

340+
bool _saved_sync_rowsets = false;
339341
std::unique_ptr<CloudStorageEngine> _engine;
340342
CloudTabletSPtr _tablet;
341343
CloudTabletSPtr _changelog_tablet;

0 commit comments

Comments
 (0)