AsperforMias commented on code in PR #1119:
URL: 
https://github.com/apache/incubator-seata-go/pull/1119#discussion_r3485948443


##########
pkg/datasource/sql/xa_resource_manager.go:
##########


Review Comment:
   这里是否也需要像 commit 一样缓存并短路 rollbacked 状态?目前 BranchCommit 会写入并读取 
BranchStatusPhasetwoCommitted 来保证幂等,但 BranchRollback 成功路径以及 
isBranchRollbacked(err) 分支都没有写入 BranchStatusPhasetwoRollbacked。对于 Oracle,held 
connection 被释放后,如果 TC 重试二阶段 rollback,就不能再稳定依赖本地连接状态判断终态,可能会从新连接上重新执行 XA 
rollback/commit 并得到不确定错误。感觉可以在 rollback 成功和“已 rollback”错误路径写入 rollbacked 
cache,并在方法入口遇到已 rollbacked 时直接返回成功。如果 commit 入口读到 rollbacked,也应返回 commit 
unretryable



##########
pkg/datasource/sql/xa/oracle_xa_connection.go:
##########
@@ -20,92 +20,325 @@ package xa
 import (
        "context"
        "database/sql/driver"
+       "encoding/hex"
+       "errors"
        "fmt"
+       "io"
+       "strconv"
+       "strings"
        "time"
 
        "seata.apache.org/seata-go/v2/pkg/datasource/sql/types"
-       "seata.apache.org/seata-go/v2/pkg/util/log"
 )
 
+const (
+       oracleXAMaxXIDPartSize = 64
+       oracleXAFormatID       = 0x53474F
+       oracleXAOK             = 0
+)
+
+var _ XAResource = (*OracleXAConn)(nil)
+
 func init() {
        RegisterXAResourceFactory(types.DBTypeOracle, 
&oracleXAResourceFactory{})
 }
 
 type oracleXAResourceFactory struct{}
 
 func (f *oracleXAResourceFactory) CreateXAResource(conn driver.Conn) 
XAResource {
-       return &OracleXAConn{Conn: conn}
+       return NewOracleXaConn(conn)
 }
 
 func (f *oracleXAResourceFactory) CreateErrorClassifier() XAErrorClassifier {
        return &OracleXAErrorClassifier{}
 }
 
-// OracleXAErrorClassifier classifies Oracle-specific XA errors.
+// OracleXAErrorClassifier recognizes Oracle errors that indicate an XA branch 
has already ended.
 type OracleXAErrorClassifier struct{}
 
 func (c *OracleXAErrorClassifier) IsAlreadyEnded(err error) bool {
-       // TODO: check ORA-24756 (transaction does not exist) / ORA-24761 
(rolled back)
-       return false
+       return oracleXAHasErrorCode(err, "24756") || oracleXAHasErrorCode(err, 
"24761")
+}
+
+func (c *OracleXAErrorClassifier) IsAlreadyCommitted(err error) bool {

Review Comment:
   这里直接把 ORA-24756 当成 already committed 会不会有点过。这个err更像是 “transaction does not 
exist”,不一定能完全证明之前已经 commit 成功。感觉这里要更完备的话,可以这样:要么只在本地已有 committed cache 
时当作幂等成功,要么再补个重复 commit 的 Oracle 集测来证明这个判断



-- 
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]

Reply via email to