-
Notifications
You must be signed in to change notification settings - Fork 517
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
feat: support parallel compress snapshot #2136
Conversation
...src/main/java/org/apache/hugegraph/backend/store/raft/compress/ParallelCompressStrategy.java
Fixed
Show resolved
Hide resolved
Hi, thanks for the contribution, for better review and reduce a lot of time in and also fix the CI problems -> add license header & fix security problem as the CI alerts (could copy from the exist file) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Codecov Report
@@ Coverage Diff @@
## master #2136 +/- ##
============================================
+ Coverage 62.60% 68.67% +6.06%
+ Complexity 728 676 -52
============================================
Files 481 491 +10
Lines 39704 40217 +513
Branches 5581 5615 +34
============================================
+ Hits 24857 27618 +2761
+ Misses 12389 9935 -2454
- Partials 2458 2664 +206
... and 88 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution, it's a nice enhancement.
Some tiny comments.
.../src/main/java/org/apache/hugegraph/backend/store/raft/compress/CompressStrategyManager.java
Show resolved
Hide resolved
.../src/main/java/org/apache/hugegraph/backend/store/raft/compress/CompressStrategyManager.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hugegraph/backend/store/raft/compress/ParallelCompressStrategy.java
Show resolved
Hide resolved
...ph-core/src/main/java/org/apache/hugegraph/backend/store/raft/compress/CompressStrategy.java
Show resolved
Hide resolved
...src/main/java/org/apache/hugegraph/backend/store/raft/compress/ParallelCompressStrategy.java
Outdated
Show resolved
Hide resolved
Thanks for your review, code style has fixed, but |
// add parallel compress strategy | ||
if (compressStrategies[PARALLEL_STRATEGY] == null) { | ||
final CompressStrategy compressStrategy = new ParallelCompressStrategy( | ||
config.get(CoreOptions.RAFT_SNAPSHOT_COMPRESS_THREADS), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can align with "CompressStrategy" or "compressStrategy"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.../src/main/java/org/apache/hugegraph/backend/store/raft/compress/CompressStrategyManager.java
Outdated
Show resolved
Hide resolved
config.get(CoreOptions.RAFT_SNAPSHOT_COMPRESS_THREADS), | ||
config.get(CoreOptions.RAFT_SNAPSHOT_DECOMPRESS_THREADS)); | ||
CompressStrategyManager.addCompressStrategy( | ||
CompressStrategyManager.PARALLEL_STRATEGY, compressStrategy); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
align
...src/main/java/org/apache/hugegraph/backend/store/raft/compress/ParallelCompressStrategy.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/hugegraph/backend/store/raft/compress/CompressStrategyManager.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/hugegraph/backend/store/raft/compress/CompressStrategyManager.java
Outdated
Show resolved
Hide resolved
CI error: [INFO] Running org.apache.hugegraph.api.ApiTestSuite
Error: Tests run: 69, Failures: 32, Errors: 1, Skipped: 0, Time elapsed: 2,511.93 s <<< FAILURE! - in org.apache.hugegraph.api.ApiTestSuite
Error: testTruncate(org.apache.hugegraph.api.GremlinApiTest) Time elapsed: 1.173 s <<< FAILURE!
java.lang.AssertionError: Response with status 500 and content {"exception":"java.lang.IllegalStateException","message":"The snapshot future can't be null","cause":"[java.lang.IllegalStateException]"} expected:<200> but was:<500>
at org.apache.hugegraph.api.GremlinApiTest.testTruncate(GremlinApiTest.java:139) |
improve #2116