github-actions[bot] commented on code in PR #66448:
URL: https://github.com/apache/doris/pull/66448#discussion_r3713640704
##########
be/src/storage/tablet/tablet.cpp:
##########
@@ -3268,8 +3268,7 @@ Status Tablet::prepare_txn(TPartitionId partition_id,
TTransactionId transaction
}
std::lock_guard<std::mutex> push_lock(get_push_lock());
- return _engine.txn_manager()->prepare_txn(partition_id, transaction_id,
tablet_id(),
- tablet_uid(), load_id, ingest);
+ return _engine.txn_manager()->prepare_txn(partition_id, *this,
transaction_id, load_id, ingest);
Review Comment:
[P1] Revalidate captured tablets through commit — the overload invoked here
reads plain `TabletMeta::_tablet_state`, but bad-tablet repair and manual
reload/replacement do not share this migration/push fence. A load that already
captured the `TabletSharedPtr` can observe `RUNNING`, be preempted while the
object is shut down or replaced with a new UID, and then insert the retired
identity; the later commit path also has no state/current-object revalidation.
FE drop's rollback is only a one-shot scan and can finish before that
insertion. Please fence the full prepare-to-commit lifecycle or revalidate that
the tablet ID/UID is still current at both boundaries, with a deterministic
interleaving test.
##########
be/src/storage/tablet/tablet.cpp:
##########
@@ -3268,8 +3268,7 @@ Status Tablet::prepare_txn(TPartitionId partition_id,
TTransactionId transaction
}
std::lock_guard<std::mutex> push_lock(get_push_lock());
- return _engine.txn_manager()->prepare_txn(partition_id, transaction_id,
tablet_id(),
- tablet_uid(), load_id, ingest);
+ return _engine.txn_manager()->prepare_txn(partition_id, *this,
transaction_id, load_id, ingest);
Review Comment:
[P1] Fence slave pulls before allowing migration — this restored overload
protects only callers that reach `Tablet::prepare_txn`.
`request_slave_tablet_pull_rowset` captures the old tablet, performs the full
download, and then directly calls the raw `commit_txn` overload without prepare
or a migration lock. Migration can therefore see no running transaction,
replace the tablet, and release its lock before the slave commits and
acknowledges the retired UID; publish then fails the exact `(tablet_id,
tablet_uid)` lookup. Please register/prepare this slave transaction before the
download with reliable rollback, or add an equivalent lifecycle fence that does
not hold a lock across network I/O.
--
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]
