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

Implement raft snapshot mechanism by rocksdb checkpoint #1247

Merged
merged 8 commits into from
Nov 18, 2020
Merged

Conversation

Linary
Copy link
Contributor

@Linary Linary commented Nov 9, 2020

Change-Id: I761c01da871aac6fe6e906f6150c66c7f5b8f67b

@Linary Linary force-pushed the fix-snapshot branch 2 times, most recently from f027175 to 8405036 Compare November 9, 2020 10:13
@@ -129,6 +129,7 @@ public void open(String name) {
@Override
public void waitStoreStarted() {
this.context.initRaftNode();
LOG.info("The raft node was inited");
Copy link
Contributor

Choose a reason for hiding this comment

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

The raft node is initialized

}
LOG.info("The store {} load snapshot succeed", this.store);
} catch (RocksDBException e) {
throw new BackendException("Failed to reload rocksdb: " + e.toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

use %s

@@ -586,21 +581,22 @@ protected Session session(HugeType tableType) {

@Override
public void writeSnapshot(String parentPath) {
Lock readLock = this.truncateLock.readLock();
readLock.lock();
Lock writeLock = this.truncateLock.writeLock();
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to storeLock

* it may produce dirty data.
*/
for (RocksDBSessions sessions : this.dbs.values()) {
if (sessions instanceof RocksDBStdSessions) {
Copy link
Contributor

Choose a reason for hiding this comment

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

move forceCloseRocksDB to RocksDBSessions

for (RocksDBSessions sessions : this.dbs.values()) {
sessions.reload();
}
LOG.info("The store {} load snapshot succeed", this.store);
Copy link
Contributor

Choose a reason for hiding this comment

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

successfully

executor.execute(() -> compressSnapshot(writer, metaBuilder,
done));
} else {
LOG.error("Failed to save snapshot, path={}, files={}, {}.",
writerPath, writer.listFiles(), throwable);
writerPath, writer.listFiles(), t);
Copy link
Contributor

Choose a reason for hiding this comment

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

"Failed to save snapshot, path={}, files={}", the exception stack will be printed

return CompletableFuture.completedFuture(LocalFileMeta.newBuilder());
}

public void doSnapshotLoad(BackendStore store, String snapshotPath) {
store.readSnapshot(snapshotPath);
public void doSnapshotLoad(String snapshotPath) {
Copy link
Contributor

Choose a reason for hiding this comment

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

set private

@@ -186,6 +186,10 @@ public StoreType storeType(String store) {
}
}

public RaftBackendStore[] stores() {
Copy link
Contributor

Choose a reason for hiding this comment

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

set protected

@@ -64,7 +64,8 @@ public void save(SnapshotWriter writer, Closure done,
executor.execute(() -> compressSnapshot(writer, metaBuilder,
done));
} else {
LOG.error("Failed to save snapshot, path={}, files={}, {}.",
LOG.error("Failed to save snapshot, path={}, files={}, " +
"the exception stack will be printed.",
Copy link
Contributor

Choose a reason for hiding this comment

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

"the exception stack will be printed" is just comment

javeme
javeme previously approved these changes Nov 10, 2020
@@ -129,8 +129,9 @@ public void open(String name) {
@Override
public void waitStoreStarted() {
this.context.initRaftNode();
LOG.info("The raft node is initialized");
Copy link
Contributor

Choose a reason for hiding this comment

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

remove "is"

Copy link
Contributor

Choose a reason for hiding this comment

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

prefer keep it

this.context.waitRaftNodeStarted();
LOG.info("The raft store was started");
LOG.info("The raft store is started");
Copy link
Contributor

Choose a reason for hiding this comment

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

remove "is"

houzhizhen
houzhizhen previously approved these changes Nov 17, 2020
Change-Id: I761c01da871aac6fe6e906f6150c66c7f5b8f67b
Change-Id: Ia423824791a1c6bd611e981facaf7a26c94fc5ca
Change-Id: Ib2c72eab8b91abbba55cca3b309ed91e35654083
Change-Id: I27be0ad4bf78694e98855d77450a97f447832114
Change-Id: I157a1c3b6f893083f507b99a22e717e2b89eb2ce
Change-Id: I205329c07fc8bac0f9731e77ddc842f1366d21ec
Change-Id: I37779caec0fdd50276f8597fa4540df39ad40942
javeme
javeme previously approved these changes Nov 17, 2020
houzhizhen
houzhizhen previously approved these changes Nov 17, 2020
zhoney
zhoney previously approved these changes Nov 17, 2020
public void reload() throws RocksDBException {
if (this.rocksdb.isOwningHandle()) {
this.rocksdb.close();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

move line 259 to line 256

@@ -265,7 +260,7 @@ protected RocksDBSessions open(HugeConfig config, String dataPath,
sessions = this.openSessionPool(config, dataPath,
walPath, tableNames);
} catch (RocksDBException e) {
RocksDBSessions origin = dbs.get(dataPath);
RocksDBSessions origin = this.dbs.get(dataPath);
if (origin != null) {
if (e.getMessage().contains("No locks available")) {
// Open twice, but we should support keyspace
Copy link
Contributor

Choose a reason for hiding this comment

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

update comments:
Open twice, copy a RocksDBSessions reference, since from v0.11.2 release we don't support multi graphs share rocksdb instance (before v0.11.2 graphs with different CF-prefix share one rocksdb instance and data path), so each graph has its independent data paths, but multi CFs may share same optimized disk(or optimized disk path).

Change-Id: I5faf30bc530a6ad21fee81fef22614d4e46123a9
@Linary Linary dismissed stale reviews from zhoney, houzhizhen, and javeme via a6aac8f November 18, 2020 06:49
@codecov
Copy link

codecov bot commented Nov 18, 2020

Codecov Report

Merging #1247 (a6aac8f) into master (b4c767b) will decrease coverage by 6.80%.
The diff coverage is 42.88%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1247      +/-   ##
============================================
- Coverage     69.19%   62.38%   -6.81%     
- Complexity     5401     5766     +365     
============================================
  Files           328      374      +46     
  Lines         26334    31329    +4995     
  Branches       3750     4398     +648     
============================================
+ Hits          18222    19545    +1323     
- Misses         6333     9800    +3467     
- Partials       1779     1984     +205     
Impacted Files Coverage Δ Complexity Δ
...api/src/main/java/com/baidu/hugegraph/api/API.java 69.04% <ø> (+3.49%) 0.00 <0.00> (ø)
...n/java/com/baidu/hugegraph/api/auth/AccessAPI.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...n/java/com/baidu/hugegraph/api/auth/BelongAPI.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...in/java/com/baidu/hugegraph/api/auth/GroupAPI.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...n/java/com/baidu/hugegraph/api/auth/TargetAPI.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ain/java/com/baidu/hugegraph/api/auth/UserAPI.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...com/baidu/hugegraph/api/gremlin/GremlinClient.java 90.90% <ø> (ø) 0.00 <0.00> (ø)
.../java/com/baidu/hugegraph/api/job/ComputerAPI.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...ain/java/com/baidu/hugegraph/api/raft/RaftAPI.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...a/com/baidu/hugegraph/api/schema/EdgeLabelAPI.java 59.72% <ø> (+2.77%) 0.00 <0.00> (ø)
... and 316 more

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 5a68f50...a6aac8f. Read the comment docs.

@Linary Linary merged commit d3fc084 into master Nov 18, 2020
@Linary Linary deleted the fix-snapshot branch November 18, 2020 08:09
tmljob pushed a commit to tmljob/hugegraph that referenced this pull request Dec 10, 2020
* adapt with dbs as member field
* Adapt MultiGraphTests

Change-Id: I5faf30bc530a6ad21fee81fef22614d4e46123a9
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.

None yet

4 participants