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: add LZ4 compressor #2510

Merged
merged 6 commits into from
Apr 29, 2020
Merged

feature: add LZ4 compressor #2510

merged 6 commits into from
Apr 29, 2020

Conversation

diguage
Copy link
Contributor

@diguage diguage commented Apr 4, 2020

LZ4 is an extremely fast Compression algorithm. Maybe it should be supported.

@diguage diguage force-pushed the lz4-compressor branch 3 times, most recently from a44a3fb to c41baea Compare April 4, 2020 14:56
@zjinlei zjinlei added the first-time contributor first-time contributor label Apr 4, 2020
@zjinlei zjinlei changed the title add LZ4 compressor feature: add LZ4 compressor Apr 4, 2020
@zjinlei
Copy link
Contributor

zjinlei commented Apr 4, 2020

please fix CI error:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:check (validate) on project seata-compressor-lz4: Failed during checkstyle execution: There are 4 errors reported by Checkstyle 6.18 with /home/travis/build/seata/seata/style/seata_checkstyle.xml ruleset.

@funky-eyes
Copy link
Contributor

please format the code according to the p3c specification

@diguage diguage force-pushed the lz4-compressor branch 2 times, most recently from f78f4dc to cac665f Compare April 7, 2020 03:46
@diguage
Copy link
Contributor Author

diguage commented Apr 7, 2020

please fix CI error:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:check (validate) on project seata-compressor-lz4: Failed during checkstyle execution: There are 4 errors reported by Checkstyle 6.18 with /home/travis/build/seata/seata/style/seata_checkstyle.xml ruleset.

There is an error. And there is no report. So I cannot find it.

@a364176773 I use p3c to check the code. There is not any warning.

Please help me.

@funky-eyes
Copy link
Contributor

please fix CI error:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:check (validate) on project seata-compressor-lz4: Failed during checkstyle execution: There are 4 errors reported by Checkstyle 6.18 with /home/travis/build/seata/seata/style/seata_checkstyle.xml ruleset.

There is an error. And there is no report. So I cannot find it.

@a364176773 I use p3c to check the code. There is not any warning.

Please help me.

please check the code format and local mvn clean install -DskipTests=true

@diguage
Copy link
Contributor Author

diguage commented Apr 7, 2020

please check the code format and local mvn clean install -DskipTests=true

I ran the command. The log had an error. But did not show the detail.

The log is as following:

...
[INFO] seata-spring-boot-starter 1.2.0-SNAPSHOT ........... SUCCESS [  1.714 s]
[INFO] seata-compressor-lz4 1.2.0-SNAPSHOT ................ FAILURE [  0.235 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:27 min
[INFO] Finished at: 2020-04-07T12:39:15+08:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:check (validate) on project seata-compressor-lz4: Failed during checkstyle execution: There is 1 error reported by Checkstyle 6.18 with /Users/diguage/develop/java/javaprojects/seata/style/seata_checkstyle.xml ruleset. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :seata-compressor-lz4

@funky-eyes
Copy link
Contributor

please check the code format and local mvn clean install -DskipTests=true

I ran the command. The log had an error. But did not show the detail.

The log is as following:

...
[INFO] seata-spring-boot-starter 1.2.0-SNAPSHOT ........... SUCCESS [  1.714 s]
[INFO] seata-compressor-lz4 1.2.0-SNAPSHOT ................ FAILURE [  0.235 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  01:27 min
[INFO] Finished at: 2020-04-07T12:39:15+08:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.0.0:check (validate) on project seata-compressor-lz4: Failed during checkstyle execution: There is 1 error reported by Checkstyle 6.18 with /Users/diguage/develop/java/javaprojects/seata/style/seata_checkstyle.xml ruleset. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <goals> -rf :seata-compressor-lz4

scroll up to find the corresponding module log, which should have a prompt message

@diguage
Copy link
Contributor Author

diguage commented Apr 7, 2020

scroll up to find the corresponding module log, which should have a prompt message

I saw the seata/compressor/seata-compressor-lz4/target/checkstyle-result.xml file. There is a tip.

<file name="/Users/diguage/develop/java/javaprojects/seata/compressor/seata-compressor-lz4/src/main/java/io/seata/compressor/lz4/Lz4Util.java">
<error line="18" severity="error" message="Using the &apos;.*&apos; form of import should be avoided - net.jpountz.lz4.*." source="com.puppycrawl.tools.checkstyle.checks.imports.AvoidStarImportCheck"/>
</file>

Thank you very much.

@slievrly
Copy link
Member

slievrly commented Apr 7, 2020

@diguage mvn clean install -DskipTests=true -Dmaven.javadoc.skip=true -Dcheckstyle.skip=false

@codecov-io
Copy link

codecov-io commented Apr 7, 2020

Codecov Report

Merging #2510 into develop will increase coverage by 0.03%.
The diff coverage is 76.66%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2510      +/-   ##
=============================================
+ Coverage      51.18%   51.21%   +0.03%     
- Complexity      2806     2813       +7     
=============================================
  Files            554      556       +2     
  Lines          17772    17801      +29     
  Branches        2099     2100       +1     
=============================================
+ Hits            9096     9117      +21     
- Misses          7819     7825       +6     
- Partials         857      859       +2     
Impacted Files Coverage Δ Complexity Δ
...in/java/io/seata/compressor/sevenz/SevenZUtil.java 82.14% <ø> (ø) 6.00 <0.00> (ø)
...main/java/io/seata/compressor/bzip2/BZip2Util.java 76.19% <ø> (ø) 5.00 <0.00> (ø)
.../java/io/seata/core/compressor/CompressorType.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...src/main/java/io/seata/compressor/lz4/Lz4Util.java 80.00% <80.00%> (ø) 6.00 <6.00> (?)
...in/java/io/seata/compressor/lz4/Lz4Compressor.java 100.00% <100.00%> (ø) 3.00 <3.00> (?)
...o/seata/server/coordinator/DefaultCoordinator.java 54.63% <0.00%> (-0.52%) 28.00% <0.00%> (-1.00%)
...in/java/io/seata/server/session/GlobalSession.java 83.71% <0.00%> (-0.46%) 71.00% <0.00%> (-1.00%)

@diguage
Copy link
Contributor Author

diguage commented Apr 7, 2020

@diguage mvn clean install -DskipTests=true -Dmaven.javadoc.skip=true -Dcheckstyle.skip=false

Thank you. I found the error, and fixed it. Thanks again.

@zjinlei zjinlei added this to the 1.2.0 milestone Apr 8, 2020
Copy link
Contributor

@ph3636 ph3636 left a comment

Choose a reason for hiding this comment

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

Add enumeration to CompressorType

compressor/seata-compressor-lz4/pom.xml Outdated Show resolved Hide resolved
throw new NullPointerException("bytes is null");
}

ByteArrayOutputStream outputStream = new ByteArrayOutputStream(ARRAY_SIZE);
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need to initialize such a large value, just keep the default value

Copy link
Contributor Author

@diguage diguage Apr 8, 2020

Choose a reason for hiding this comment

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

The default size is 32. Is it too small?
Maybe I can change the ARRAY_SIZE to 1024. Is it OK?

@diguage diguage force-pushed the lz4-compressor branch 5 times, most recently from 7d855c5 to 481b9e0 Compare April 10, 2020 12:46
@slievrly slievrly modified the milestones: 1.2.0, 1.3.0 Apr 12, 2020
@ph3636
Copy link
Contributor

ph3636 commented Apr 12, 2020

seata-all file is missing some configuration

@diguage
Copy link
Contributor Author

diguage commented Apr 13, 2020

seata-all file is missing some configuration

OK

@zjinlei
Copy link
Contributor

zjinlei commented Apr 13, 2020

Please keep the commit log, we can only review the increment.

@ph3636
Copy link
Contributor

ph3636 commented Apr 13, 2020

seata-all file is missing some configuration

OK

The 682 line of seata-al file is missing some configuration

@diguage
Copy link
Contributor Author

diguage commented Apr 13, 2020

The 682 line of seata-all file is missing some configuration

OK 👌

@diguage
Copy link
Contributor Author

diguage commented Apr 13, 2020

Please keep the commit log, we can only review the increment.
👌 Thanks.

Copy link
Contributor

@ph3636 ph3636 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

@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
Agree with @ph3636

all/pom.xml Show resolved Hide resolved
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 58aed9e into apache:develop Apr 29, 2020
@diguage diguage deleted the lz4-compressor branch April 29, 2020 07:55
xiaowan pushed a commit to xiaowan/seata that referenced this pull request Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants