Copilot commented on code in PR #8138:
URL: https://github.com/apache/incubator-seata/pull/8138#discussion_r3379480948
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/fence/store/CommonFenceStore.java:
##########
@@ -74,12 +74,15 @@ public interface CommonFenceStore {
boolean deleteCommonFenceDO(Connection conn, String xid, Long branchId);
/**
- * Delete tcc fence do boolean.
+ * Delete tcc fence by the given xids, restricted to expired end-status
rows.
+ * The datetime and end-status predicates guard against removing sibling
branch rows of the same xid
+ * that are still in progress (TRIED) or not yet expired.
* @param conn the connection
* @param xids the global transaction ids
- * @return the boolean
+ * @param datetime the expiry threshold; only rows with gmt_modified
before this are deleted
+ * @return the deleted row count
*/
- int deleteTCCFenceDO(Connection conn, List<String> xids);
+ int deleteTCCFenceDO(Connection conn, List<String> xids, Date datetime);
Review Comment:
`CommonFenceStore` is a public interface in `integration-tx-api`. Changing
the `deleteTCCFenceDO` method signature is a breaking API/SPI change for any
external `CommonFenceStore` implementations, even though the in-repo
implementation/callers were updated. Consider keeping the old signature
(possibly deprecated) and adding an overloaded method (or a default method)
that accepts `datetime`, so existing implementations remain source/binary
compatible while allowing the cleanup path to pass the expiry threshold.
##########
integration-tx-api/src/main/java/org/apache/seata/integration/tx/api/fence/store/db/CommonFenceStoreDataBaseDAO.java:
##########
@@ -189,6 +189,8 @@ public int deleteTCCFenceDO(Connection conn, List<String>
xids) {
for (int i = 0; i < xids.size(); i++) {
Review Comment:
`deleteTCCFenceDO` builds an `IN (...)` clause from `xids.size()`. If `xids`
is null/empty, the generated SQL becomes invalid (e.g. `xid in ()`) and will
throw at runtime. Since this is part of a public store API, it should
defensively handle empty input and fail fast on a null `datetime` to avoid
unclear NPEs.
--
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]
