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

Fix queryByRaft readIndex failed #1121

Merged
merged 7 commits into from
Jul 31, 2020
Merged

Fix queryByRaft readIndex failed #1121

merged 7 commits into from
Jul 31, 2020

Conversation

Linary
Copy link
Contributor

@Linary Linary commented Jul 29, 2020

ReadIndex request rejected because leader has not committed any log entry at its term

Change-Id: I6f5c6fd3770da732caf0c6acd41f0441aa4acb55

ReadIndex request rejected because leader has not committed any log entry at its term

Change-Id: I6f5c6fd3770da732caf0c6acd41f0441aa4acb55
@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #1121 into master will increase coverage by 7.66%.
The diff coverage is 16.21%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1121      +/-   ##
============================================
+ Coverage     58.20%   65.86%   +7.66%     
- Complexity     4801     5652     +851     
============================================
  Files           354      354              
  Lines         28748    28836      +88     
  Branches       4056     4062       +6     
============================================
+ Hits          16732    18994    +2262     
+ Misses        10190     7964    -2226     
- Partials       1826     1878      +52     
Impacted Files Coverage Δ Complexity Δ
...a/com/baidu/hugegraph/auth/HugeGraphAuthProxy.java 4.18% <0.00%> (-0.04%) 0.00 <0.00> (ø)
...e/src/main/java/com/baidu/hugegraph/HugeGraph.java 65.30% <ø> (ø) 9.00 <0.00> (ø)
...hugegraph/backend/store/raft/RaftBackendStore.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...h/backend/store/raft/RaftBackendStoreProvider.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> (ø)
...in/java/com/baidu/hugegraph/StandardHugeGraph.java 80.54% <88.88%> (+1.41%) 110.00 <2.00> (+8.00)
...in/java/com/baidu/hugegraph/core/GraphManager.java 57.14% <100.00%> (+2.66%) 0.00 <0.00> (ø)
...ph/backend/store/AbstractBackendStoreProvider.java 93.75% <100.00%> (+0.07%) 23.00 <1.00> (+1.00)
... and 134 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 2df4f68...338ca00. Read the comment docs.

this.node().node().readIndex(BytesUtil.EMPTY_BYTES,
readIndexClosure);
try {
return closure.waitFinished();
Copy link
Contributor

Choose a reason for hiding this comment

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

rename StoreClosure closure to future

Copy link
Contributor Author

Choose a reason for hiding this comment

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

other place used closure

query);
closure.failure(status, new RaftException(
"Failed to execute query '%s' because of leader " +
"has not committed any log entry at its term",
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 "status.getRaftError() == RaftError.EAGAIN" block to line 247, just throw exception in else block

}
throw new BackendException("Failed to query in the case of %s " +
"retries times");
Copy link
Contributor

Choose a reason for hiding this comment

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

collect last error message like "retries times with error: %s"

Change-Id: Ic39d81623efa89b3f35b794f7d183201348ab871
Change-Id: Ied4194a6e20c3365dede4df922d4eade9b4f7477
try {
return closure.waitFinished();
} catch (Throwable t) {
throw new BackendException("Failed to query", t);
throw new BackendException("Failed to execute query", t);
Copy link
Contributor

Choose a reason for hiding this comment

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

move LOG.warn at line 231 here

@Override
public void onHeartbeated(PeerId peer) {
LOG.info("The node {} replicator has heartbeated", peer);
started = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

RaftNode.this.started

@@ -266,6 +300,7 @@ private boolean isWriteBufferOverflow(Status status) {
@Override
public void onDestroyed(PeerId peer) {
LOG.warn("The node {} prepare to offline", peer);
started = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -165,6 +175,24 @@ protected void waitLeaderElected() {
this.group(), WAIT_LEADER_TIMEOUT);
}

protected boolean waitHeartbeated() {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename waitStarted

}
try {
Thread.sleep(heartbeatInterval);
} catch (InterruptedException ex) {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename ex to e

Copy link
Contributor

Choose a reason for hiding this comment

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

address

try {
Thread.sleep(heartbeatInterval);
} catch (InterruptedException ex) {
throw new BackendException("Waiting heartbeated is interrupted");
Copy link
Contributor

Choose a reason for hiding this comment

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

add cause param

StoreCommand command = new StoreCommand(StoreAction.QUERY);
StoreClosure closure = new StoreClosure(command);
RaftNode raftNode = this.node();
raftNode.node().readIndex(BytesUtil.EMPTY_BYTES, new ReadIndexClosure() {
ReadIndexClosure readIndexClosure = new ReadIndexClosure() {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename StoreClosure closure to future

RaftNode node = this.node();
node.waitLeaderElected();
if (node.node().isLeader()) {
if (!node.waitHeartbeated()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add a common waitFor(future, timeout) method

node.waitLeaderElected();
if (node.node().isLeader()) {
if (!node.waitHeartbeated()) {
throw new BackendException("Wait raft node heartbeat failed");
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 wait raft node heartbeat

@Linary Linary force-pushed the fix-read-index branch 2 times, most recently from e55434b to b42e21e Compare July 31, 2020 03:08
Change-Id: I689149d598ad902cc1ff5175d76557fd7b1491a0

@Override
public void onStopFollowing(LeaderChangeContext ctx) {
LOG.info("The node {} become to follower", this.node.nodeId());
Copy link
Contributor

Choose a reason for hiding this comment

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

abdicated from follower

@@ -337,7 +335,7 @@ private HugeServerInfo serverInfo(Id server) {
}

private HugeServerInfo removeSelfServerInfo() {
if (this.initialized()) {
if (this.graph.initialized()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

should use this.call() if graph.initlalized() will open tx (ensure graph tx closed when graph close)

if (!node.waitHeartbeated()) {
throw new BackendException("Wait raft node heartbeat failed");
}
node.waitHeartbeated(-1);
Copy link
Contributor

Choose a reason for hiding this comment

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

also define NO_TIMEOUT

@@ -105,6 +105,13 @@ public void open(String name) {
this.provider.open(name);
}

@Override
public void waitStoreStarted() {
Copy link
Contributor

Choose a reason for hiding this comment

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

check not null

Change-Id: I1218ec628755434be909f520fe9655ff2b01af85
Change-Id: I2a2e0cc6f0a2e65e23a1b9b2192dd50d8b28224c
return;
} else {
long beginTime = System.currentTimeMillis();
int internalTimeout = 3000;
Copy link
Contributor

Choose a reason for hiding this comment

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

define const interval

}
};
long beginTime = System.currentTimeMillis();
int internalTimeout = 3000;
Copy link
Contributor

Choose a reason for hiding this comment

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

define

}
try {
Thread.sleep(heartbeatInterval);
} catch (InterruptedException ex) {
Copy link
Contributor

Choose a reason for hiding this comment

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

address

Change-Id: I1f191a261979a9ffe232827d0c639642547ba5b7
@Linary Linary merged commit 52d2eac into master Jul 31, 2020
@Linary Linary deleted the fix-read-index branch July 31, 2020 11:13
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