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

ui: time selection interference between the Insights page and the Statement Fingerprint page #97952

Closed
andreimatei opened this issue Mar 2, 2023 · 1 comment
Assignees
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@andreimatei
Copy link
Contributor

andreimatei commented Mar 2, 2023

Go to the Workload Insights page and select some time interval. Then click on a statement fingerprint to navigate to the Statement Fingerprint page. When you do that, you get a time selection on this page that reads something like 0m 22:10 - 22:10 (an empty interval). Which somehow gets translated into Showing aggregated stats from 22:00 to 22:59 (UTC).

The first funky thing is that, because of the 0m selection, the arrows for Previous/next time interval do not work.

The second funky thing is that, if you now navigate to back to the Insights page (either through the browser's back button or through clicking on the Insights link in the menu), the 0m 22:10 - 22:10 has overwritten whatever interval you had before on that page.

Jira issue: CRDB-24982

@andreimatei andreimatei added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-sql-observability Related to observability of the SQL layer T-sql-observability labels Mar 2, 2023
@ericharmeling ericharmeling self-assigned this Mar 6, 2023
ericharmeling added a commit to ericharmeling/cockroach that referenced this issue Mar 7, 2023
…details

Fixes cockroachdb#97952.

This commit removes the setTimeScale call from the StatementDetailsLink function.
Navigating to the Statement Details page from the Insights Details page will no
longer set the global time scale to 0 minutes.

Epic: none

Release note: None
ericharmeling added a commit to ericharmeling/cockroach that referenced this issue Mar 7, 2023
…t details

Part of cockroachdb#97952.

This commit removes the setTimeScale call from the StatementDetailsLink
and TransactionDetailsLink functions. Navigating to the Fingerprint Details
pages from the Insights Details pages will no longer set the global time scale.

Epic: none

Release note: None
ericharmeling added a commit to ericharmeling/cockroach that referenced this issue Mar 7, 2023
Fixes cockroachdb#97952.

This commit extends the window of the timeScale set by clicking
the statement details link from the Insight Overview and Statement
Insight Details pages to 65 minutes, to match the behavior of the
transaction details link.

Epic: none

Release note: None
craig bot pushed a commit that referenced this issue Mar 7, 2023
97805: settings: add COCKROACH_IGNORE_CLUSTER_SETTINGS env var r=dt a=dt

This provides an override usable in siturations where the persisted values for cluster settings may be causing a problem such that one cannot even start and interact with the the cluster enough to update them, such as if a given persisted setting causes nodes to crash on start or reject all SQL connections. In such cases, this env var provides a mechanism to start a node which will ignore the persisted values for all cluster settings and operate with just the default values instead, allowing alterations of the persisted values (though those alterations will similarly be ignored).

Release note (ops change): The enviornment variable COCKROACH_IGNORE_CLUSTER_SETTINGS can be used to start a node that ignores all stored cluster setting values in emergencies.
Epic: none.

97948: opt: don't fold sub-operators with null operands during type check r=DrewKimball a=DrewKimball

This commit prevents type-checking from replacing expressions like `NULL = ANY(...)` with `NULL`. This is necessary because in the case when the right operand is an empty array, the result of the expression is `False` instead of `NULL`. It is not always possible to know what the right operand will evaluate to, and constant folding can be handled during normalization anyway.

Fixes #37793

Release note (bug fix): Fixed a long-standing and rare bug in evaluation of `ANY`, `SOME`, and `ALL` sub-operators that would cause expressions like `NULL = ANY(ARRAY[]::INT[])` to return `NULL` instead of `False`.

98111: rangefeedcache: handle auth error in initial scan r=JeffSwenson a=JeffSwenson

Previously, if the rangfeedcache.Watcher ran into an auth error, it was never reported to the caller. This was detected as part of #97991. Moving the settingswatcher to be the first component initialized in the sql server caused the TestTenantUnauthenticatedAccess test to fail. Now, the error is reported to the caller and rebasing #97991 on top of this change allows the test to pass.

Part of #94843

Release note: None

98143: ui: remove timeScale update from fingerprint details link from insight details r=ericharmeling a=ericharmeling

Part of #97952.

This PR removes the setTimeScale call from the StatementDetailsLink
and TransactionDetailsLink functions. Navigating to the Fingerprint Details
pages from the Insights Details pages will no longer set the global time scale.

Loom (CC Console): https://www.loom.com/share/ca388c8c0068446d960f89cc6c1f6af5
Loom (DB Console): https://www.loom.com/share/209f2d21a1784d1d86adffabddb9b47f

Epic: none

Release note: None

Co-authored-by: David Taylor <tinystatemachine@gmail.com>
Co-authored-by: Drew Kimball <drewk@cockroachlabs.com>
Co-authored-by: Jeff <swenson@cockroachlabs.com>
Co-authored-by: Eric Harmeling <eric.harmeling@cockroachlabs.com>
@ericharmeling
Copy link
Contributor

Thanks for filing this issue!

We've got 2 changes merged that resolve it:

22.2: #98151
23.1: #98143

The 22.2 and 23.1 fixes differ because the Insights pages on 22.2 do not have time scale state. 22.2 insights surface executions that span across the lifetime of the insights and contention events internal tables, so we want to update the time scale state on the statement fingerprint details page to reflect the time scale for that "insight execution", and not use the global time scale state (either set to the default, or set earlier in the session on the overview page).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-observability Related to observability of the SQL layer C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

No branches or pull requests

2 participants