Skip to content

feat: support parallel compress snapshot - #2136

Merged
imbajin merged 11 commits into
apache:masterfrom
wuchaojing:master
Mar 14, 2023
Merged

imbajin merged 11 commits into
apache:masterfrom
wuchaojing:master

Conversation

@wuchaojing

Copy link
Copy Markdown
Contributor

improve #2116

@imbajin

imbajin commented Mar 1, 2023 •

Copy link
Copy Markdown
Member

Hi, thanks for the contribution, for better review and reduce a lot of time in code-style problems,
It's highly recommended to use our style config if u use IDEA, import the style file and reformat your code 📑

and also fix the CI problems -> add license header & fix security problem as the CI alerts (could copy from the exist file)

@JackyYangPassion

This comment was marked as off-topic.

@codecov

codecov Bot commented Mar 2, 2023 •

Copy link
Copy Markdown

Codecov Report

Merging #2136 (d475b22) into master (76fa644) will increase coverage by 6.06%.
The diff coverage is 19.01%.

@@             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     
Impacted Files Coverage Δ
.../store/raft/compress/ParallelCompressStrategy.java 0.00% <0.00%> (ø)
...ugegraph/backend/store/raft/StoreSnapshotFile.java 41.88% <32.00%> (-0.37%) ⬇️
...d/store/raft/compress/CompressStrategyManager.java 40.00% <40.00%> (ø)
...nd/store/raft/compress/SerialCompressStrategy.java 62.50% <62.50%> (ø)
...ache/hugegraph/backend/store/raft/RaftContext.java 81.25% <100.00%> (+0.09%) ⬆️
.../java/org/apache/hugegraph/config/CoreOptions.java 99.49% <100.00%> (+0.02%) ⬆️

... and 88 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@javeme javeme left a comment

Copy link
Copy Markdown
Contributor

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.

@wuchaojing

Copy link
Copy Markdown
Contributor Author

Hi, thanks for the contribution, for better review and reduce a lot of time in code-style problems, It's highly recommended to use our style config if u use IDEA, import the style file and reformat your code 📑

and also fix the CI problems -> add license header & fix security problem as the CI alerts (could copy from the exist file)

Thanks for your review, code style has fixed, but fix security problem as the CI alerts I don't know how to fix it

// add parallel compress strategy
if (compressStrategies[PARALLEL_STRATEGY] == null) {
final CompressStrategy compressStrategy = new ParallelCompressStrategy(
config.get(CoreOptions.RAFT_SNAPSHOT_COMPRESS_THREADS),

Copy link
Copy Markdown
Contributor

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"

Copy link
Copy Markdown
Member

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"

It's better not to use the manually align, it will break the auto-format style and greatly raise the threshold of contributing code (Otherwise we could define a automatic rule for it)

image

config.get(CoreOptions.RAFT_SNAPSHOT_COMPRESS_THREADS),
config.get(CoreOptions.RAFT_SNAPSHOT_DECOMPRESS_THREADS));
CompressStrategyManager.addCompressStrategy(
CompressStrategyManager.PARALLEL_STRATEGY, compressStrategy);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

align

@javeme javeme added the raft label Mar 6, 2023
javeme
javeme previously approved these changes Mar 8, 2023
@javeme

javeme commented Mar 10, 2023

Copy link
Copy Markdown
Contributor

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)

@imbajin
imbajin merged commit 82c7e78 into apache:master Mar 14, 2023
@imbajin imbajin linked an issue Mar 31, 2023 that may be closed by this pull request
1 task done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Question] 如何提升snapshot压缩/解压效率

5 participants