mgr/dashboard: Add monitor list #19632

Rubab-Syed · 2017-12-21T12:25:29Z

This PR adds a new page for monitor daemon. It shows following metrics:

Cluster ID
Monmap modification time
Features
Monmap epoch
Monitor's name
Rank
Public IP Address
Inline graph of open sessions

jcsp · 2017-12-21T12:45:23Z

Thanks -- to get this to a mergeable point, please do the following:

Rebase your branch on the latest master (this will involve resolving the merge conflict on module.py). You first fetch the latest code (use "git fetch origin" if your repo was originally cloned from the main ceph repo, or otherwise add a remote ("git remote add upstream git@github.com:ceph/ceph.git") and then fetch from that ("git fetch upstream"). Then, do a "git rebase upstream/master". This will complain about a conflict in module.py, which you need to edit, then do a "git add" on that file and finally a "git rebase --continue".
Squash the commits into larger logical changes (in this PR that probably just means one commit). I usually do this with "git rebase -i upstream/master", and then mark the smaller commits as "f" so that they are squashed into the commit before them.
Edit the commit messages to match the standard commit message style (see other recent commits in git log, the main thing is to start it with the component and a short description ``mgr/dashboard: add monitor listing" and include a "Signed-off-by: Your Name " at the end) . I usually do the edit with another "rebase -i" and mark the commit as "r" so that I can edit the commit message.
Finally, re-push your branch (same push command as originally, but add a --force).

It's quite a lot of git to just update a PR, but most of the time we're just using these same few commands, so it quickly becomes familiar. If you already knew all these git commands then I apologise for being so verbose :-)

There's a longer description of the submission guidelines here: https://github.com/ceph/ceph/blob/master/SubmittingPatches.rst (ignore the "Patch submission via ceph-devel@vger.kernel.org" part as we're using pull requests)

jcsp · 2017-12-21T12:52:13Z

I wonder if @jecluis or @tchaikov would have an opinion about the info displayed here? (see bottom of monitors.html for list of columns)

jecluis

First of all, one thing that would make our life easier when adding new pages would be adding a screenshot with the page itself. Just a thought for the future (@jcsp if you think this is something useful and reasonable to ask for all of dashboard's PRs, maybe that could be added that to the contributing doc?).

Additionally, there are a few fields that could make sense to expose as well: current monmap epoch, and monmap features.

jecluis · 2017-12-21T16:56:59Z

src/pybind/mgr/dashboard/monitors.html

+        <table class="table">
+            <tr style="font-size:15px">
+	         <td><p class="text-left"><span style="color:yellow; font-weight:bold">Cluster ID: </span> {mon_status.monmap.fsid}</p></td>
+	         <td><p class="text-left"><span style="color:yellow; font-weight:bold">Monmap Created: </span> {mon_status.monmap.created}</p></td>


nit: I really dislike Monmap. IMO, it should always either be capitalized as MonMap or, even better, all lower caps monmap. Maybe @jcsp has a better sense of what would be user friendlier though.

Also, I would personally not care much about the creation time of the monmap. I'd much rather only see when it was last modified, as it should also reflect the creation time for the initial monmap.

jecluis · 2017-12-21T17:01:25Z

src/pybind/mgr/dashboard/monitors.html

+        </table>	
+    </div>
+    <div class="box">
+        <h3 style="color:#00bb00; font-weight:bold">INSIDE QUORUM</h3>


I would prefer "IN QUORUM" rather than "INSIDE QUORUM". Inside doesn't make much sense in the context of a quorum - you're either in it or you are not.

jecluis · 2017-12-21T17:10:30Z

src/pybind/mgr/dashboard/monitors.html

+	             <th>Name</th>
+	  	     <th>Rank</th>
+		     <th>IP Addr</th>
+		     <th>Public Addr</th>


addr and public_addr should be the same, and I'm pretty sure addr is only there for legacy reasons. In MonMap::dump(), both fields have the same value, and we should always honor public_addr in this case. What is missing though (and would be really useful) is showing the monitor's public_bind_addr, but I don't think we expose that on the monmap (it's a runtime thing). It should be exposed via the daemon's config get, but I don't think that's reasonable to expect the dashboard to do, really.

In a nutshell, drop IP Addr.

jecluis · 2017-12-21T17:12:42Z

src/pybind/mgr/dashboard/monitors.html

+		     <th>Public Addr</th>
+		     <th>Open Sessions</th>
+		     <th>Trimmed Sessions</th>
+		     <th>Elections Participated In</th>


I don't think these fields are particularly helpful. The only thing that would be interesting to know is whether the monitor has ever been in-quorum (in case it is not currently in quorum).

Sorry, should have been cleared: I meant the Trimmed Sessions and Elections Participated In.

jecluis · 2017-12-21T17:13:03Z

src/pybind/mgr/dashboard/monitors.html

+    </div>
+
+    <div class="box">
+        <h3 style="color:#00bb00; font-weight:bold">OUTSIDE QUORUM</h3>


"OUTSIDE QUORUM" -> "NOT IN QUORUM"

jecluis · 2017-12-21T17:13:22Z

src/pybind/mgr/dashboard/monitors.html

+		     <th>Public Addr</th>
+		     <th>Open Sessions</th>
+		     <th>Trimmed Sessions</th>
+		     <th>Elections Participated</th>


Same comments from above apply.

Rubab-Syed · 2018-01-04T02:06:00Z

jecluis

The PR looks good. Minor nit in the not in quorum list.

The only thing missing is figuring out whether we have a decent way to show the actual feature names in the con and required features fields, and then to fix the commit title for e0d91cf .

jecluis · 2018-01-04T03:07:52Z

src/pybind/mgr/dashboard/monitors.html

+		    <tr><td><p><span style="color:yellow; font-weight:bold">monmap modified: </span></td><td> {mon_status.monmap.modified}</p></td></tr>
+		    <tr><td><p><span style="color:yellow; font-weight:bold">monmap epoch: </span></td><td> {mon_status.monmap.epoch}</p></td></tr>
+		    <tr><td><p><span style="color:yellow; font-weight:bold">quorum con: </span></td><td> {mon_status.features.quorum_con}</p></td></tr>
+		    <tr><td><p><span style="color:yellow; font-weight:bold">required con: </span></td><td> {mon_status.features.required_con}</p></td></tr>


While I think quorum_con and required_con features are nice values to know, I think the integer values have little value when exposed by themselves to the user on the dashboard. It would be nice to actually have the features being used instead.

It may just so be that you do not have a good way to obtain those; if that is the case, please file a ticket in the tracker mentioning just that, assign it to me, link this PR in the ticket, and I'll address that (and then we can amend this feature to include those features at a later date).

jecluis · 2018-01-04T03:08:55Z

src/pybind/mgr/dashboard/monitors.html

+		    <th>Name</th>
+		    <th>Rank</th>
+		    <th>Public Addr</th>
+		    <th>Open Sessions</th>


There is a fair chance that this field is actually irrelevant if the monitor is not in quorum. I'd suggest dropping it.

jecluis · 2018-01-04T03:12:47Z

Also, Thank you for the screenshot! That helped immensely! :) 👍

liewegas · 2018-01-04T03:30:11Z

I wouldn't worry about the features for now.. we can either drop it or replace it with the release code name. We don't have a feature bit mask -> string function except for a function that tells you which release it looks like (e.g., "luminous", "firefly", etc.), and that's probably not very helpful at this level (the ceph version of the daemon is probably more interesting).

Love the "open sessions" mini plot!

liewegas · 2018-01-04T03:33:25Z

Hmm it would be nice to show the out-of-quorum nodes' sync status (when they are doing a sync). It's a bit hard because we don't have a percentage value, though (we have the current key position but we don't know how far that is). Also sync is rare-ish.. but when it does happen you really want to know how close it is to finishing.

Another thing that would be nice is a "how long in quorum" time value. I've been pining for a "pretty time" helper for a while that returns things like "32s", "12m", "3.4h", "7w", etc. Seeing a time in quorum for each mon would make flappy/unreliable mons visible.

(These are all nice-to-haves that can come later of course!)

jecluis · 2018-01-04T05:57:06Z

@liewegas Now that you mention that, I do have a how long has mon.foo been out of the quorum on my to-do list. I guess this is as good time as it will ever be to get that going.

Signed-off-by: Rubab Syed<rubab.syed21@gmail.com>

tchaikov · 2018-01-08T08:26:09Z

src/pybind/mgr/dashboard/monitors.html

+	    </div>    
+	</div>
+    </div>
+    <div class="box">


i'd suggest move this div right after the the "IN QUORUM" div, i.e after https://github.com/ceph/ceph/pull/19632/files#diff-4e15ad4bfa3f75fa7ca9156de9eedd25R77. so it can be aligned with the "IN QUORUM" table. looks better this way. what do you think?

jcsp · 2018-01-11T11:01:27Z

@jecluis @tchaikov do you guys mind if I merge this, and leave any other tweaks to a follow up PR?

jcsp added the mgr label Dec 21, 2017

jcsp changed the title ~~Outreachy mgr dashboard: Add "mon" daemon statistics~~ mgr/dashboard: Add monitor list Dec 21, 2017

jecluis requested changes Dec 21, 2017

View reviewed changes

Rubab-Syed force-pushed the outreachy_mgr_dashboard branch from 6ce48a6 to c7b6104 Compare December 31, 2017 21:05

jecluis requested changes Jan 4, 2018

View reviewed changes

tchaikov self-requested a review January 4, 2018 03:34

Rubab-Syed force-pushed the outreachy_mgr_dashboard branch 2 times, most recently from fa9780e to dda8401 Compare January 7, 2018 23:32

mgr/dashboard: add monitor listing

01edf5e

Signed-off-by: Rubab Syed<rubab.syed21@gmail.com>

Rubab-Syed force-pushed the outreachy_mgr_dashboard branch from dda8401 to 01edf5e Compare January 7, 2018 23:38

tchaikov reviewed Jan 8, 2018

View reviewed changes

tchaikov approved these changes Jan 11, 2018

View reviewed changes

jcsp merged commit 53348dc into ceph:master Jan 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mgr/dashboard: Add monitor list #19632

mgr/dashboard: Add monitor list #19632

Rubab-Syed commented Dec 21, 2017 •

edited

jcsp commented Dec 21, 2017

jcsp commented Dec 21, 2017

jecluis left a comment

jecluis Dec 21, 2017

jecluis Dec 21, 2017

jecluis Dec 21, 2017

jecluis Dec 21, 2017

jecluis Dec 21, 2017

jecluis Dec 21, 2017

jecluis Dec 21, 2017

jecluis Dec 21, 2017

Rubab-Syed commented Jan 4, 2018 •

edited

jecluis left a comment

jecluis Jan 4, 2018

jecluis Jan 4, 2018

jecluis commented Jan 4, 2018

liewegas commented Jan 4, 2018

liewegas commented Jan 4, 2018

jecluis commented Jan 4, 2018

tchaikov Jan 8, 2018

jcsp commented Jan 11, 2018

mgr/dashboard: Add monitor list #19632

mgr/dashboard: Add monitor list #19632

Conversation

Rubab-Syed commented Dec 21, 2017 • edited

jcsp commented Dec 21, 2017

jcsp commented Dec 21, 2017

jecluis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rubab-Syed commented Jan 4, 2018 • edited

jecluis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jecluis commented Jan 4, 2018

liewegas commented Jan 4, 2018

liewegas commented Jan 4, 2018

jecluis commented Jan 4, 2018

Choose a reason for hiding this comment

jcsp commented Jan 11, 2018

Rubab-Syed commented Dec 21, 2017 •

edited

Rubab-Syed commented Jan 4, 2018 •

edited