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

Move lz4 decompress to backend executor #1237

Merged
merged 7 commits into from
Nov 5, 2020
Merged

Move lz4 decompress to backend executor #1237

merged 7 commits into from
Nov 5, 2020

Conversation

Linary
Copy link
Contributor

@Linary Linary commented Nov 2, 2020

Change-Id: Idf2e796739fda1dc5ce5ffcfa16bc61b6e1a6281

Change-Id: Idf2e796739fda1dc5ce5ffcfa16bc61b6e1a6281
Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

update the title to "Move lz4 decompress to backend executor"

@@ -230,7 +230,8 @@ private void waitIfBusy() {
if (this.busyCounter.get() > 0) {
synchronized (this) {
if (this.busyCounter.get() > 0) {
this.busyCounter.decrementAndGet();
LOG.info("Decrease busy counter: [{}]",
this.busyCounter.decrementAndGet());
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer to define a var

/**
* Maybe useful in the future
*/
private boolean isRPCTimeout(Status status) {
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer isRpcTimeout

Change-Id: I4b4c470228caa4ba7e44df9618a7aa1f35da06ef
public static final ConfigOption<Integer> RAFT_RPC_BUF_HIGH_WATER_MARK =
new ConfigOption<>(
"raft.rpc_buf_high_water_mark",
"",
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

public static final ConfigOption<Integer> RAFT_RPC_BUF_LOW_WATER_MARK =
new ConfigOption<>(
"raft.rpc_buf_low_water_mark",
"",
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer write some description

@@ -266,6 +267,9 @@ public void notifyCache(HugeType type, Id id) {
eventHub = this.params.graphEventHub();
} else if (type.isSchema()) {
eventHub = this.params.schemaEventHub();
if (id.number() && id.asLong() < 0) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

add some comment

@@ -54,6 +54,10 @@ public static BytesBuffer compress(byte[] bytes, int blockSize,
} catch (IOException e) {
throw new BackendException("Failed to compress", e);
}
/*
* If need perform reading outside the method,
Copy link
Contributor

Choose a reason for hiding this comment

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

need to perform

@Linary Linary changed the title Let lz4 decompress in thread pool Move lz4 decompress to backend executor Nov 3, 2020
Change-Id: I70c2fcfd92ed923d801394cd6f97b31afa8475d4
zhoney
zhoney previously approved these changes Nov 3, 2020
@@ -107,7 +107,9 @@ private void listenChanges() {
if ("invalid".equals(args[0])) {
HugeType type = (HugeType) args[1];
Id id = (Id) args[2];
this.arrayCaches.remove(type, id);
if (id.number() && id.asLong() > 0) {
this.arrayCaches.remove(type, id);
Copy link
Contributor

Choose a reason for hiding this comment

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

move to remove() method

Change-Id: I8008c86b8e7c92d72dac205eca8894826a39edb8
javeme
javeme previously approved these changes Nov 4, 2020
Copy link
Contributor

@javeme javeme left a comment

Choose a reason for hiding this comment

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

image
ci failure

this.set(schema.type(), id, schema);
}
}

public V get(HugeType type, Id id) {
assert id.number() && id.asLong() > 0 : id;
assert id.number();
if (id.asLong() <= 0L) {
Copy link
Contributor

Choose a reason for hiding this comment

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

define var longId=id.asLong()

@@ -348,7 +349,10 @@ public V get(HugeType type, Id id) {
}

public void set(HugeType type, Id id, V value) {
assert id.number() && id.asLong() > 0 : id;
assert id.number();
if (id.asLong() <= 0L) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -373,7 +377,10 @@ public void set(HugeType type, Id id, V value) {
}

public void remove(HugeType type, Id id) {
assert id.number() && id.asLong() > 0 : id;
assert id.number();
if (id.asLong() <= 0L) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Change-Id: Ia15672c9e0564cb9673a0b8ab983272c3943c276
assert id.number() && id.asLong() > 0 : id;
assert id.number();
if (id.asLong() <= 0L) {
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

add assert false : id;

Change-Id: I4d7a90c5e40117e00c1e017917ac5d83824df3ce
Change-Id: Ic7770f0b8cb9a81dbb714ea9ce8ef99d5b710807
@codecov
Copy link

codecov bot commented Nov 4, 2020

Codecov Report

Merging #1237 into master will increase coverage by 0.20%.
The diff coverage is 23.07%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1237      +/-   ##
============================================
+ Coverage     62.21%   62.42%   +0.20%     
- Complexity     5736     5758      +22     
============================================
  Files           371      374       +3     
  Lines         31153    31284     +131     
  Branches       4381     4393      +12     
============================================
+ Hits          19382    19529     +147     
+ Misses         9795     9772      -23     
- Partials       1976     1983       +7     
Impacted Files Coverage Δ Complexity Δ
...hugegraph/backend/store/raft/RaftBackendStore.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...aidu/hugegraph/backend/store/raft/RaftClosure.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...m/baidu/hugegraph/backend/store/raft/RaftNode.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ugegraph/backend/store/raft/RaftSharedContext.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ugegraph/backend/store/raft/StoreStateMachine.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...rc/main/java/com/baidu/hugegraph/util/LZ4Util.java 70.00% <ø> (ø) 3.00 <0.00> (ø)
...gegraph/backend/cache/CachedSchemaTransaction.java 89.80% <31.57%> (-3.53%) 33.00 <5.00> (ø)
...n/java/com/baidu/hugegraph/config/CoreOptions.java 99.44% <100.00%> (+0.01%) 2.00 <0.00> (ø)
...com/baidu/hugegraph/job/computer/ComputerPool.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...baidu/hugegraph/job/computer/AbstractComputer.java 0.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
... and 10 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 76d0cb3...21178e0. Read the comment docs.

@Linary Linary merged commit 636bcbb into master Nov 5, 2020
@Linary Linary deleted the improve-raft-perf branch November 5, 2020 02:48
tmljob pushed a commit to tmljob/hugegraph that referenced this pull request Dec 10, 2020
* use whitebox to set LOW_WATER_MARK and HIGH_WATER_MARK

Change-Id: Ic7770f0b8cb9a81dbb714ea9ce8ef99d5b710807
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