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.

Sign up for GitHub

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

Jump to bottom

refactor : separate the different storage pattern processing logic #2329

Merged
merged 12 commits into from
Mar 20, 2020
Merged

refactor : separate the different storage pattern processing logic #2329

merged 12 commits into from
Mar 20, 2020

Conversation

ph3636
Copy link
Contributor

@ph3636 ph3636 commented Mar 2, 2020

No description provided.

@codecov-io
Copy link

codecov-io commented Mar 2, 2020 •
edited

Codecov Report

Merging #2329 into develop will decrease coverage by 0.23%.
The diff coverage is 64.38%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2329      +/-   ##
=============================================
- Coverage      51.80%   51.57%   -0.24%     
+ Complexity      2705     2665      -40     
=============================================
  Files            518      517       -1     
  Lines          16822    16781      -41     
  Branches        2033     1992      -41     
=============================================
- Hits            8715     8655      -60     
+ Misses          7293     7286       -7     
- Partials         814      840      +26
Impacted Files Coverage Δ Complexity Δ
...java/io/seata/server/lock/AbstractLockManager.java 64.40% <ø> (-0.60%) 13.00 <0.00> (-1.00)
...ta/server/storage/db/lock/DataBaseLockManager.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...o/seata/server/storage/db/lock/DataBaseLocker.java 16.27% <0.00%> (ø) 5.00 <0.00> (?)
...a/server/storage/db/lock/LockStoreDataBaseDAO.java 57.21% <0.00%> (ø) 26.00 <0.00> (?)
...io/seata/server/storage/db/lock/LockStoreSqls.java 18.75% <ø> (ø) 3.00 <0.00> (?)
...ver/storage/db/session/DataBaseSessionManager.java 34.42% <0.00%> (ø) 12.00 <0.00> (?)
...a/server/storage/db/store/LogStoreDataBaseDAO.java 71.66% <ø> (ø) 34.00 <0.00> (?)
...io/seata/server/storage/db/store/LogStoreSqls.java 18.46% <ø> (ø) 11.00 <0.00> (?)
...va/io/seata/server/storage/file/FlushDiskMode.java 77.77% <ø> (ø) 3.00 <0.00> (?)
...ata/server/storage/file/TransactionWriteStore.java 83.33% <ø> (ø) 8.00 <0.00> (?)
... and 52 more

objcoding
objcoding reviewed Mar 8, 2020
View reviewed changes
server/src/main/java/io/seata/server/lock/LockerManagerFactory.java Outdated
/**
* the lock manager
*/
private static LockManager lockManager = EnhancedServiceLoader.load(LockManager.class,
Copy link
Contributor

Choose a reason for hiding this comment

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

Uppercase static constants.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

objcoding
objcoding reviewed Mar 8, 2020
View reviewed changes
Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

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

review is ok:
1.LockerManagerFactory
2.locker: FileLocker, DBLocker

@zjinlei zjinlei added this to the 1.2.0 milestone Mar 12, 2020
objcoding
objcoding approved these changes Mar 18, 2020
View reviewed changes
Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

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

LGTM.

objcoding
objcoding reviewed Mar 18, 2020
View reviewed changes
Copy link
Contributor

@objcoding objcoding left a comment

Choose a reason for hiding this comment

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

  1. Test case passed.
  2. seata-sample passed.

slievrly
slievrly reviewed Mar 19, 2020
View reviewed changes
server/src/main/java/io/seata/server/storage/db/lock/DataBaseLockManager.java Outdated Show resolved Hide resolved
server/src/main/java/io/seata/server/lock/AbstractLockManager.java Outdated
@@ -106,7 +111,7 @@ protected Locker getLocker() {
* @return the locker
*/
protected Locker getLocker(BranchSession branchSession) {
Copy link
Member

Choose a reason for hiding this comment

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

why not abstract method？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@slievrly
Copy link
Member

slievrly commented Mar 19, 2020 •
edited

  1. LockStore 取消SPI，new LockStoreDataBaseDAO 代替，通过DataBaseLockManager SPI 入口控制。

  2. TransactionStoreManager 取消SPI，new DataBaseTransactionStoreManager，FileTransactionStoreManager，通过 DataBaseSessionManager FileSessionManager SPI控制入口

  3. 去除 DefaultSessionManager 并入 FileSessionManager 通过路径判断是否持久化还是内存初始化不同transactionStoreManager

slievrly
slievrly approved these changes Mar 19, 2020
View reviewed changes
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

ph3636 and others added 2 commits March 20, 2020 13:16
zjinlei
zjinlei reviewed Mar 20, 2020
View reviewed changes
server/src/main/java/io/seata/server/storage/db/session/DataBaseSessionManager.java Outdated
@@ -79,7 +77,7 @@ public DataBaseSessionManager(String name) {

@Override
public void init() {
transactionStoreManager = EnhancedServiceLoader.load(TransactionStoreManager.class, StoreMode.DB.name());
transactionStoreManager = new DataBaseTransactionStoreManager();
Copy link
Contributor

Choose a reason for hiding this comment

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

Will be initialized 4 times by SessionHolder, it should be static.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

zjinlei
zjinlei approved these changes Mar 20, 2020
View reviewed changes
Copy link
Contributor

@zjinlei zjinlei left a comment

Choose a reason for hiding this comment

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

LGTM

@zjinlei zjinlei merged commit 44cbf05 into apache:develop Mar 20, 2020
@ph3636 ph3636 deleted the separate_store_mode branch March 27, 2020 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@objcoding objcoding objcoding approved these changes

@slievrly slievrly slievrly approved these changes

@zjinlei zjinlei zjinlei approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@ph3636 @codecov-io @slievrly @zjinlei @objcoding