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

Add bar charts to the Statements Details Overview page #74517

Closed
jocrl opened this issue Jan 6, 2022 · 2 comments · Fixed by #82426
Closed

Add bar charts to the Statements Details Overview page #74517

jocrl opened this issue Jan 6, 2022 · 2 comments · Fixed by #82426
Assignees
Labels
A-sql-console-general SQL Observability issues on the DB console spanning multiple areas. Includes Cockroach Cloud Console C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) sync-me-8

Comments

@jocrl
Copy link
Contributor

jocrl commented Jan 6, 2022

On both DB Console and Cluster UI
May not include the contention plot if that endpoint is not yet done.
Figma mockups: https://www.figma.com/file/xdmwvnFQd6KkO9RJ0XLDH0/22.1_SQL-obsrv_query-performance?node-id=4015%3A90729
Epic CRDB-13581

Blocked on #74516

Jira issue: CRDB-12131

@jocrl jocrl added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-observability A-sql-console-general SQL Observability issues on the DB console spanning multiple areas. Includes Cockroach Cloud Console labels Jan 6, 2022
@jocrl jocrl self-assigned this Jan 6, 2022
@maryliag maryliag assigned maryliag and unassigned jocrl Mar 2, 2022
@maryliag
Copy link
Contributor

maryliag commented Jun 3, 2022

@Annebirzin from the figma design, I can see you removed the values: Max memory usage, Max scratch disk usage, Distributed execution? and Full scan? that exist on the current page. I want to confirm if this was indeed on purpose.

@Annebirzin
Copy link

  • Distributed execution: I believe we removed because we now show regions on this page (which would imply distributed if there were more than one)
  • Full scan: I think we could keep this in, not sure why we removed. I added to the designs here (As well as 'Nodes' which was missing)
  • Max memory and Max scratch disk usage: I think we removed because the concept of 'Max' usage isn't as useful now that this page represents multiple aggregated intervals. @kevin-v-ngo any thoughts on how we might want to show these metrics (would they make sense as charts?)

maryliag added a commit to maryliag/cockroach that referenced this issue Jun 7, 2022
The overview for a statement details page now
shows charts instead of just the mean value for:

- Execution and Planning Time
- Rows Processed
- Execution Retries
- Execution Count
- Contention

Fixes cockroachdb#74517

This commit also introduces mock for matchMedia and canvas
used for testing with jest.

Release note (ui change): Statement Details page now shows charts
for: Execution and Planning Time, Rows Processed, Execution Retries,
Execution Count and Contention.
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 8, 2022
The overview for a statement details page now
shows charts instead of just the mean value for:

- Execution and Planning Time
- Rows Processed
- Execution Retries
- Execution Count
- Contention

Fixes cockroachdb#74517
Fixes cockroachdb#81153

This commit also introduces mock for matchMedia and canvas
used for testing with jest.

Release note (ui change): Statement Details page now shows charts
for: Execution and Planning Time, Rows Processed, Execution Retries,
Execution Count and Contention.
craig bot pushed a commit that referenced this issue Jun 9, 2022
81071: sql: Allow DEFAULT/ON UPDATE types that can be assignment-cast r=e-mbrown a=e-mbrown

Resolves #74854

postgres:
```
postgres=# create table casts (b BOOL DEFAULT 'foo'::VARCHAR);
 ERROR:  column "b" is of type boolean but default expression is of type character varying>
HINT:  You will need to rewrite or cast the expression.
```
crdb:
  ```
[email protected]:26257/movr> CREATE TABLE t (b BOOL DEFAULT 'foo'::VARCHAR);
CREATE TABLE

Time: 8ms total (execution 8ms / network 0ms)
```

Release note (sql change): A column's DEFAULT/ON UPDATE clause
can now have a type that differs from the column type, as long as
that type can be assignment-cast into the column's type. This
increases our PostgreSQL compatibility.

81160: opt: simplify lookup join constraint building r=mgartner a=mgartner

#### opt: simplify lookup join constraint building

The lookup join constraint builder has been simplified so that it builds
constraints in a single pass over an index's columns. This is in
contrast to the previous method which would require a second pass over
the index's columns after it was discovered that a multi-span lookup
join with a lookup expression would be required. This change has a
number of consequences:

  1. Lookup joins can be planned in more cases. As an example, a lookup
     join can now be planned when there is an equality filter on a
     indexed computed column and a range filter on another indexed
     column.
  2. Constant value filters, like `x = 1`, are no longer included in
     lookup expressions. Instead, the constant value will be projected
     and the lookup expression will contain an equality and the
     projected column, like `x = projected_col`. This should have
     little, if any, effect on the performance of lookup joins.

This change required a minor change to the
`GenerateLocalityOptimizedAntiJoin` rule to ensure that projected
constant values are produced by the the inner anti-join. This is
necessary because the outer anti-join's lookup expression references the
projected column. This wasn't previously necessary because constant
values weren't projected and referenced in lookup expressions - they
were inlined directly in lookup expressions.

Release note: None

#### opt: remove "cross-join" from some tests

As of #79586, we no longer plan cross-joins as inputs of lookup joins.
This commit updates some test comments to reflect this.

Release note: None


81713: kvserver: benchmark empty replica addition/removal r=aayushshah15 a=aayushshah15

This commit adds a small benchmark to track low level regressions in the time it takes to add/remove an empty range's replica.

Relates to #72083

Release note: None

82317: ccl/sqlproxyccl: enable a more graceful shutdown r=pjtatlow a=pjtatlow

When a drainSignal is received, the sql proxy now waits for all connections to
close within a certain time limit (59 minutes) before shutting down.

The next drainSignal will be ignored, but the third will forcefully shut down
the server by panicking. This is to resolve an issue with Kubernetes where
traffic could be lost during upgrades. See CC-5298 for more details.

Release notes: None

82426: ui: new charts to statement details page r=maryliag a=maryliag

The overview for a statement details page now
shows charts instead of just the mean value for:

- Execution and Planning Time
- Rows Processed
- Execution Retries
- Execution Count
- Contention

https://www.loom.com/share/2b6d0311e61a433d81ad37b8b62fba7d

<img width="1566" alt="Screen Shot 2022-06-06 at 6 14 41 PM" src="https://user-images.githubusercontent.com/1017486/172258188-d0367377-140e-4863-8ecf-f06f14a0e277.png">
<img width="1576" alt="Screen Shot 2022-06-03 at 4 53 49 PM" src="https://user-images.githubusercontent.com/1017486/171951030-0535484d-f4f2-4af4-aa50-f53247951f8c.png">
<img width="1575" alt="Screen Shot 2022-06-03 at 4 54 02 PM" src="https://user-images.githubusercontent.com/1017486/171951037-af6bf17d-0128-49dc-861a-250adaa0f383.png">


Fixes #74517

Release note (ui change): Statement Details page now shows charts
for: Execution and Planning Time, Rows Processed, Execution Retries,
Execution Count and Contention.

82561: sql: put connExecutor's AutoRetry fields into txnState's mutex r=ajwerner a=RichardJCai

Auto retry variables could be used outside the connExecutor's goroutine in calls to
serialize. If this is the case, the field should be in txnState's mutex struct.

Release note: None

Fixes #82506
Fixes #78178

82658: storage: stop writing SST table properties r=jbowens a=erikgrinaker

**util/hlc: add `Timestamp.WallNext()`**

Release note: None

**storage: stop writing SST table properties**

Previously, SSTables were written with the table properties
`crdb.ts.min` and `crdb.ts.max` tracking the minimum and maximum
timestamps in the SSTable. This was used to filter SSTables via the
`MVCCIterator` options `MinTimestampHint` and `MaxTimestampHint`,
aka time-bound iterators.

In 22.1, we additionally began writing the block property
`MVCCTimeInterval` containing the minimum and maximum timestamps for
each SSTable block, aggregated up to the SSTable. Time-bound iterators
filter on this block property in addition to the table properties.

In 22.2, we will be introducing Pebble range keys. Table properties do
not support range keys, so we must migrate entirely to block properties.

This patch therefore stops writing table properties entirely. This
ensures that SSTables containing range keys (introduced in 22.2) will
not have table properties -- conversely, SSTables that do have table
properties can only contain point keys. By doing so, we avoid
erroneously filtering out SSTables with range keys that satisfy the time
predicate but were not taken into account by table properties. A later
patch will add block property collectors for range keys.

We still respect table properties while filtering, to maintain
compatibility with SSTables written by 21.2 nodes and earlier. We must
retain this backwards compatibility as long as it is possible for a
cluster to have SSTables written by older nodes, which can happen e.g.
if a cluster is upgraded from 21.2 to 22.2 in rapid succession.

The change from table to block properties slightly changes the filtering
semantics, as block properties discard the logical timestamp component.
This should not be an issue, since timestamp hints are inclusive, and
TBIs are allowed to emit false positives.

This patch also removes the `IteratorStats.TimeBoundNumSSTs` field, as
block property filters do not update it.

Touches #82596.

Release note: None
  
**storage: remove `pebbleDeleteRangeCollector`**

This was a noop property collector no longer in use.

Release note: None

82664: roachtest: skip flaky costfuzz r=meridional a=meridional

Github issue: #81717

Release note: None

Co-authored-by: e-mbrown <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Aayush Shah <[email protected]>
Co-authored-by: PJ Tatlow <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: richardjcai <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Ye Ji <[email protected]>
@craig craig bot closed this as completed in c3eda02 Jun 9, 2022
maryliag added a commit to maryliag/cockroach that referenced this issue Jun 15, 2022
The overview for a statement details page now
shows charts instead of just the mean value for:

- Execution and Planning Time
- Rows Processed
- Execution Retries
- Execution Count
- Contention

Fixes cockroachdb#74517
Fixes cockroachdb#81153

This commit also introduces mock for matchMedia and canvas
used for testing with jest.

Release note (ui change): Statement Details page now shows charts
for: Execution and Planning Time, Rows Processed, Execution Retries,
Execution Count and Contention.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-console-general SQL Observability issues on the DB console spanning multiple areas. Includes Cockroach Cloud Console C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) sync-me-8
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants