Seol-JY opened a new pull request, #8138: URL: https://github.com/apache/incubator-seata/pull/8138
- [x] I have read the [CONTRIBUTING.md](https://github.com/apache/incubator-seata/blob/2.x/CONTRIBUTING.md) guidelines. - [x] I have registered the PR [changes](https://github.com/apache/incubator-seata/tree/2.x/changes). ### Ⅰ. Describe what this PR did Fixes a correctness issue in the date-based TCC fence-log cleanup (`SpringFenceHandler#deleteFenceByDate`). The cleanup selects expired end-status xids via `queryEndStatusXidsByDate` (`gmt_modified < ?` AND `status in (COMMITTED, ROLLBACKED, SUSPENDED)`), but then deletes them with `DELETE ... WHERE xid IN (...)` — **with no date or status predicate**. Because the `tcc_fence_log` primary key is `(xid, branch_id)`, a single global transaction can own several branch rows. Deleting by xid alone therefore also removes sibling branch rows of the same xid that are still in progress (`TRIED`) or not yet expired — the very records that guard idempotency / anti-hanging. This PR: 1. Restricts the delete by `gmt_modified < ?` and end status, so only the rows the query intended are removed (the `DELETE_BY_BRANCH_XIDS` predicate now matches `QUERY_END_STATUS_BY_DATE`). The pre-existing but unused `DELETE_BY_DATE_AND_STATUS` constant — evidence the date/status filter was the original intent — is removed as it becomes redundant. 2. Changes the query to `select distinct xid`, so the row limit bounds the number of *distinct xids*. This keeps the `xidSet.size() < LIMIT_DELETE` loop-termination check in `deleteFenceByDate` consistent when one xid owns multiple branch rows (applies to MySQL/PostgreSQL/MariaDB via `limit ?`; on Oracle `ROWNUM` still bounds raw rows, i.e. unchanged / no regression). The delete still batches by xid (PK-prefix indexed) exactly as before, so there is no performance change. ### Ⅱ. Does this pull request fix one issue? No linked issue — this is a latent correctness bug found by code review. ### Ⅲ. Why don't you add test cases (unit test/integration test)? Added `CommonFenceStoreSqlsTest` (asserts the delete SQL carries the `gmt_modified` + end-status guard and the query selects distinct xids) and extended `CommonFenceStoreDataBaseDAOTest` (asserts the `gmt_modified` parameter is bound right after the xid placeholders). These pin the fix without introducing a new test dependency to this module. ### Ⅳ. Describe how to verify it With `useTCCFence=true` and a global transaction whose branches resolve to mixed states under one xid (e.g. branch A `COMMITTED` & expired, branch B `TRIED` or committed-but-recent), running the cleanup must delete only the expired end-status rows and preserve branch B. The added unit tests cover the SQL/binding contract; `mvn -pl integration-tx-api -am test` passes. ### Ⅴ. Special notes for reviews - `CommonFenceStore#deleteTCCFenceDO` gains a `Date datetime` parameter. The only implementation and caller are internal (`CommonFenceStoreDataBaseDAO`, `SpringFenceHandler`) and are both updated; there is no `compatible` mirror. -- 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]
