ui: rationalize GC Bytes Age on the range status page #38283

andreimatei · 2019-06-18T23:56:46Z

Before this patch, we were interpreting this gc score as a duration in
nanos which made no sense. This patch displays it as a raw number and
separately as an average age per dead byte.

Release note: None

cockroach-teamcity · 2019-06-18T23:56:53Z

This change is

andreimatei · 2019-06-19T00:02:11Z

@tbg I'm pretty confused about whether displaying this score in the UI makes sense because I can't really come up with a coherent story about how this age is "aged" in the copy of the stats stored in the database (as opposed to the on-demand computation performed by the GC queue).
I'm failing to answer the question about who, if anybody, updates it during normal operation. We update it when there's commands applying on the range, I think, but what if there aren't?

I've run some experiments and I do see it updated sometimes, but not reliably.
And more surprisingly, I've seen it once go from 0 to non-zero and then to 0 again (although GC didn't actually collect anything).
Do you know what the story is?

@andreimatei

Addresses part of this [comment] by @andreimatei: > I'm pretty confused about whether displaying this score in the UI makes sense because I can't really come up with a coherent story about how this age is "aged" in the copy of the stats stored in the database (as opposed to the on-demand computation performed by the GC queue). I'm failing to answer the question about who, if anybody, updates it during normal operation. We update it when there's commands applying on the range, I think, but what if there aren't? The stats are aged by each write to the range. If there hasn't been one, no aging happens and prior to this PR the Range endpoint would just return whatever was present. Now it makes sure the stats are aged appropriately. [comment]: cockroachdb#38283 (comment) Release note: None

tbg · 2019-06-19T09:04:55Z

^- I just sent PR #38287 to make sure you see a "recent" age in the range ui page.

I'm not aware of any reason why it would reset to zero. It shouldn't ever decrease unless there's been GC (or you're somehow seeing a stats object that's aged to less than a previous one).

tbg

Post screenshot plz

Reviewed 1 of 1 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei and @tbg)

pkg/ui/src/views/reports/containers/range/rangeTable.tsx, line 82 at r2 (raw file):

  { variable: "mvccLastUpdate", display: "MVCC Last Update", compareToLeader: true },
  { variable: "mvccIntentAge", display: "MVCC Intent Age", compareToLeader: true },
  { variable: "GCBytesAge", display: "GC Bytes Age (score)", compareToLeader: false},

Why false? Ditto below

If the info comes from the range, they should match (if the log position also matches, but that's the assumption for all the trues above). Or at least that would be my expectation.

andreimatei

See the two new rows towards the bottom

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @tbg)

pkg/ui/src/views/reports/containers/range/rangeTable.tsx, line 82 at r2 (raw file):

Previously, tbg (Tobias Grieger) wrote…

Why false? Ditto below

If the info comes from the range, they should match (if the log position also matches, but that's the assumption for all the trues above). Or at least that would be my expectation.

done

tbg

Reviewed 2 of 2 files at r2, 1 of 1 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained

pkg/ui/src/views/reports/containers/range/rangeTable.tsx, line 509 at r2 (raw file):

        mvccSystemBytesCount: this.contentMVCC(FixLong(mvcc.sys_bytes), FixLong(mvcc.sys_count)),
        rangeMaxBytes: this.contentBytes(FixLong(info.state.range_max_bytes)),
        mvccIntentAge: this.contentDuration(FixLong(mvcc.intent_age)),

btw this has similar semantics, not sure if you wanna pull on this thread too but if so now's the time to do it

@andreimatei

38287: server: age the MVCCStats returned from Range endpoint r=andreimatei a=tbg Addresses part of this [comment] by @andreimatei: > I'm pretty confused about whether displaying this score in the UI makes sense because I can't really come up with a coherent story about how this age is "aged" in the copy of the stats stored in the database (as opposed to the on-demand computation performed by the GC queue). I'm failing to answer the question about who, if anybody, updates it during normal operation. We update it when there's commands applying on the range, I think, but what if there aren't? The stats are aged by each write to the range. If there hasn't been one, no aging happens and prior to this PR the Range endpoint would just return whatever was present. Now it makes sure the stats are aged appropriately. [comment]: #38283 (comment) Release note: None 38294: sql: disambiguate ARRAY[] type r=RaduBerinde a=RaduBerinde The optimizer uses `AsStringWithFlags` with `FmtCheckEquivalence` to deduplicate expressions. If we have two empty array expressions that are resolved to different types, the string is just `ARRAY[]` and we incorrectly assume the expressions are the same. The fix is to add a type annotation in this case. Fixes #38293. Release note (bug fix): Fixed an incorrect type mismatch error when empty array values are used as default values (and potentially in other contexts). 38298: roachtest: add some new tests to hibernate blacklist r=jordanlewis a=jordanlewis These tests got added in a recent version of hibernate. Release note: None Co-authored-by: Tobias Schottdorf <tobias.schottdorf@gmail.com> Co-authored-by: Radu Berinde <radu@cockroachlabs.com> Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>

andreimatei

threw in the intents age too

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @tbg)

tbg

Reviewed 1 of 1 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andreimatei)

pkg/ui/src/views/reports/containers/range/rangeTable.tsx, line 84 at r4 (raw file):

  { variable: "GCAvgAge", display: "Dead Value average age", compareToLeader: true},
  { variable: "NumIntents", display: "Intents", compareToLeader: true},
  { variable: "IntentAvgAge", display: "Intent Average Age", compareToLeader: true},

Nit: for bytes age you first show the number, then the average. Settle for one order and stick to it.

andreimatei

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @tbg)

pkg/ui/src/views/reports/containers/range/rangeTable.tsx, line 84 at r4 (raw file):

Previously, tbg (Tobias Grieger) wrote…

Nit: for bytes age you first show the number, then the average. Settle for one order and stick to it.

done

craig · 2019-06-20T21:28:22Z

Build failed

GitHub CI (Cockroach)

Release note: None

Before this patch, we were interpreting this gc score as a duration in nanos which made no sense. This patch displays it as a raw number and separately as an average age per dead byte. Same for the intent score. Fixes cockroachdb#38276 Release note (bug fix): The MVCC Intent Age and the MVCC GC Bytes Age fields were displayed with the wrong unit in the Range Status page.

andreimatei

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @tbg)

38283: ui: rationalize GC Bytes Age on the range status page r=andreimatei a=andreimatei Before this patch, we were interpreting this gc score as a duration in nanos which made no sense. This patch displays it as a raw number and separately as an average age per dead byte. Release note: None Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>

craig · 2019-06-25T18:51:27Z

Build failed

GitHub CI (Cockroach)

andreimatei

the bors build looks successful, don't know why it's complaining

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @tbg)

38283: ui: rationalize GC Bytes Age on the range status page r=andreimatei a=andreimatei Before this patch, we were interpreting this gc score as a duration in nanos which made no sense. This patch displays it as a raw number and separately as an average age per dead byte. Release note: None Co-authored-by: Andrei Matei <andrei@cockroachlabs.com>

craig · 2019-06-26T19:34:39Z

Build succeeded

GitHub CI (Cockroach)

andreimatei requested review from tbg and a team June 18, 2019 23:56

tbg mentioned this pull request Jun 19, 2019

server: age the MVCCStats returned from Range endpoint #38287

Merged

tbg requested changes Jun 19, 2019

View reviewed changes

andreimatei force-pushed the ui.gc_age_bytes branch from 8e6f6ee to cff4fe7 Compare June 19, 2019 17:39

andreimatei commented Jun 19, 2019

View reviewed changes

tbg approved these changes Jun 19, 2019

View reviewed changes

andreimatei force-pushed the ui.gc_age_bytes branch 2 times, most recently from 396d512 to 1d175a6 Compare June 20, 2019 20:18

andreimatei commented Jun 20, 2019

View reviewed changes

tbg approved these changes Jun 20, 2019

View reviewed changes

andreimatei force-pushed the ui.gc_age_bytes branch from 1d175a6 to f0774b1 Compare June 20, 2019 21:03

andreimatei commented Jun 20, 2019

View reviewed changes

ui: fix spelling

7ecb49b

Release note: None

andreimatei force-pushed the ui.gc_age_bytes branch from f0774b1 to fc2579c Compare June 25, 2019 17:14

andreimatei force-pushed the ui.gc_age_bytes branch from fc2579c to 5a413a7 Compare June 25, 2019 17:29

andreimatei commented Jun 25, 2019

View reviewed changes

andreimatei commented Jun 26, 2019

View reviewed changes

craig bot merged commit 5a413a7 into cockroachdb:master Jun 26, 2019

andreimatei deleted the ui.gc_age_bytes branch July 1, 2019 18:29

jseldess mentioned this pull request Aug 8, 2019

ui: rationalize GC Bytes Age on the range status page cockroachdb/docs#5195

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ui: rationalize GC Bytes Age on the range status page #38283

ui: rationalize GC Bytes Age on the range status page #38283

andreimatei commented Jun 18, 2019

cockroach-teamcity commented Jun 18, 2019

andreimatei commented Jun 19, 2019

tbg commented Jun 19, 2019

tbg left a comment

andreimatei left a comment

tbg left a comment

andreimatei left a comment

tbg left a comment

andreimatei left a comment

craig bot commented Jun 20, 2019

andreimatei left a comment

craig bot commented Jun 25, 2019

andreimatei left a comment

craig bot commented Jun 26, 2019

ui: rationalize GC Bytes Age on the range status page #38283

ui: rationalize GC Bytes Age on the range status page #38283

Conversation

andreimatei commented Jun 18, 2019

cockroach-teamcity commented Jun 18, 2019

andreimatei commented Jun 19, 2019

tbg commented Jun 19, 2019

tbg left a comment

Choose a reason for hiding this comment

andreimatei left a comment

Choose a reason for hiding this comment

tbg left a comment

Choose a reason for hiding this comment

andreimatei left a comment

Choose a reason for hiding this comment

tbg left a comment

Choose a reason for hiding this comment

andreimatei left a comment

Choose a reason for hiding this comment

craig bot commented Jun 20, 2019

Build failed

andreimatei left a comment

Choose a reason for hiding this comment

craig bot commented Jun 25, 2019

Build failed

andreimatei left a comment

Choose a reason for hiding this comment

craig bot commented Jun 26, 2019

Build succeeded