Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: support XA transaction mode #2381

Merged
merged 68 commits into from
Apr 9, 2020
Merged

Conversation

sharajava
Copy link
Contributor

Ⅰ. Describe what this PR did

Provide XA mode to Seata. Issue #2022

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

all test cases can be found in XAModeTest2

Ⅴ. Special notes for reviews

# Conflicts:
#	server/src/main/java/com/alibaba/fescar/server/coordinator/DefaultCoordinator.java
…try other application on the same resource.
# Conflicts:
#	test/src/main/java/com/alibaba/fescar/test/DataSourceBasicTest.java
# Conflicts:
#	rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/sql/druid/MySQLInsertRecognizer.java
#	rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/sql/struct/TableMetaCache.java
#	test/src/main/java/com/alibaba/fescar/test/DataSourceBasicTest.java
@sharajava sharajava requested a review from zhangthen March 9, 2020 13:56
@sharajava sharajava self-assigned this Mar 9, 2020
@sharajava sharajava added the type: feature Category issues or prs related to feature request. label Mar 9, 2020
@sharajava sharajava requested a review from slievrly March 9, 2020 13:57
private static final String mockXid = "127.0.0.1:8091:" + testTid;
private static final long mockBranchId = testTid + 1;

private static final String pg_jdbcUrl = "jdbc:postgresql://127.0.0.1:5432/postgres";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unable to connect to 127.0.0.1 database, CI occurred a large number of error logs. Please use mock or disable, and remove all printf logs.

Copy link
Contributor

@zhangthen zhangthen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job~

@apache apache deleted a comment from codecov-io Mar 23, 2020
@codecov-io
Copy link

codecov-io commented Mar 23, 2020

Codecov Report

Merging #2381 into develop will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop    #2381   +/-   ##
==========================================
  Coverage      50.94%   50.94%           
  Complexity      2763     2763           
==========================================
  Files            551      551           
  Lines          17569    17569           
  Branches        2063     2063           
==========================================
  Hits            8950     8950           
  Misses          7775     7775           
  Partials         844      844           

@sharajava
Copy link
Contributor Author

Samples For XA mode: apache/incubator-seata-samples#324

Copy link
Contributor

@zhangthen zhangthen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

看了设计方案,本地跑了demo,无问题

rm-datasource/pom.xml Show resolved Hide resolved
XAXid xaBranchXid = XAXidBuilder.build(xid, branchId);
Resource resource = dataSourceCache.get(resourceId);
if (resource instanceof AbstractDataSourceProxyXA) {
try (ConnectionProxyXA connectionProxyXA = ((AbstractDataSourceProxyXA)resource).getConnectionForXAFinish(xaBranchXid)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clientA, clientB->TC clientA branchCommit but clientA ->TC channel is bad and XAConn is ok, so TC send request to clientB.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, such cases should be considered in enhancement later.

Just keep the basic logic by now.

return BranchStatus.PhaseTwo_Rollbacked;
}
} catch (XAException xe) {
throw new TransactionException("XA Exception " + xe.getMessage(), xe);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw exception will result in encode error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged SQLException and XAException cases.

}
}
} else {
throw new TransactionException("Unknown Resource for XA resource " + resourceId + " " + resource);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

throw exception will result in encode error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, this is a case that SHOULD NEVER happen.

Just merged XAException and SQLException into retryable status.

@slievrly
Copy link
Member

slievrly commented Apr 7, 2020

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@slievrly slievrly merged commit 084708c into apache:develop Apr 9, 2020
@slievrly slievrly added this to the 1.2.0 milestone Apr 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature Category issues or prs related to feature request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants