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

UndoLogParser change to SPI. #1099

Merged
merged 4 commits into from
May 26, 2019
Merged

UndoLogParser change to SPI. #1099

merged 4 commits into from
May 26, 2019

Conversation

ujjboy
Copy link
Contributor

@ujjboy ujjboy commented May 23, 2019

Ⅰ. Describe what this PR did

  • UndoLogParser change to SPI mode and make fastjson as default implement.
  • Add config transaction.undo.log.serialization
  • Fix one typo.
  • Add test cases.

Ⅱ. Does this pull request fix one issue?

Fix #1010

@codecov-io
Copy link

codecov-io commented May 23, 2019

Codecov Report

Merging #1099 into develop will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1099      +/-   ##
=============================================
- Coverage      41.84%   41.83%   -0.01%     
  Complexity      1361     1361              
=============================================
  Files            243      243              
  Lines          10111    10113       +2     
  Branches        1319     1319              
=============================================
  Hits            4231     4231              
- Misses          5334     5335       +1     
- Partials         546      547       +1
Impacted Files Coverage Δ Complexity Δ
...ata/rm/datasource/undo/JSONBasedUndoLogParser.java 100% <ø> (ø) 3 <0> (ø) ⬇️
...ava/io/seata/core/constants/ConfigurationKeys.java 0% <ø> (ø) 0 <0> (ø) ⬇️
...seata/rm/datasource/undo/UndoLogParserFactory.java 80% <100%> (+13.33%) 1 <0> (ø) ⬇️
...seata/rm/datasource/undo/AbstractUndoExecutor.java 83.15% <100%> (ø) 20 <0> (ø) ⬇️
...server/store/file/FileTransactionStoreManager.java 45.64% <0%> (-0.7%) 19% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b54c46...884dbd7. Read the comment docs.

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

i left some comments.

*/
public class UndoLogParserFactory {

private static class SingletonHolder {
private static final UndoLogParser INSTANCE = new JSONBasedUndoLogParser();
private static UndoLogParser INSTANCE;
Copy link
Member

Choose a reason for hiding this comment

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

maybe there should use final?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I' will modify it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xingfudeshi done.

server/src/main/resources/nacos-config.txt Show resolved Hide resolved
server/src/main/resources/nacos-config.txt Show resolved Hide resolved
Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

LGTM.

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.

+1

@zhangthen zhangthen merged commit 027fefe into apache:develop May 26, 2019
@ujjboy ujjboy deleted the spi_undo branch May 26, 2019 03:16
nick-tan pushed a commit to nick-tan/seata that referenced this pull request Jul 12, 2019
@wangliang181230 wangliang181230 added this to the 0.6.* milestone Aug 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UndoLogParser should use SPI
5 participants