You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2017/12/01 03:08:49 UTC

[kudu-CR] consensus: Make queue more debuggable

Hello Andrew Wong,

I'd like you to do a code review. Please visit

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

to review the following change.


Change subject: consensus: Make queue more debuggable
......................................................................

consensus: Make queue more debuggable

After the introduction of SafeToEvict() logic in the consensus queue,
the eviction criteria has become more subtle. It would be helpful to get
more information about the state of the leader queue from the web UI to
aid in troubleshooting. This patch dumps the queue information in the
web UI and also includes the time since last communication with each
peer in the per-peer Watermarks table.

Change-Id: I052285a6885c251adbbab62410530fef4769fe72
---
M src/kudu/consensus/consensus_queue.cc
1 file changed, 5 insertions(+), 2 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/8708/1
-- 
To view, visit http://gerrit.cloudera.org:8080/8708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Gerrit-Change-Number: 8708
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>

[kudu-CR] consensus: Make queue more debuggable

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, 

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

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

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

Change subject: consensus: Make queue more debuggable
......................................................................

consensus: Make queue more debuggable

After the introduction of SafeToEvict() logic in the consensus queue,
the eviction criteria has become more subtle. It would be helpful to get
more information about the state of the leader queue from the web UI to
aid in troubleshooting. This patch dumps the queue information in the
web UI and also includes the time since last communication with each
peer in the per-peer Watermarks table.

Change-Id: I052285a6885c251adbbab62410530fef4769fe72
---
M src/kudu/consensus/consensus_queue.cc
1 file changed, 4 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/08/8708/2
-- 
To view, visit http://gerrit.cloudera.org:8080/8708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Gerrit-Change-Number: 8708
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] consensus: Make queue more debuggable

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8708 )

Change subject: consensus: Make queue more debuggable
......................................................................


Patch Set 1:

(1 comment)

> Patch Set 1:
> 
> (1 comment)
> 
> How's the overlap of the queue_state with what's already dumped to html?

the queue state is what gets dumped when you become leader and it's just a laundry list of queue items like majority replicated index and stuff like that.

string PeerMessageQueue::QueueState::ToString() const {
  return Substitute("All replicated index: $0, Majority replicated index: $1, "
      "Committed index: $2, Last appended: $3, Last appended by leader: $4, Current term: $5, "
      "Majority size: $6, State: $7, Mode: $8$9",
      all_replicated_index, majority_replicated_index,
      committed_index, OpIdToString(last_appended), last_idx_appended_to_leader, current_term,
      majority_size_, state, (mode == LEADER ? "LEADER" : "NON_LEADER"),
      active_config ? ", active raft config: " + SecureShortDebugString(*active_config) : "");
}

http://gerrit.cloudera.org:8080/#/c/8708/1/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

http://gerrit.cloudera.org:8080/#/c/8708/1/src/kudu/consensus/consensus_queue.cc@131
PS1, Line 131: );
> nit: previous line? Or is this stylistic
ah, yeah, mistake. will fix



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Gerrit-Change-Number: 8708
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 01 Dec 2017 04:37:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] consensus: Make queue more debuggable

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8708 )

Change subject: consensus: Make queue more debuggable
......................................................................


Patch Set 1:

(1 comment)

How's the overlap of the queue_state with what's already dumped to html?

http://gerrit.cloudera.org:8080/#/c/8708/1/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:

http://gerrit.cloudera.org:8080/#/c/8708/1/src/kudu/consensus/consensus_queue.cc@131
PS1, Line 131: );
nit: previous line? Or is this stylistic



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Gerrit-Change-Number: 8708
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 01 Dec 2017 03:26:23 +0000
Gerrit-HasComments: Yes

[kudu-CR] consensus: Make queue more debuggable

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/8708 )

Change subject: consensus: Make queue more debuggable
......................................................................


Patch Set 2: Code-Review+2

Gotcha


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Gerrit-Change-Number: 8708
Gerrit-PatchSet: 2
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 01 Dec 2017 04:40:28 +0000
Gerrit-HasComments: No

[kudu-CR] consensus: Make queue more debuggable

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8708 )

Change subject: consensus: Make queue more debuggable
......................................................................


Patch Set 1: Code-Review+1

LGTM, maybe just address the stylistic issue, if needed.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Gerrit-Change-Number: 8708
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Comment-Date: Fri, 01 Dec 2017 04:35:38 +0000
Gerrit-HasComments: No

[kudu-CR] consensus: Make queue more debuggable

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8708 )

Change subject: consensus: Make queue more debuggable
......................................................................

consensus: Make queue more debuggable

After the introduction of SafeToEvict() logic in the consensus queue,
the eviction criteria has become more subtle. It would be helpful to get
more information about the state of the leader queue from the web UI to
aid in troubleshooting. This patch dumps the queue information in the
web UI and also includes the time since last communication with each
peer in the per-peer Watermarks table.

Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Reviewed-on: http://gerrit.cloudera.org:8080/8708
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/consensus/consensus_queue.cc
1 file changed, 4 insertions(+), 2 deletions(-)

Approvals:
  Andrew Wong: Looks good to me, approved
  Kudu Jenkins: Verified

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I052285a6885c251adbbab62410530fef4769fe72
Gerrit-Change-Number: 8708
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>