Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cloud/src/common/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,8 @@ CONF_mBool(enable_load_txn_status_check, "true");
CONF_mBool(enable_tablet_job_check, "true");

CONF_mBool(enable_recycle_delete_rowset_key_check, "true");
CONF_mBool(enable_mark_delete_rowset_before_recycle, "true");
CONF_mBool(enable_abort_txn_and_job_for_delete_rowset_before_recycle, "true");
CONF_mBool(enable_mark_delete_rowset_before_recycle, "false");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve an ownership fence before disabling these defaults

With both defaults false, the non-versioned recycler can select an expired PREPARE rowset and queue object deletion while its recycle key is still present and unmarked. A concurrent commit_rowset() can consume that key and publish tmp metadata, after which the queued worker deletes the now-committed files. The tmp path has the same problem for deferred commits: it can delete/remove all unmarked tmp rows of a transaction left COMMITTED, and TxnLazyCommitTask accepts the later empty scan and advances that transaction to VISIBLE. Versioned ref-counts and retention only narrow the window; they do not establish ownership. Every changed test forces true/true, so none exercises the shipped false/false path. Please keep both fences enabled or add an equivalent atomic claim-before-delete protocol with deterministic race coverage.

CONF_mBool(enable_abort_txn_and_job_for_delete_rowset_before_recycle, "false");

// Declare a selection strategy for those servers have many ips.
// Note that there should at most one ip match this list.
Expand Down
30 changes: 30 additions & 0 deletions cloud/test/meta_service_job_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,27 @@ static void create_and_refresh_instance(MetaServiceProxy* service, std::string i
namespace {
const std::string instance_id = "MetaServiceJobTest";

struct DeleteRowsetRecycleConfigGuard {
DeleteRowsetRecycleConfigGuard()
: original_enable_mark_delete_rowset_before_recycle(
config::enable_mark_delete_rowset_before_recycle),
original_enable_abort_txn_and_job_for_delete_rowset_before_recycle(
config::enable_abort_txn_and_job_for_delete_rowset_before_recycle) {
config::enable_mark_delete_rowset_before_recycle = true;
config::enable_abort_txn_and_job_for_delete_rowset_before_recycle = true;
}

~DeleteRowsetRecycleConfigGuard() {
config::enable_mark_delete_rowset_before_recycle =
original_enable_mark_delete_rowset_before_recycle;
config::enable_abort_txn_and_job_for_delete_rowset_before_recycle =
original_enable_abort_txn_and_job_for_delete_rowset_before_recycle;
}

bool original_enable_mark_delete_rowset_before_recycle;
bool original_enable_abort_txn_and_job_for_delete_rowset_before_recycle;
};

void start_compaction_job(MetaService* meta_service, int64_t tablet_id, const std::string& job_id,
const std::string& initiator, int base_compaction_cnt,
int cumu_compaction_cnt, TabletCompactionJobPB::CompactionType type,
Expand Down Expand Up @@ -5753,6 +5774,7 @@ TEST(MetaServiceJobTest, ResetStreamingJobOffsetTest) {

// Test: Complete flow - begin_txn -> prepare_rowset -> recycle x 2 -> abort -> verify commit fails
TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest1) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -5882,6 +5904,7 @@ TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest1) {

// Test: Complete flow - start job -> prepare_rowset -> recycle x 2 -> abort job -> verify
TEST(MetaServiceJobTest, AbortJobForRelatedRowsetTest1) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -6009,6 +6032,7 @@ TEST(MetaServiceJobTest, AbortJobForRelatedRowsetTest1) {

// Test: Complete flow - begin_txn -> prepare_rowset -> commit_rowset -> recycle x 2 -> abort -> verify commit fails
TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest2) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -6134,6 +6158,7 @@ TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest2) {

// Test: Complete flow - start compaction job -> prepare_rowset -> commit_rowset -> recycle x 2 -> abort job -> verify
TEST(MetaServiceJobTest, AbortCompactionJobForRelatedRowsetTest2) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -6270,6 +6295,7 @@ TEST(MetaServiceJobTest, AbortCompactionJobForRelatedRowsetTest2) {

// Test: Complete flow - start schema change job -> prepare_rowset -> commit_rowset -> recycle x 2 -> abort job -> verify
TEST(MetaServiceJobTest, AbortSchemaChangeJobForRelatedRowsetTest2) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -6455,6 +6481,7 @@ TEST(MetaServiceJobTest, AbortSchemaChangeJobForRelatedRowsetTest2) {

// Test: Complete flow - begin_txn -> prepare_rowset -> recycle x 1 -> commit_rowset -> commit_txn -> verify commit fails
TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest3) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -6565,6 +6592,7 @@ TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest3) {

// Test: Complete flow - start job -> prepare_rowset -> recycle x 1 -> commit_rowset -> finish job -> verify
TEST(MetaServiceJobTest, AbortJobForRelatedRowsetTest3) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -6689,6 +6717,7 @@ TEST(MetaServiceJobTest, AbortJobForRelatedRowsetTest3) {

// Test: Complete flow - begin_txn -> prepare_rowset -> commit_rowset -> recycle x 1 -> commit_txn -> verify commit fails
TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest4) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down Expand Up @@ -6812,6 +6841,7 @@ TEST(MetaServiceJobTest, AbortTxnForRelatedRowsetTest4) {

// Test: Complete flow - start job -> prepare_rowset -> commit_rowset -> recycle x 1 -> finish job -> verify
TEST(MetaServiceJobTest, AbortJobForRelatedRowsetTest4) {
DeleteRowsetRecycleConfigGuard config_guard;
auto meta_service = get_meta_service();
auto* sp = SyncPoint::get_instance();
DORIS_CLOUD_DEFER {
Expand Down
Loading