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

open rocksdb graph store using multi-threads when multi disks #721

Merged
merged 7 commits into from
Nov 4, 2019

Conversation

zhoney
Copy link
Contributor

@zhoney zhoney commented Oct 28, 2019

Change-Id: I83525ff97942c49c80f3767ed19d891617ae54f9

@codecov
Copy link

codecov bot commented Oct 28, 2019

Codecov Report

Merging #721 into master will decrease coverage by 1.18%.
The diff coverage is 86.36%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #721      +/-   ##
============================================
- Coverage     71.41%   70.22%   -1.19%     
+ Complexity     4328     4298      -30     
============================================
  Files           280      281       +1     
  Lines         20593    20737     +144     
  Branches       2893     2922      +29     
============================================
- Hits          14706    14563     -143     
- Misses         4444     4729     +285     
- Partials       1443     1445       +2
Impacted Files Coverage Δ Complexity Δ
...raph/backend/store/rocksdb/RocksDBStdSessions.java 69.68% <100%> (+0.03%) 28 <0> (-1) ⬇️
...ugegraph/backend/store/rocksdb/RocksDBOptions.java 99.09% <100%> (+0.07%) 2 <0> (ø) ⬇️
.../hugegraph/backend/store/rocksdb/RocksDBStore.java 70.28% <79.31%> (+6.91%) 56 <4> (+11) ⬆️
...raph/backend/store/postgresql/PostgresqlStore.java 0% <0%> (-100%) 0% <0%> (-2%)
...raph/backend/store/postgresql/PostgresqlTable.java 0% <0%> (-100%) 0% <0%> (-20%)
...kend/store/postgresql/PostgresqlStoreProvider.java 0% <0%> (-92.11%) 0% <0%> (-5%)
...aph/backend/store/postgresql/PostgresqlTables.java 0% <0%> (-87.81%) 0% <0%> (-1%)
...backend/store/postgresql/PostgresqlSerializer.java 0% <0%> (-84.62%) 0% <0%> (-4%)
...h/backend/store/postgresql/PostgresqlSessions.java 0% <0%> (-70.28%) 0% <0%> (-8%)
.../hugegraph/backend/store/AbstractBackendStore.java 72.22% <0%> (-13.5%) 5% <0%> (+1%)
... and 30 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 2785a09...b616401. Read the comment docs.

@zhoney zhoney force-pushed the rocksdb-speedsup branch 2 times, most recently from 9e3f63f to 276fd3d Compare October 29, 2019 08:33
Change-Id: I83525ff97942c49c80f3767ed19d891617ae54f9
@@ -273,6 +300,22 @@ public void close() {

this.checkOpened();
this.sessions.close();

boolean terminated;
Copy link
Contributor

Choose a reason for hiding this comment

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

boolean terminated = false better

Change-Id: Id1f61eb808b5782a137f53c852cbb5aa48c6f870
Change-Id: Ib25b9138aed71e3b4e60e1ab9927c8813cf58f1d
@@ -69,6 +74,11 @@
private RocksDBSessions sessions;
private final Map<HugeType, String> tableDiskMapping;

private static final String DB_OPEN = "db-open-";
Copy link
Contributor

Choose a reason for hiding this comment

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

"db-open-%s"

}));
}
}
for (Future<?> f : futures) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Future<?> future

@@ -273,6 +300,22 @@ public void close() {

this.checkOpened();
this.sessions.close();

Copy link
Contributor

Choose a reason for hiding this comment

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

ensure sessions for all threads are closed

try {
f.get();
} catch (Throwable e) {
throw new BackendException("Failed to open RocksDB store", e);
Copy link
Contributor

Choose a reason for hiding this comment

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

seems can destroy thread pool after open

Change-Id: Ibf6aac1a35e0eda76f07d66875e25f1d169dd638
Change-Id: Ic893d41710b51634aa9a9f962d49ba33291e4056
} catch (Throwable e) {
throw new BackendException("Failed to open RocksDB store", e);
}
}
this.shutdownOpenPool();
Copy link
Contributor

Choose a reason for hiding this comment

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

should create pool before submit for every open()

Change-Id: Iad1330c553839aec3a3238fc9f42fef13faf6463
public static final ConfigOption<Boolean> SKIP_STATS_UPDATE_ON_DB_OPEN =
new ConfigOption<>(
"rocksdb.skip_stats_update_on_db_open",
"This flag allows us to not update statistics.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Whether to skip statistics update when opening the database,
setting this flag true allows us to not update statistics.

@@ -151,20 +159,59 @@ public synchronized void open(HugeConfig config) {
return;
}

List<Future<?>> futures = new ArrayList<>();
ExecutorService openPool = ExecutorUtil.newFixedThreadPool(8, DB_OPEN);
Copy link
Contributor

Choose a reason for hiding this comment

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

can also use MAX_FILE_OPENING_THREADS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this thread pool is used to open DBs, not RocksDB sst files.

Change-Id: I80234139e2fa2a08d7f94bf0bde2056efb4b6ee9
@Linary Linary merged commit 8ee971f into master Nov 4, 2019
@Linary Linary deleted the rocksdb-speedsup branch November 4, 2019 05:22
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

3 participants