Skip to content

Commit 2f5ebe1

Browse files
banmoyclaude
authored andcommitted
[BugFix] Keep base_version in sync with base_metadata in publish_version
When a single-mode publish already succeeded and its txnlog was deleted but FE never received the response, the txn gets re-sent as the first element of a batch publish. The recovery branch that handles this reassigns base_metadata to the newer version but left the base_version scalar stale, breaking the base_version == base_metadata->version() invariant relied on elsewhere in the function. Sync base_version alongside base_metadata and add a DCHECK to guard the invariant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFu5UZqgfiVXuyUSnttF8J
1 parent 1215f68 commit 2f5ebe1

2 files changed

Lines changed: 55 additions & 0 deletions

File tree

be/src/storage/lake/transactions.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,8 @@ StatusOr<TabletMetadataPtr> publish_version(TabletManager* tablet_mgr, const Pub
476476
return new_version_metadata_or_error(missig_txn_log_meta.status());
477477
} else {
478478
base_metadata = std::move(missig_txn_log_meta).value();
479+
// Keep base_version in sync with base_metadata.
480+
base_version = base_metadata->version();
479481
continue;
480482
}
481483
} else if (txns[i].force_publish()) {
@@ -494,6 +496,7 @@ StatusOr<TabletMetadataPtr> publish_version(TabletManager* tablet_mgr, const Pub
494496

495497
if (log_applier == nullptr) {
496498
// init log_applier
499+
DCHECK_EQ(base_version, base_metadata->version());
497500
new_metadata = std::make_shared<TabletMetadataPB>(*base_metadata);
498501
log_applier = new_txn_log_applier(Tablet(tablet_mgr, tablet_info.get_tablet_id_in_metadata()), new_metadata,
499502
new_version, txns[i].rebuild_pindex(), skip_write_tablet_metadata);

be/test/storage/lake/transactions_test.cpp

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,58 @@ TEST_F(NoOpPublishTest, multi_txn_first_missing_txnlog_no_force_publish_errors)
484484
ASSERT_FALSE(result.ok());
485485
}
486486

487+
// Regression test: a txn already published in single mode (meta@2 written,
488+
// txnlog gone) gets re-sent as the first element of a batch publish, hitting
489+
// the base_version skip branch. Confirms publish_version succeeds without
490+
// tripping the log_applier-init DCHECK.
491+
TEST_F(NoOpPublishTest, single_to_batch_conversion_keeps_base_version_in_sync) {
492+
const int64_t tablet_id = _tablet_metadata->id();
493+
const int64_t txn_id_1 = next_id(); // already published in single mode; its txnlog is gone
494+
const int64_t txn_id_2 = next_id();
495+
const int64_t txn_id_3 = next_id();
496+
497+
// txn_id_1 already published: meta@2 exists, its txnlog is gone.
498+
auto meta_v2 = std::make_shared<TabletMetadataPB>(*_tablet_metadata);
499+
meta_v2->set_version(2);
500+
CHECK_OK(_tablet_mgr->put_tablet_metadata(meta_v2));
501+
502+
// Real txnlogs for txn_id_2/3 so the batch can proceed past the skip.
503+
for (auto txn_id : {txn_id_2, txn_id_3}) {
504+
auto txn_log = std::make_shared<TxnLogPB>();
505+
txn_log->set_tablet_id(tablet_id);
506+
txn_log->set_txn_id(txn_id);
507+
auto log_path = _tablet_mgr->txn_log_location(tablet_id, txn_id);
508+
CHECK_OK(_tablet_mgr->put_txn_log(txn_log, log_path));
509+
}
510+
511+
TxnInfoPB t1;
512+
t1.set_txn_id(txn_id_1);
513+
t1.set_txn_type(TXN_NORMAL);
514+
t1.set_combined_txn_log(false);
515+
t1.set_commit_time(time(nullptr));
516+
517+
TxnInfoPB t2;
518+
t2.set_txn_id(txn_id_2);
519+
t2.set_txn_type(TXN_NORMAL);
520+
t2.set_combined_txn_log(false);
521+
t2.set_commit_time(time(nullptr));
522+
523+
TxnInfoPB t3;
524+
t3.set_txn_id(txn_id_3);
525+
t3.set_txn_type(TXN_NORMAL);
526+
t3.set_combined_txn_log(false);
527+
t3.set_commit_time(time(nullptr));
528+
529+
// Re-sent as one batch with the stale base_version=1.
530+
std::vector<TxnInfoPB> txns{t1, t2, t3};
531+
auto result = publish_version(_tablet_mgr.get(), PublishTabletInfo(tablet_id), 1, 4, txns,
532+
/*skip_write_tablet_metadata=*/false);
533+
ASSERT_OK(result.status());
534+
535+
auto new_metadata = result.value();
536+
ASSERT_EQ(4, new_metadata->version());
537+
}
538+
487539
// Coverage test for the i>0 + missing-txnlog + !force_publish branch (legacy
488540
// hard-error path). When a non-first txn in a batch is missing its log and is
489541
// not flagged for force_publish, the publish loop must abort rather than

0 commit comments

Comments
 (0)