Copilot commented on code in PR #1085:
URL:
https://github.com/apache/incubator-seata-go/pull/1085#discussion_r3045266128
##########
pkg/rm/tcc/fence/handler/tcc_fence_wrapper_handler.go:
##########
@@ -243,16 +267,21 @@ func (handler *tccFenceWrapperHandler)
deleteBatchFence(tx *sql.Tx, batch []mode
}
func (handler *tccFenceWrapperHandler) pushCleanChannel(xid string, branchId
int64) {
- // todo implement
fli := &model.FenceLogIdentity{
Xid: xid,
BranchId: branchId,
}
select {
case handler.logQueue <- fli:
- // todo add batch delete from log cache.
default:
- handler.logCache.PushBack(fli)
+ handler.cacheMutex.Lock()
+ handler.logCache.PushBack(&fenceLogCacheEntry{identity: *fli})
+ handler.cacheMutex.Unlock()
+
+ handler.logCacheOnce.Do(func() {
+ handler.stopDrainCache = make(chan struct{})
+ go handler.drainCacheTask()
+ })
}
log.Infof("add one log to clean queue: %v ", fli)
}
Review Comment:
The log message always says the fence log was added to the "clean queue",
but in the queue-full path the identity is cached (logCache) instead. Consider
logging which path was taken (queued vs cached) to avoid misleading operational
signals under queue pressure.
##########
pkg/rm/tcc/fence/handler/tcc_fence_wrapper_handler.go:
##########
@@ -225,6 +246,9 @@ func (handler *tccFenceWrapperHandler)
enqueueFenceLogIdentities(identityList []
func (handler *tccFenceWrapperHandler) DestroyLogCleanChannel() {
handler.logQueueCloseOnce.Do(func() {
close(handler.logQueue)
+ if handler.stopDrainCache != nil {
+ close(handler.stopDrainCache)
+ }
handler.dbMutex.Lock()
Review Comment:
DestroyLogCleanChannel can panic by calling close(handler.logQueue) when
logQueue is still nil (it’s only initialized inside traversalCleanChannel).
Additionally, the stopDrainCache close has a race: DestroyLogCleanChannel can
run before pushCleanChannel’s logCacheOnce.Do assigns stopDrainCache, allowing
drainCacheTask to start after Destroy and leak forever. Consider (1) guarding
close(logQueue) with a nil check, and (2) initializing stopDrainCache eagerly
(or creating it via a separate once) and closing it via a dedicated close-once
so it cannot be missed even under concurrent Destroy/pushCleanChannel.
--
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]
