nsqadmin: refactoring

[mreiferson]

nsqadmin is a Go web app, which has some tradeoffs. It's great because it's dead simple to deploy. It's terrible because it's a PITA writing Go based web apps.

I think an ideal middle ground is:

1. nsqadmin becomes a JSON API with convenience endpoints to query the cluster and aggregate data
2. nsqadmin UI is a client-side app
3. all the static assets for (2) are bundled into the binary so it's still easy to deploy (this is already the case, just stating the obvious)

Simultaneously, it could use a facelift, but this refactoring alone makes things much easier to extend and for others to contribute.

cc @visionmedia

[mreiferson]

ping #322

[michaelhood]

really like the client side UI approach. will be exciting to see what people create on it.

[tj]

@mreiferson the "messages" count includes messages in-flight right? would you be interested in changing that to only messages that have been FIN'd? or maybe adding another

[mreiferson]

@visionmedia there should be a separate counter for just FIN message count, the "regular" message count is specifically the # of PUB messages to that nsqd

[mreiferson]

edit: s/should be/is

[mreiferson]

edit again: I'm crazy, there isn't...

[mreiferson]

I suppose it is derivable as message_count - in_flight_count (and optionally subtracting deferred_count and requeued_count if you wanted to exclude retried duplicates and count "unique" FIN)

[mreiferson]

@visionmedia you starting to work on this?

[tj]

nope just debugging some prod stuff ATM

[pkieltyka]

Great idea, what about having the admin endpoints on nsqd itself? .. and that way the client-side admin could be just some static html/js files, in the index.html just specify a jsonp endpoint to an nsqlookupd and done.

[mreiferson]

@pkieltyka that's a little more difficult, in short, because that means all of the daemons are exposed to the internet. Technically nsqd and nsqlookupd already do have all the endpoints they need. nsqadmin really just aggregates and coordinates requests.

I think it still makes sense for nsqadmin to serve that role behind the client-side application (and to serve the initial static asset payload).

[pkieltyka]

That makes sense

[allgeek]

Is there a timeline for getting this reviewed & landed in master? I'm looking into submitting a pull request for #347, and obviously there are a number of nsqadmin changes involved there. Is there something a new contributor could possibly help with here? (I noticed a Travis build failure on these commits, but haven't dug into it yet - don't want to step on any toes or duplicate efforts)

[mreiferson]

Hi @allgeek - great to hear that you're interested in contributing, thanks!

I just need to finish up here, so this is on me. I've been distracted the past few weeks 😁

If you want to dig into this PR via code review, by all means that would be helpful - this way we're on the same page with the direction I'm taking things.

[mreiferson]

I just remembered - if you are interested in helping out with this PR, I could use some help with the client side changes referenced in the OP.

[allgeek]

I'll try to catch you in the IRC channel at some point, although I've got meetings most of the day. Is there a particular JS framework or approach you had wanted to take with the new admin client page? I see the current site just uses jQuery and Bootstrap...

[mreiferson]

seems like we could use a little helper for this dance we're doing here a thousand times?

[jehiah]

haha clearly i was just in brute force make-it-work mode. fixed

[mreiferson]

cool, LGTM - let's land this ✈️

Want to try to squash 091eabc on?

[jehiah]

Close. two things i found in validation

1. Bugs getting per-nsqd rate on the topics page (this should be easy; i think it's from variable renames)
2. some bugs on the Counter page (it's not working correctly for me). Not sure what's happening there yet.

[jehiah]

I spoke too soon. A few more items:

• It seems the default timeframe for graphite charts is defaulting to 'off' I expect that to default to 2h when it's not set via a cookie.
• The 'attributes' section which should display TLS and auth state isn't displaying TLS details, or auth state. (see image below)

[mreiferson]

It seems the default timeframe for graphite charts is defaulting to 'off' I expect that to default to 2h when it's not set via a cookie.

I think it's now in local storage, should be easy to default it to 2h

[mreiferson]

some bugs on the Counter page (it's not working correctly for me). Not sure what's happening there yet.

I didn't translate the JS 1:1 here, it's just broken or?

[jehiah]

For the counter page, i'm getting Zero's back from the /api/counter endpoint, but also the graphite URLS have unexpected &amp instead of just &.

[jehiah]

I think i fixed all the previously mentioned bugs (please take a look before i squash those); glyphicon fonts are not being embedded properly and i think that's the only outstanding thing left.

[mreiferson]

For the counter page, i'm getting Zero's back from the /api/counter endpoint, but also the graphite URLS have unexpected &amp instead of just &.

Did you figure this out?

[jehiah]

For the counter page, i'm getting Zero's back from the /api/counter endpoint, but also the graphite URLS have unexpected &amp instead of just &.

Did you figure this out?

Yes, it was broken handling of message_count also affecting the Message Count columns (i just didn't notice on my dev instance that it was always Zero). Fixed by f9cdd4f

[mreiferson]

cool 🔨

[jehiah]

@mreiferson font issues addressed; take a look at last two commits?

[mreiferson]

@jehiah don't you think the console.log might be useful when people inevitably encounter bugs?

[mreiferson]

also, do we need another HTTP route for the same handler? Is it because bootstrap expects them at that URL?

[jehiah]

yeah, bootstrap requests them as fonts/.... and that felt like the easiest path to resolving since the gulp step currently flattens directories. I'd rather one line of Go to more gulp =)

I have "no console.log in production" burned into my brain since it's not present in all browsers and thus causes JS errors (Thanks IE 9!)

[mreiferson]

very well, squash time?

[mreiferson]

😱

[klucar]

Has anyone started work on refactoring the UI for #443? We get i/o timeout errors when nsq is under heavy load. I'd like to get rid of this timeout, make it configurable or ?

I have experience with doing ReactJS interfaces and this is kind of a perfect use-case for it. If nobody is already working this, I'll give it a go.

[mreiferson]

@klucar we made a bunch of improvements recently in #701 that should significantly improve this situation, if you're interested in deploying an alpha

[klucar]

we're just evaluating if NSQ can work/scale for us, so I'll gladly give an alpha a shot :)

[klucar]

@mreiferson any idea when a new release will be cut?

[mreiferson]

@klucar I've been lazy, we should get one out (I've been running it in production without issue for a few weeks)