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

Improve top table to better cope with high RPS traffic #1634

Merged
merged 4 commits into from
Sep 12, 2018
Merged

Conversation

rmars
Copy link

@rmars rmars commented Sep 12, 2018

There are two variables we use to control the volume of Top output, maxRowsToDisplay, which controls how many rows are in the table, and maxRowsToStore, which controls the size of the event index we keep in memory for aggregating results.

Previously, we were only keeping in index maxRowsToDisplay rows, which for the Resource Detail page was 10 (which is really small for high traffic rest-y resource traffic - it causes rows to be deleted from the index too soon, and then causes the data in the table to change a lot). Change this to store maxRowsToStore rows, and also bump this to 50. This allows us to store results for longer, and also ensures more consistent data over time.

Another fix for the appearance of the Top columns is to add fixed widths to the metrics. This will prevent the table from wobbling from side to side. Some widths were added in #1620 and #1628 though, so it'd be helpful to look at widths again once these three branches have merged.

Fixes #1601

@rmars rmars added the area/web label Sep 12, 2018
@rmars rmars self-assigned this Sep 12, 2018
@rmars rmars changed the base branch from rmars/ui-small-fixes to master September 12, 2018 20:32
Copy link
Member

@siggy siggy left a comment

Choose a reason for hiding this comment

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

looks good!

screen shot 2018-09-12 at 1 34 06 pm

Copy link
Contributor

@klingerf klingerf left a comment

Choose a reason for hiding this comment

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

⭐️ Nice fix!

Copy link
Contributor

@dadjeibaah dadjeibaah left a comment

Choose a reason for hiding this comment

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

LGTM 📦 Left a comment but not a big deal.

@dadjeibaah
Copy link
Contributor

Small UI nit, but I don't think it's a big deal. If the Top table is displayed on a really wide monitor the success rate has some padding to the right that makes it look like this:
screen shot 2018-09-12 at 2 47 40 pm

@rmars
Copy link
Author

rmars commented Sep 12, 2018

Hmmm I think you have an old branch? I did rebase on top of master (since I merged two branches that had conflicted with this), so if you checked it out before I'd done that you'd see an old version, without the success rate chart.
Mine looks like this, which admittedly is also weird at wide widths, but the spacing is at least after path...
screen shot 2018-09-12 at 2 53 00 pm

@rmars rmars merged commit 7d2f2af into master Sep 12, 2018
@rmars rmars deleted the rmars/top-frenzy branch September 12, 2018 21:56
zachalbert added a commit to zachalbert/linkerd2 that referenced this pull request Sep 13, 2018
* master:
  Update CHANGES.md for v18.9.1 release (linkerd#1631)
  Cleanly shutdown tap stream to data plane proxies (linkerd#1624)
  Change breadcrumb header to default font in styles.css (linkerd#1633)
  Improve top table to better cope with high RPS traffic (linkerd#1634)
  Add small success rate chart to table, misc web tweaks (linkerd#1628)
  Consolidate the source and destination columns in the Tap and Top tables (linkerd#1620)
  remove extraneous calc function in sidebar.css (linkerd#1632)
  Display more helpful websocket errors (linkerd#1626)
  Add breadcrumb navigation at the top of linkerd dashboard (linkerd#1613)
  Introduce inject check for known sidecars (linkerd#1619)
  Bump Prometheus to v2.4.0, Grafana to 5.2.4 (linkerd#1625)
  Improve performance of tap table by throttling updates (linkerd#1623)
  Add with-source flag to top (linkerd#1614)

Conflicts:
	web/app/css/styles.css
	web/app/js/components/ResourceDetail.jsx
	web/app/js/index.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Top table for large rps
4 participants