github-actions[bot] commented on code in PR #67989:
URL: https://github.com/apache/doris/pull/67989#discussion_r4013955721
##########
cloud/src/meta-service/meta_service_txn.cpp:
##########
@@ -2008,6 +2008,34 @@ void MetaServiceImpl::commit_txn_immediately(
continue;
}
+ {
+ // Recheck the tmp keys from scan_tmp_rowset() in this write
transaction.
+ // Non-snapshot reads detect concurrent lazy cleanup; already
missing keys
+ // require a retry to avoid publishing the same rowset twice.
+ auto tmp_keys = to_container<std::vector<std::string>>(
+ std::ranges::ref_view(tmp_rowsets_meta) |
std::ranges::views::keys);
+ std::vector<std::optional<std::string>> tmp_values;
+ err = txn->batch_get(&tmp_values, tmp_keys,
Transaction::BatchGetOptions(false));
+ if (err != TxnErrorCode::TXN_OK) {
+ code = cast_as<ErrCategory::READ>(err);
+ msg = fmt::format("failed to get tmp rowsets before commit,
txn_id={} err={}",
+ txn_id, err);
+ LOG(WARNING) << msg;
+ return;
+ }
+ for (size_t i = 0; i < tmp_keys.size(); ++i) {
+ if (!tmp_values[i].has_value()) {
Review Comment:
[P1] Reject changed tmp-rowset values, not just missing keys
This transaction still publishes the `tmp_rowsets_meta` captured by the
earlier range scan. An outstanding/retried delete-bitmap task can overwrite the
same key through `update_tmp_rowset()` after merging partial-update segments;
if that write commits before this `batch_get`, FDB returns a present value and
creates no later conflict, but this path discards those bytes, publishes the
old rowset metadata/statistics, and then deletes the updated key. Please
compare the returned value with the scanned value and retry on any mismatch, or
rebuild all derived state from the values read in this transaction.
##########
cloud/test/txn_lazy_commit_test.cpp:
##########
@@ -1211,6 +1215,363 @@ TEST(TxnLazyCommitTest, CommitTxnImmediatelyTest) {
}
}
+// Use real FDB transactions to verify both stale scans and read/write
conflicts.
+// When another transaction deletes a temporary key after it was read, FDB must
+// reject the publishing transaction at commit time.
+class ImmediateCommitTmpRowsetTest : public testing::Test {
+protected:
+ using Metadata = std::map<std::string, std::string>;
+ static constexpr int64_t DB_ID = 91001;
+ static constexpr int64_t TABLE_ID = 91002;
+ static constexpr int64_t INDEX_ID = 91003;
+ static constexpr int64_t PARTITION_ID = 91004;
+ static constexpr int64_t TABLET_ID = 91006;
+
+ void SetUp() override {
+ config::txn_lazy_commit_rowsets_thresold = 1;
+ config::enable_cloud_parallel_txn_lazy_commit = true;
Review Comment:
[P2] Pin the partition worker count used by this gate
`StaleScanAfterLazyPartitionCommit` blocks each partition callback until
`partitions_entered == 2`, but the fixture only enables parallel mode.
`parallel_txn_lazy_commit_num_threads` may validly be 1, and `TxnLazyCommitter`
honors that value exactly, leaving the sole worker waiting until this test
times out without reaching the intended interleaving. Please save/restore this
config and set it to at least 2 before constructing `service_`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
