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

Increase visibility as to which interval is being shown on SQL Activity pages #82914

Closed
kevin-v-ngo opened this issue Jun 14, 2022 · 9 comments · Fixed by #83229
Closed

Increase visibility as to which interval is being shown on SQL Activity pages #82914

kevin-v-ngo opened this issue Jun 14, 2022 · 9 comments · Fixed by #83229
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@kevin-v-ngo
Copy link

kevin-v-ngo commented Jun 14, 2022

We should increase the visibility and provide clarity as to which start and end time interval is being shown on the SQL Activity pages since we are showing the last full hour.

Few options:

  1. Remove the 'Past 10 minutes' and 'Past 30 minutes' since we always show only the past full hour
  2. Prominently Highlight in the UI the intervals we are showing stats for since a custom time or 'Past 1 hour' could fall within an hour

CC @Annebirzin

Jira issue: CRDB-16725

@kevin-v-ngo kevin-v-ngo added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jun 14, 2022
@maryliag
Copy link
Contributor

we do have an issue to add this info on the details page, should we also add to the main page?

Screen Shot 2022-06-14 at 7 39 27 PM

@kevin-v-ngo
Copy link
Author

Thanks @maryliag! Yeah, i couldn't find this issue as I was searching for different key words. We can close this as a dup or perhaps just scope this issue to removing the "Past 10 minutes" and "Past 30 minutes". Not sure if we can conditionally do this for the SQL Activity pages since i remember this picker was shared with our metrics pages.

@Annebirzin
Copy link

Annebirzin commented Jun 15, 2022

@maryliag @kevin-v-ngo Along with removing 'past 10/30 minutes', we could include the text about the selected timeframe to the Statement/Transaction overview pages as well as the fingerprint. Maybe something like this next to the page count? (not sure if we have a learn more doc to point to about this time aggregation?)

Screen Shot 2022-06-15 at 2 12 03 PM

Also made a couple tweaks to the copy:

  • removed the word Currently
  • updated time to 24 hour

Here is the updated fingerprint page:

Screen Shot 2022-06-15 at 2 12 22 PM

Also note that on the fingerprint explain plan tab, the copy is updated from aggregated stats to explain plans

Screen Shot 2022-06-15 at 2 12 38 PM

@maryliag maryliag self-assigned this Jun 15, 2022
@maryliag
Copy link
Contributor

Btw, one thing I noticed was that rounding the end date to the next X:00 was a little weird, specially on the Statement Details page, because you would see the values for the next hour (because of the way we group in 1h buckets), so I changed to the end value going until X:59, so on your example, with the time picker on May 5, 14:34 - May 5, 20:34 the label would be Showing aggregated stats from May 5, 14:00 to 20:59

Also, since we're using 24h format, no need for the PM, right?

@Annebirzin
Copy link

ah yes, removed the PM and also updated the time to 20:59, thanks!

maryliag added a commit to maryliag/cockroach that referenced this issue Jun 20, 2022
This commit adds the period label with information
about the period to which we're showing information from.
The label is added to Statement, Statement Details,
Transaction and Transaction Details pages.

Partially addresses cockroachdb#82914
Fixes cockroachdb#74523

This commit also removed the tab Exec Stats from the
Statement Details page.

Fixes cockroachdb#74526

Release note (ui change): Add period label to pages Statement,
Statement Details and Transaction, with information about the
period to which we're showing information from. Removal of
exec stats tab under Statement Details page.
@maryliag
Copy link
Contributor

For the metrics page we do have info about last 10 or 30min, are we also removing the options from that page? Just thinking about alignment, since we want to have a global value that if you change in one, also affects the other and it would be very janky with those options in just one of the pages.

@Annebirzin
Copy link

Annebirzin commented Jun 21, 2022

hm yea this is tricky.

Anecdotally, for DB Console I've heard users like defaulting to the smaller time windows (10/30 minutes). And I've also seen feedback for DB console where users want to drag-to-zoom time periods even smaller than 10 minutes.

We also default to 30 minutes on the Serverless Overview page.

A couple of options:

  • remove 10/30 minute options, and default to 'past hour' across all time pickers (Overview, Metrics, SQL Activity). For Serverless, the user would have to use the custom time picker to get a smaller time window since we don't have drag-to zoom. In DB console they could still use drag-to-zoom for smaller time windows.
  • Keep the options for past 10/30 minute views on the Overview and Metrics pages (defaulting to 30 minutes). If the user navigations to SQL Activity pages then the time picker defaults to past hour.

@kevin-v-ngo maybe we can look at what something like PG-Analyze does in this case? Or maybe they don't have the same hour time aggregations we have?

@maryliag
Copy link
Contributor

I do think we should keep the 10/30min option, but the concern I had with option 2 is the flow:
Metrics page select 10 min -> Go to SQL Activity page -> The time picker will change the value to 1h -> go back to Metrics page will show 1h and not the 10min
is that okay?

@Annebirzin
Copy link

Makes sense, I think that behavior works

maryliag added a commit to maryliag/cockroach that referenced this issue Jun 22, 2022
Previously we had the options for 10 and 30min on
SQL Activity pages, which created some confusion, since
we would always show the last 1h info.
This commit remove those 2 options.
If the user select any of those options on the Metrics
page, it will get updated to 1h on the SQL Activity
pages.

Fixes cockroachdb#82914

Release note (ui change): Removal of the 10 and 30min options
on the SQL Activity page.
craig bot pushed a commit that referenced this issue Jun 23, 2022
83103: ui: add period label to SQL Activity pages r=maryliag a=maryliag

This commit adds the period label with information
about the period to which we're showing information from.
The label is added to Statement, Statement Details,
Transaction and Transaction Details pages.

Partially addresses #82914
Fixes #74523

Statement page no filter
<img width="851" alt="Screen Shot 2022-06-20 at 10 17 23 AM" src="https://user-images.githubusercontent.com/1017486/174626137-729a8351-a32d-46df-a884-a9aeb49a8ace.png">

Statement Page with filter
<img width="883" alt="Screen Shot 2022-06-20 at 10 17 07 AM" src="https://user-images.githubusercontent.com/1017486/174626135-e39be0d4-045c-49bc-b886-4337cb4baf21.png">

Statement Details Overview Tab
<img width="468" alt="Screen Shot 2022-06-20 at 10 18 24 AM" src="https://user-images.githubusercontent.com/1017486/174626138-2b5bae17-f64f-4ddd-a8eb-de0f92f53a3b.png">

Statement Details Explain Tab
<img width="463" alt="Screen Shot 2022-06-20 at 10 18 30 AM" src="https://user-images.githubusercontent.com/1017486/174626141-1e925490-667c-47e9-bc6e-b8227d189c1a.png">

Transaction page no filter
<img width="874" alt="Screen Shot 2022-06-20 at 10 21 24 AM" src="https://user-images.githubusercontent.com/1017486/174626144-10448c80-df40-4b85-bae5-19484fa97752.png">

Transaction page with filter
<img width="756" alt="Screen Shot 2022-06-20 at 10 21 38 AM" src="https://user-images.githubusercontent.com/1017486/174626146-16a0ad7b-7506-4447-8f5e-c29a0d9cde56.png">

Transaction Details
<img width="647" alt="Screen Shot 2022-06-20 at 10 36 51 AM" src="https://user-images.githubusercontent.com/1017486/174626147-de9619c6-b11e-42c7-90b3-57a5bed1f6f6.png">


This commit also removed the tab Exec Stats from the
Statement Details page.

Fixes #74526

Release note (ui change): Add period label to pages Statement, Statement Details, 
Transaction and Transaction Details, with information about the
period to which we're showing information from. Removal of
Exec stats tab under Statement Details page.

Co-authored-by: Marylia Gutierrez <[email protected]>
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 24, 2022
This commit adds the period label with information
about the period to which we're showing information from.
The label is added to Statement, Statement Details,
Transaction and Transaction Details pages.

Partially addresses cockroachdb#82914
Fixes cockroachdb#74523

This commit also removed the tab Exec Stats from the
Statement Details page.

Fixes cockroachdb#74526

Release note (ui change): Add period label to pages Statement,
Statement Details and Transaction, with information about the
period to which we're showing information from. Removal of
exec stats tab under Statement Details page.
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 28, 2022
Previously we had the options for 10 and 30min on
SQL Activity pages, which created some confusion, since
we would always show the last 1h info.
This commit remove those 2 options.
If the user select any of those options on the Metrics
page, it will get updated to 1h on the SQL Activity
pages.

Fixes cockroachdb#82914

Release note (ui change): Removal of the 10 and 30min options
on the SQL Activity page.
craig bot pushed a commit that referenced this issue Jun 28, 2022
82352: server, sql: surface session txnCount, recent txn fingerprints, active time r=xinhaoz a=xinhaoz

Finishing up Gerardo's PR, original review here: #80717

--------------------------------------------------
Partially addresses #74257.

Previously, the status server did not provide session details such as
total number of transactions executed, transaction fingerprint
IDs, and total active time. This change adds the aforementioned session
details to the `serverpb.Session` struct.

To track recently executed transaction fingerprint IDs, a FIFO cache
`TxnFingerprintIDCache` is introduced with its corresponding cluster
setting `TxnFingerprintIDBufferCapacity` to control the capacity. The
default capacity is set at 100 fingerprints.

The total number of transactions executed is filled using the existing
`txnCounter` from the `extraTxnState` in `connExecutor`. The total active
time is calculated by introducing a `timeutil.StopWatch` to the connection
executor, which is started and stopped when a transaction is started and
finished respectively.

Release note (api change): the `serverpb.Session` struct now has three
new fields: number of transactions executed, transaction fingerprint
IDs, and total active time.

82623: backupinfo: introduce a backupinfo package r=stevendanna a=adityamaru

The backupinfo package contains logic related to interacting
with information and metadata describing the backup. After this
change we have `backupdest` depending on `backupinfo`.

Release note: None

82718: kvserver: emit MVCC range tombstones over rangefeeds r=aliher1911 a=erikgrinaker

This patch adds MVCC range tombstone support in rangefeeds. Whenever an
MVCC range tombstone is written, a new `MVCCDeleteRangeOp` logical op
is recorded and emitted across the rangefeed as a `RangeFeedDeleteRange`
event. MVCC range tombstones will only be written when the
`MVCCRangeTombstones` version gate has been enabled.

Changefeeds will emit an error for these events. We do not expect to see
these in online spans with changefeeds, since they are initially only
planned for use with schema GC and import rollbacks.

The rangefeed client library has been extended with support for these
events, but no existing callers handle them for the same reason as
changefeeds. Initial scans do not emit regular tombstones, and thus not
range tombstones either, but catchup scans will emit them if
encountered.

This patch has rudimentary testing of MVCC range tombstones in
rangefeeds. A later patch will add a data-driven test harness for
rangefeeds with more exhaustive tests.

Resolves #82449.
Touches #70433.

Release note: None

82936: sql/schemachanger: implement DROP OWNED BY r=jasonmchan a=jasonmchan

Previously, we did not support the DROP OWNED BY statement (#55381).
This commit adds partial support for DROP OWNED BY in the declarative
schema changer. Followup work is needed to support the CASCADE modifier.

Release note (sql change): Support `DROP OWNED BY`.

83229: ui: remove option 10/30 min from SQL Activity page r=maryliag a=maryliag

Note to reviewers: only 2nd commit is relevant to this PR

Previously we had the options for 10 and 30min on
SQL Activity pages, which created some confusion, since
we would always show the last 1h info.
This commit remove those 2 options.
If the user select any of those options on the Metrics
page, it will get updated to 1h on the SQL Activity
pages.

<img width="444" alt="Screen Shot 2022-06-22 at 5 43 53 PM" src="https://user-images.githubusercontent.com/1017486/175144243-2f084e0b-5e09-4874-9640-e7eea6179343.png">

https://www.loom.com/share/226e54322df6456aa2039b5c54f72eb1


Fixes #82914

Release note (ui change): Removal of the 10 and 30min options
on the SQL Activity page.

83420: ui: improve tooltip UX with text updates r=ericharmeling a=ericharmeling

Fixes #81374.
Fixes #83256.
Fixes #81248.
Fixes #79018.

Note the following:

- The updates resolving #79018 effectively revert the tooltip text for Rows Read to the original wording (which [was updated for accuracy](e379e9d#diff-492398441e971e355a687a4ce333a9766e2195287d0227682444d5dc0eb7ee1a)). I assume this is okay. `@kevin-v-ngo`
- The updates resolving #81248 do not in fact refer to the time intervals as date ranges, as this language is misleading (a 1h interval is an interval and not a date range). Instead, this update just removes the anchor and the link to the non-existent Interval Range section of https://www.cockroachlabs.com/docs/stable/ui-statements-page.html. We may want to consider updating the docs to call the "time picker" data type a time interval and not a date range. This appears to have been the case in previous releases (https://www.cockroachlabs.com/docs/v21.1/ui-statements-page#time-interval). `@stbof` 

Release note (ui change): Updated tooltips on the Statements and
Transactions pages in the DB Console for improved UX.

83428: sql: rename anonymizedStmt in sqlstats pkg to stmtNoConstants r=ericharmeling a=ericharmeling

Note that this commit does not change any files outside the sqlstats package.

Fixes #80725.

Release note: None

83468: ui: update all dates to use same format r=maryliag a=maryliag

Update all dates to use the same format.

Fixes #81159

Release note: None

83520: kv: don't try to reject lease transfer when flushing proposal buffer r=nvanbenschoten a=nvanbenschoten

Fixes #83498.
Fixes #83402.
Fixes #83308.

This was fallout from #82758.

This commit adds logic to `propBuf.maybeRejectUnsafeProposalLocked` to avoid
trying to reject proposals based on the state of the raft group when the group
is not provided (e.g. when flushing the buffer). We already had this logic for
`RequestLease` (indirectly), but did not for `TransferLease`.

Co-authored-by: Gerardo Torres <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Jason Chan <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Eric Harmeling <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in e4d7f25 Jun 28, 2022
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 28, 2022
Previously we had the options for 10 and 30min on
SQL Activity pages, which created some confusion, since
we would always show the last 1h info.
This commit remove those 2 options.
If the user select any of those options on the Metrics
page, it will get updated to 1h on the SQL Activity
pages.

Fixes cockroachdb#82914

Release note (ui change): Removal of the 10 and 30min options
on the SQL Activity page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants