github-actions[bot] commented on code in PR #68514:
URL: https://github.com/apache/doris/pull/68514#discussion_r4106633250
##########
fe/fe-core/src/main/java/org/apache/doris/transaction/DatabaseTransactionMgr.java:
##########
@@ -911,17 +911,21 @@ public void commitTransaction(List<Table> tableList, long
transactionId, List<Ta
checkCommitStatus(tableList, transactionState, tabletCommitInfos,
txnCommitAttachment, errorReplicaIds,
tableToPartition, totalInvolvedBackends);
}
+ // Fetch before callbacks can acquire job locks or mark a job as
committing. A disabled or
+ // unavailable TSO service must fail without leaving callback state
behind.
+ long commitTSO = TransactionUtil.getCommitTSO(transactionId, db, is2PC
Review Comment:
[P1] Recheck the transaction state after allocating the TSO
This call now creates a blocking interval outside `synchronized
(transactionState)`. The timeout daemon can run `abortTransaction` in that
interval without taking the caller's table locks and persist `ABORTED`. When
this thread resumes, each `unprotectedCommitTransaction*` helper simply returns
for the non-PREPARE/non-PRECOMMITTED state, but the caller still persists, sets
`txnOperated = true`, invokes the COMMITTED callback, and logs a successful
commit. For a streaming insert, `afterCommitted(..., true)` then advances the
source offset even though the data transaction was aborted and cleared. Please
make the monitored transition report whether it actually committed (and only
persist/callback/update on that result), covering normal, 2PC, and
subtransaction paths with an abort-vs-TSO interleaving test.
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/BinlogManager.java:
##########
@@ -176,10 +172,6 @@ private void addBinlog(TBinlog binlog, Object raw) {
private void addBinlog(long dbId, List<Long> tableIds, long commitSeq,
long timestamp, TBinlogType type,
String data, boolean removeEnableCache, Object raw) {
Review Comment:
[P1] Evaluate CCR disables against the pre-update configuration
After image recovery this cache is cold. The live and replay property paths
install the new database or table config before calling the binlog manager, so
a first-operation CCR disable makes the eligibility lookup load the
already-disabled value and drop the terminal ALTER_DATABASE_PROPERTY or
MODIFY_TABLE_PROPERTY event. The later `removeEnableCache` handling cannot
recover it; whether the event exists currently depends on an unrelated
operation having warmed the cache with the old value. Please carry/evaluate the
pre-update config (or seed it deterministically during recovery) and add
restore-then-disable tests for both scopes.
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/BinlogManager.java:
##########
@@ -145,10 +145,6 @@ private boolean isTemporaryTable(TBinlog binlog) {
}
private void addBinlog(TBinlog binlog, Object raw) {
- if (!Config.enable_feature_binlog) {
Review Comment:
[P1] Classify DROP records before unregistering the table
Removing this gate sends DROP records through filters that consult the live
catalog, but the production path calls `unprotectDropTable` before
`logDropTable`. With a cold cache (notably after upgrading an image written
while this flag was false), a table-scoped CCR table under a disabled database
can no longer be resolved and its DROP plus tombstone are lost. In the opposite
direction, database-level CCR makes a temporary table's DROP eligible, but
`isTemporaryTable` now returns false, so an orphan DROP is emitted even though
CREATE was filtered; async-MV classification has the same cold-cache
dependency. Please capture effective CCR eligibility and exclusion/type before
unregistering and carry them through normal and replay DROP, with actual-DDL
cold-cache tests rather than calling `addDropTableRecord` while the table still
exists.
--
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]
