[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Mike Percy (Code Review)]

Mike Percy has submitted this change and it was merged.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


KUDU-1469. Fix handling of fully-deduped requests after a leader change

This fixes KUDU-1469, a bug in which the leader and follower could get
into a tight loop of RPCs making no progress replicating operations.

The newly included test fails without the bug fix, and passes with it.
See its comments for details on the bug itself.

Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Reviewed-on: http://gerrit.cloudera.org:8080/3228
Tested-by: Kudu Jenkins
Reviewed-by: David Ribeiro Alves 
Reviewed-by: Mike Percy 
---
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/raft_consensus-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/raft_consensus-itest.cc
8 files changed, 158 insertions(+), 24 deletions(-)

Approvals:
  David Ribeiro Alves: Looks good to me, approved
  Mike Percy: Looks good to me, approved
  Kudu Jenkins: Verified



-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3228/4/src/kudu/consensus/raft_consensus_state.cc
File src/kudu/consensus/raft_consensus_state.cc:

Line 653:   last_received_op_id_current_leader_ = last_received_op_id_;
> gotcha. ok
Err. Is that something that would not be hard to fix? It seems like missing 
coverage.


-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3228/4/src/kudu/consensus/raft_consensus_state.cc
File src/kudu/consensus/raft_consensus_state.cc:

Line 653:   last_received_op_id_current_leader_ = last_received_op_id_;
> yea, I think because restarting the server in the test reset last_received_
gotcha. ok


-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3228/4/src/kudu/consensus/raft_consensus_state.cc
File src/kudu/consensus/raft_consensus_state.cc:

Line 653:   last_received_op_id_current_leader_ = last_received_op_id_;
> Do you have any idea why it was passing tests before?
yea, I think because restarting the server in the test reset 
last_received_op_id_ back down to 0 or somesuch


-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3228/4/src/kudu/consensus/raft_consensus_state.cc
File src/kudu/consensus/raft_consensus_state.cc:

Line 653:   last_received_op_id_current_leader_ = last_received_op_id_;
> hm yea I think you're right. will dig
Do you have any idea why it was passing tests before?


-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Mike Percy (Code Review)]

Mike Percy has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3228/4/src/kudu/consensus/raft_consensus_state.cc
File src/kudu/consensus/raft_consensus_state.cc:

Line 653:   last_received_op_id_current_leader_ = last_received_op_id_;
Shouldn't this be the following?

last_received_op_id_current_leader_ = op_id;


-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[David Ribeiro Alves (Code Review)]

David Ribeiro Alves has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4: -Verified

Build Started http://104.196.14.100/job/kudu-gerrit/1757/

-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/1756/

-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] KUDU-1469. Fix handling of fully-deduped requests after a leader change

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

I'd like you to reexamine a change.  Please visit

http://gerrit.cloudera.org:8080/3228

to look at the new patch set (#4).

Change subject: KUDU-1469. Fix handling of fully-deduped requests after a 
leader change
..

KUDU-1469. Fix handling of fully-deduped requests after a leader change

This fixes KUDU-1469, a bug in which the leader and follower could get
into a tight loop of RPCs making no progress replicating operations.

The newly included test fails without the bug fix, and passes with it.
See its comments for details on the bug itself.

Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
---
M src/kudu/consensus/consensus.proto
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/log_cache.cc
M src/kudu/consensus/raft_consensus-test.cc
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
M src/kudu/integration-tests/raft_consensus-itest.cc
8 files changed, 157 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/28/3228/4
-- 
To view, visit http://gerrit.cloudera.org:8080/3228
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iced21ae1b69c1079efc9aa9cf23e2fa592b8bebd
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon