feat(core): support load vertex/edge snapshot #269
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Radeity
force-pushed
the
Feature-239
branch
from
9f9a73c to
9a2fdb1
Compare
javeme
reviewed
Sep 24, 2023
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
computer-core/src/main/java/org/apache/hugegraph/computer/core/snapshot/SnapshotManager.java
Outdated
Show resolved
Hide resolved
|
Hi @javeme , thanks for your comments, I will pay more attention at next time to write beautiful codes : ) |
Radeity
force-pushed
the
Feature-239
branch
from
75da7fc to
8b6000c
Compare
Radeity
force-pushed
the
Feature-239
branch
from
8b6000c to
6e1e444
Compare
javeme
reviewed
Sep 30, 2023
computer-test/src/main/java/org/apache/hugegraph/computer/core/compute/ComputeManagerTest.java
Outdated
Show resolved
Hide resolved
computer-api/src/main/java/org/apache/hugegraph/computer/core/config/ComputerOptions.java
Outdated
Show resolved
Hide resolved
computer-api/src/main/java/org/apache/hugegraph/computer/core/config/ComputerOptions.java
Outdated
Show resolved
Hide resolved
...uter-test/src/main/java/org/apache/hugegraph/computer/core/compute/input/EdgesInputTest.java
Outdated
Show resolved
Hide resolved
| public static final ConfigOption<String> SNAPSHOT_VIEW_KEY = | ||
| new ConfigOption<>( | ||
| "snapshot.view_key", | ||
| "View key of target snapshot", |
There was a problem hiding this comment.
what does the view key mean? do we have a concept that is easier to understand?
There was a problem hiding this comment.
Maybe we can use SNAPSHOT_NAME instead, just a user-defined unique identifier of the snapshot.
There was a problem hiding this comment.
Hi @javeme , I wanna make sure is there any other confusion about it before make changes.
There was a problem hiding this comment.
get it. SNAPSHOT_NAME seems easy to understand
computer-api/src/main/java/org/apache/hugegraph/computer/core/config/ComputerOptions.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #269 +/- ##
============================================
- Coverage 85.81% 85.15% -0.66%
- Complexity 3233 3248 +15
============================================
Files 344 345 +1
Lines 12115 12283 +168
Branches 1092 1101 +9
============================================
+ Hits 10396 10460 +64
- Misses 1195 1297 +102
- Partials 524 526 +2
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Radeity
changed the title
coderzc
reviewed
Oct 19, 2023
| for (int partitionId = 0; partitionId < this.partitionCount; partitionId++) { | ||
| if (this.partitioner.workerId(partitionId) == id) { | ||
| // TODO: Do not need to send control message to all workers | ||
| this.sendManager.startSend(MessageType.VERTEX); |
There was a problem hiding this comment.
Why we need to send control message？
There was a problem hiding this comment.
We skip loadGraph() step when using snapshot, if we don't send control message, next steps will not go on.
coderzc
approved these changes
Oct 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Purpose of the PR
Main Changes
SnapshotManagerto handle snapshot load and write.Verifying these changes
Trivial rework / code cleanup without any test coverage. (No Need)
Already covered by existing tests, such as (please modify tests here).
Need tests and can be verified as follows.
For my LOCAL test, I deploy MinIO in standalone mode first. Then I test all possible scenarios when using this feature. And here's my testing results:
当前无快照可读
当前有快照可读
In later PR, I will support to deploy MinIO as internal storage. Then, I will add UT to replace manual test results.
Later PR (Task)
Does this PR potentially affect the following parts?
Documentation Status
Doc - TODODoc - DoneDoc - No Need