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

Provide a pop-up dialog to inform users the effects of resetting SQL stats #81867

Closed
kevin-v-ngo opened this issue May 25, 2022 · 5 comments · Fixed by #83108
Closed

Provide a pop-up dialog to inform users the effects of resetting SQL stats #81867

kevin-v-ngo opened this issue May 25, 2022 · 5 comments · Fixed by #83108
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 May 25, 2022

We should provide a pop-up dialog when users click on the "reset SQL stats" link to warn and inform them the effects of clicking the link. Statistics for both statements and transactions will be cleared and unrecoverable for all users across the entire cluster.

Jira issue: CRDB-16100

@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 May 25, 2022
@maryliag
Copy link
Contributor

@Annebirzin do we have any design we can use here?

@Annebirzin
Copy link

@maryliag @kevin-v-ngo I think we already address this with the current tooltip? The last line of the tooltip says:

...Clicking ‘reset SQL stats’ will reset SQL stats on the Statements and Transactions pages and crdb_internal tables.

Maybe we could update this sentence to add for all users across the cluster if that helps?

@maryliag
Copy link
Contributor

maryliag commented Jun 6, 2022

The problem is not the info, but it's that if the user accidentally clicks on the reset SQL stats there is no confirmation and maybe they didn't check the tooltip to realize it will reset the stats for all users.
So the confirmation is more to prevent mistakes.

@Annebirzin
Copy link

I see, that makes sense. We could add a modal like this so the user has to confirm the action:

Screen Shot 2022-06-06 at 11 09 14 AM

This action will reset SQL stats on the Statements and Transactions pages and crdb_internal tables. Statistics will be cleared and unrecoverable for all users across the entire cluster.

cc @stbof for any feedback on copy

@ghost
Copy link

ghost commented Jun 21, 2022

LGTM.

craig bot pushed a commit that referenced this issue Jun 21, 2022
78216: ui: decouple html base; allow development with short builds r=sjbarag,matthewtodd a=dhartunian

Previously, a few small issues prevented the UI development process from
making independent choices from CRDB. First, the base HTML template
contained data available in a global variable. Second, the "short" build
would break this template and keep us from developing on the UI when
we're just tweaking server endpoints.

This PR solves both problems. First, the templated variables are removed
and retrieved a startup in `index.tsx`. The React bootstrap waits for
the JSON response which is saved in a global variable by the JS code
instead of the backend. An attempt was made to remove the need for this
global variable but that changes quite a bit of code. In particular, all
of our links to docs pages rely on the global availability of the
cluster version number. Changing this would require that all docs link
generation functions take the version number as input instead of being
statically generated.

Second, the webpack dev server proxy will now attack a header to its
requests allowing us to notice this on the CRDB server and short circuit
the response on short builds to render the base HTML template as if it
was a "full" build. When the webpack dev server is running and serving
dev assets from memory, this lets us interact with DB Console on the
3000 port and develop using the hot reload workflow. This will shorten
dev feedback loops by making builds faster when backend endpoints are
being tweaked.

Release note: None

83021: cmd/reduce: reduce costfuzz logs r=msirek a=michae2

Add a `-costfuzz` mode to `reduce`, with special handling for logs
produced by the costfuzz roachtest. This mode is similar to `-tlp`.

The logs produced by costfuzz consist of various CREATE TABLE, INSERT,
UPDATE, and DELETE statements followed by three special statements:
1. A SELECT statement, called "control".
2. A SET testing_optimizer_random_cost_seed statement.
3. The same SELECT as (1), this time called "perturbed".

When the results of (1) and (3) do not match, we have a costfuzz
failure.

The `-costfuzz` mode removes these three special statements from the end
of the log, reduces the rest of the log, and then appends the statements
to the reduced log to check if the current reduction is interesting. As
long as the results of (1) and (3) continue to not match, the reduction
is interesting.

Also, make `reduce` run `cockroach demo` with an enterprise license. Now
that we use enterprise features in randgen we sometimes need this.

Informs: #81717

Release note: None

83108: ui: add confirmation modal for reset SQL Stats r=maryliag a=maryliag

Previously, there was not confirmation when the user
clicked on `reset SQL Stats`. This commit introduce
a confirmation modal, with a proper warning about
the data about to be deleted.

Fixes #81867

<img width="591" alt="Screen Shot 2022-06-20 at 4 18 51 PM" src="https://user-images.githubusercontent.com/1017486/174672688-4f983d9c-df8e-4ea6-9c59-6161fe761b09.png">

https://www.loom.com/share/9bd3c6af8f574453ac69e201697601b9

Release note (ui change): Add confirmation modal to `reset SQL Stats`

83130: dev: disable remote cache when building bazel-remote r=rail a=rickystewart

Typically you're building this binary because your cache is not up, but
if your cache is not up you can encounter a lot of these
`Connection refused` errors. Disable the remote cache when building in
this case.

Also disable `nogo` when building `dev`, similarly to save time, and
disable remote caching when building from the top-level `dev` script.

Release note: None

Co-authored-by: David Hartunian <[email protected]>
Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
@craig craig bot closed this as completed in 3be1098 Jun 21, 2022
blathers-crl bot pushed a commit that referenced this issue Jun 21, 2022
Previously, there was not confirmation when the user
clicked on `reset SQL Stats`. This commit introduce
a confirmation modal, with a proper warning about
the data about to be deleted.

Fixes #81867

Release note (ui change): Add confirmation modal to `reset SQL Stats`
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