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: sessions duration time for closed session is incorrectly calculated #85968

Closed
xinhaoz opened this issue Aug 11, 2022 · 0 comments · Fixed by #85974
Closed

ui: sessions duration time for closed session is incorrectly calculated #85968

xinhaoz opened this issue Aug 11, 2022 · 0 comments · Fixed by #85974
Assignees
Labels
A-sql-cli-observability Issues related to surfacing SQL observability in SHOW, CRDB_INTERNAL, SYSTEM, etc. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@xinhaoz
Copy link
Member

xinhaoz commented Aug 11, 2022

Currently for closed sessions in the sessions pages, the session duration time is calculated as if the session were active, i.e. duration = now - start. For closed sessions, it should be duration = end - now.

Jira issue: CRDB-18507

@xinhaoz xinhaoz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-sql-observability A-sql-cli-observability Issues related to surfacing SQL observability in SHOW, CRDB_INTERNAL, SYSTEM, etc. labels Aug 11, 2022
@xinhaoz xinhaoz self-assigned this Aug 11, 2022
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Aug 11, 2022
Fixes cockroachdb#85968
Closes cockroachdb#85912
Closes cockroachdb#85973

This commit adds new details to the active execution details pages:
full scan (both stmt and txn), priority (txn only), and last retry
reason (txn only). New information is also added to the sessions
table and details pages: transaction count, active duration,
recent txn fingerprint ids (cache size comes from a cluster setting).

This commit also fixes a bug in the sessions overview UI where
the duration for closed sessions was incorrectly calcualted based
on the current time instead of the session end time.

Release note (ui change): the following fields have been added to
the active stmt/txn details pages:
- Full Scan: indicates if the execution contains a full scan
- Last Retry Reason (txn page only): the last recorded reason the
txn was retried
- Priority (txn page only): the txn priority
The following fields have been added to the sessions table and page:
- Transaction  count: the number of txns executed by the session
- Session active duration: the time a session spent executing txns
- Session most recent fingerprint ids
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Aug 12, 2022
Fixes cockroachdb#85968
Closes cockroachdb#85912
Closes cockroachdb#85973

This commit adds new details to the active execution details pages:
full scan (both stmt and txn), priority (txn only), and last retry
reason (txn only). New information is also added to the sessions
table and details pages: transaction count, active duration,
recent txn fingerprint ids (cache size comes from a cluster setting).

This commit also fixes a bug in the sessions overview UI where
the duration for closed sessions was incorrectly calcualted based
on the current time instead of the session end time.

Release note (ui change): the following fields have been added to
the active stmt/txn details pages:
- Full Scan: indicates if the execution contains a full scan
- Last Retry Reason (txn page only): the last recorded reason the
txn was retried
- Priority (txn page only): the txn priority
The following fields have been added to the sessions table and page:
- Transaction  count: the number of txns executed by the session
- Session active duration: the time a session spent executing txns
- Session most recent fingerprint ids
craig bot pushed a commit that referenced this issue Aug 13, 2022
77070: sql: add SHOW STATISTICS WITH FORECAST r=michae2 a=michae2

**sql/stats: replace eval.Context with tree.CompareContext**

Most uses of eval.Context in the sql/stats package can actually be
tree.CompareContext instead, so make the replacement.

Release note: None

**sql/stats: bump histogram version to 2**

In 22.2 as of 963deb8 we support multiple histograms for trigram-
indexed strings. Let's bump the histogram version for this change, as we
may want to know whether multiple histograms are possible for a given
row in system.table_statistics.

(I suspect that during upgrades to 22.2 the 22.1 statistics builder will
choke on these statistics, so maybe we should also backport a version
check to 22.1.)

Also update avgRefreshTime to work correctly in multiple-histogram
cases.

Release note: None

**sql/stats: teach histogram.adjustCounts to remove empty buckets**

Sometimes when adjusting counts down we end up with empty buckets in the
histogram. They don't hurt anything, but they take up some memory (and
some brainpower when examining test results). So, teach adjustCounts to
remove them.

Release note: None

**sql/stats: always use non-nil buckets for empty-table histograms**

After 82b5926 I've been using the convention that nil histogram
buckets = no histogram, and non-nil-zero-length histogram buckets =
histogram on empty table. This is mostly useful for testing but is also
important for forecasting histograms.

Fix a spot that wasn't following this convention.

Also, add some empty-table testcases and some other testcases for
histogram.adjustCounts.

Release note: None

**sql/stats: make ordering of SHOW STATISTICS more deterministic**

Make two changes to produce more deterministic SHOW STATISTICS output:

1. Sort column IDs when creating statistics. We already use `FastIntSet`
   in both create_stats.go and statistics_builder.go to ignore ordering
   when gathering statistics by column set, but the column ordering from
   CREATE STATISTICS leaks into `system.table_statistics` and can affect
   SQL on that table, such as SHOW STATISTICS and various internal
   DELETE statements.

2. Order by column IDs and statistic IDs when reading from
   `system.table_statistics` in both SHOW STATISTICS and the stats
   cache. This will ensure SHOW STATISTICS always produces the same
   output, and shows us rows in the same order as the stats cache sees
   them (well, reverse order of the stats cache).

Release note (sql change): Make SHOW STATISTICS output more
deterministic.

**sql/stats: forecast table statistics**

Add function to forecast table statistics based on observed statistics.
These forecasts are based on linear regression models over time. For
each set of columns with statistics, we construct a linear regression
model over time for each statistic (row count, null count, distinct
count, average row size, and histogram). If all models are good fits
then we produce a statistics forecast for the set of columns.

Assists: #79872

Release note: None

**sql: add SHOW STATISTICS WITH FORECAST**

Add a new WITH FORECAST option to SHOW STATISTICS which calculates and
displays forecasted statistics along with the existing table statistics.

Also, forbid injecting forecasted stats.

Assists: #79872

Release note (sql change): Add a new WITH FORECAST option to SHOW
STATISTICS which calculates and displays forecasted statistics along
with the existing table statistics.

85673: storage: Incrementally calculate range key stats in CheckSSTConflicts r=erikgrinaker a=itsbilal

This change updates CheckSSTConflicts to incrementally
calculate stats in the presence of range keys in the
SST being ingested. This avoids expensive stats recomputation
after AddSSTable, as previously we were marking stats as
estimates if an SST with range keys was added.

Fixes #83405.

Release note: None.

85794: sql: add not visible index to optimizer r=wenyihu6 a=wenyihu6

This commit adds the logic of the invisible index feature to the optimizer.
After this commit has been merged, the invisible index feature should be fully
functional with `CREATE INDEX` and `CREATE TABLE`.

Assists: #72576

See also: #85239

Release note (sql change): creating a not visible index using
`CREATE TABLE …(INDEX … NOT VISIBLE)` or
`CREATE INDEX … NOT VISIBLE` is now supported.

85974: cluster-ui: update active execution and sessions details r=xinhaoz a=xinhaoz

Fixes #85968
Closes #85912
Closes #85973

This commit adds new details to the active execution details pages:
full scan (both stmt and txn), priority (txn only), and last retry
reason (txn only). New information is also added to the sessions
table and details pages: transaction count, active duration,
recent txn fingerprint ids (cache size comes from a cluster setting).

This commit also fixes a bug in the sessions overview UI where
the duration for closed sessions was incorrectly calcualted based
on the current time instead of the session end time.

Release note (ui change): the following fields have been added to
the active stmt/txn details pages:
- Full Scan: indicates if the execution contains a full scan
- Last Retry Reason (txn page only): the last recorded reason the
txn was retried
- Priority (txn page only): the txn priority
The following fields have been added to the sessions table and page:
- Transaction  count: the number of txns executed by the session
- Session active duration: the time a session spent executing txns
- Session most recent fingerprint ids

-------------------------------------
Retry reason populated example:
<img width="855" alt="image" src="https://user-images.githubusercontent.com/20136951/184201435-6d585d9b-13a9-4e87-86dd-718f03f9e92a.png">
https://www.loom.com/share/e396d5aa7dda4d5995227154c6b5076b

Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: Bilal Akhtar <[email protected]>
Co-authored-by: wenyihu3 <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
@craig craig bot closed this as completed in ac161dd Aug 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-cli-observability Issues related to surfacing SQL observability in SHOW, CRDB_INTERNAL, SYSTEM, etc. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant