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: unexpected increase of transaction count in transaction page #68375

Closed
Azhng opened this issue Aug 3, 2021 · 4 comments · Fixed by #98307
Closed

ui: unexpected increase of transaction count in transaction page #68375

Azhng opened this issue Aug 3, 2021 · 4 comments · Fixed by #98307
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

@Azhng
Copy link
Contributor

Azhng commented Aug 3, 2021

if you have a transaction with a non-1 execution count, and then re-sort the page by execution count, the execution count goes up.

image

image

The execution count goes down after page refresh.

Jira issue: CRDB-9009

@Azhng Azhng 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 Aug 3, 2021
@jordanlewis
Copy link
Member

Repro:

BEGIN;
SELECT 1;
SELECT 1;
SELECT 2;
COMMIT;

@maryliag maryliag assigned Azhng and unassigned matthewtodd Oct 4, 2021
@maryliag maryliag assigned xinhaoz and unassigned Azhng Oct 6, 2021
@jocrl jocrl self-assigned this Mar 21, 2022
@jocrl
Copy link
Contributor

jocrl commented Mar 21, 2022

@Azhng, would you mind checking if you can still repro this? I can't seem to repro this in DB Console or CC Console (dedicated or serverless), for statements or transactions.
I ran:

./cockroach demo --no-example-database --log-dir logs --insecure --multitenant=false
# then the stuff from Jordan's comment

In the video below I ran into a different bug on CC Console, so no demo for that irreproducibility but I'll file an issue for the bug 😛 it doesn't reproduce in a private window, and I can't figure out anything on staging in part due to the lack of source maps.

779678.cannot.repro.incrementing.transactions.count.mov

@Azhng
Copy link
Contributor Author

Azhng commented Mar 21, 2022

iirc this bug had a lot of randomness in it, so the reprod approach Jordan initially discovered doesn't always reliably reproduce this bug. I'll do some digging there to see if i can find something.

Edit Oh wait nvm I saw you self-assigned it.

@jocrl jocrl removed their assignment Mar 21, 2022
@jocrl
Copy link
Contributor

jocrl commented Mar 21, 2022

Ah okay, I picked up something else in the meantime, so clearing the assignment. It's yours to take if you like! Otherwise, thanks and I'll do the digging if I end up picking it up later.

craig bot pushed a commit that referenced this issue Mar 9, 2023
96967: changefeedccl: skip testing queries that are too slow as regular SQL r=[samiskin] a=HonoreDB

TestChangefeedRandomExpressions was occasionally timing out when doing the regular SELECT query--it's tricky to get sqlsmith not to generate complex expressions that are likely to not be valid for changefeeds anyway, so this PR just skips predicates that take more than a second to process.

Informs #96532.

Release note: None

97860: jobs: add VIEWJOB global privilege, remove role option r=jayshrivastava a=jayshrivastava

This change updates `VIEWJOB` to be a global privilege instead of a role option so that it can be inherited from roles to their members.

Previously, `VIEWJOB` was a role option which could be granted to users. Now, `VIEWJOB` is a global privilege. Granting this privilege to a user or role has the syntax `GRANT SYSTEM VIEWJOB TO user`. Using `VIEWJOB` as a role option is deprecated.

Note that the `VIEWJOB` role option was not included in any release so far. It was queued up to be released in 23.1, but was not. This change is also being queued for 23.1, so there should not be any backwards compatibility issues.

Informs: #96382
Epic: None
Release Note: None

98135: cdc: copy request body when registering schemas r=jayshrivastava a=jayshrivastava

cdc: copy request body when registering schemas
  
Previously, when the schema registry encountered an error when
registering a schema, it would retry the request. The problem
is that upon hitting an error, we clean the body before retrying.
Retrying with an empty body results in a obscure error message.
With this change, we now retry with the original request body
so the original error is sustained.

This change also adds the metric `changefeed.schema_registry.retry_count`
which is a counter for the number of retries performed by the schema
registry. Seeing nonzero values indicates that there is an issue
with contacting the schema registry and/or registering schemas.

Release note (ops change): A new metric `changefeed.schema_registry.retry_count`
is added. This measures the number of request retries performed when
sending requests to the schema registry. Observing a nonzero value may indicate
improper configuration of the schema registry or changefeed parameters.
Epic: None

98212: authors: add Mira Radeva to authors r=miraradeva a=miraradeva

Release note: None
Epic: None

98249: backupccl: incremental schedules always wait on_previous_running r=benbardin a=adityamaru

An incremental backup schedule must always wait if there is a running job
that was previously scheduled by this incremental schedule. This is
because until the previous incremental backup job completes, all future
incremental jobs will attempt to backup data from the same `StartTime`
corresponding to the `EndTime` of the last incremental layer. In this
case only the first incremental job to complete will succeed, while the
remaining jobs will either be rejected or worse corrupt the chain of
backups.

This change overrides the Wait behaviour for an incremental schedule to
always default to `wait` during schedule creation or in an alter statement.
Note the user specified value will still be applied to the full backup schedule.

Ideally we'd have a way to configure options for both the full and
incremental schedule separately, in which case we could reject the
`on_previous_running` configuration for incremental schedules.
Until then this workaround will have to do and we should call out this
known limitation.

Fixes: #96110

Release note (enterprise change): backup schedules created or altered to
have the option `on_previous_running` will have the full backup
schedule created with the user specified option, but will override the
incremental backup schedule to always default to `on_previous_running = wait`.
This ensures correctness of the backup chains created by the incremental
schedule by preventing duplicate incremental jobs from racing against each
other.

98307: ui: fix txn aggregations in txns fingerprints page r=xinhaoz a=xinhaoz

This commit addresses 2 issues on the txns overview page:
1. We were previously grouping txns by txn fingerprint id, agg time, agg interval, and app name. This is from a time when we wanted all these fields, but recently we only want to aggregate on txn fingerprint id.
This commit changes the grouping to only the txn id.

2. Stats aggregation causing undesired data mutations: We were seeing that in the txns fingerprint page,
stats columns would seemingly randomly continue to increase while on the page (e.g. exec count, bytes read). During stats aggregation after grouping by
the fields mentioned above, we were using the first txn in the grouping  as the base object for stats
aggregation, meaning we inherited and mutated the stats object of that txn. Since we aggregate on every re-render, This meant that we were using the result of any previous aggregations as the base for our current aggregation in the re-render. This explains the never-ending incrementing stats. This commit addresses this bug by ensuring we don't re-use the stats object between re-renders by creating a new copy of the stats for every aggregation.

Fixes: #96186
Fixes: #68375

Release note (bug fix): stats columns in txns fingerprint overview page does not continuously increment



BEFORE
https://www.loom.com/share/d9bbd98ced2742dd899031fbc16df6af	

AFTER
https://www.loom.com/share/5407fbbad086404c8d9d63e7f5ef15dd

98321: backupccl: add restore/pause/tpce/80GB/aws/nodes=4/cpus=8 to aws nightlies r=lidorcarmel a=msbutler


Epic: none

Release note: None

Co-authored-by: Aaron Zinger <[email protected]>
Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Mira Radeva <[email protected]>
Co-authored-by: adityamaru <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
@craig craig bot closed this as completed in a59d6b6 Mar 9, 2023
blathers-crl bot pushed a commit that referenced this issue Mar 9, 2023
This commit addresses 2 issues on the txns overview page:
1. We were previously grouping txns by txn fingerprint
id, agg time, agg interval, and app name. This is from
a time when we wanted all these fields, but recently
we only want to aggregate on txn fingerprint id.
This commit changes the grouping to only the txn id.

2. Stats aggregation causing undesired data mutations:
We were seeing that in the txns fingerprint page,
stats columns would seemingly randomly continue to
increase while on the page (e.g. exec count, bytes
read). During stats aggregation after grouping by
the fields mentioned above, we were using the first
txn in the grouping  as the base object for stats
aggregation, meaning we inherited and mutated the
stats object of that txn. Since we aggregate on
every re-render, This meant that we were using the
result of any previous aggregations as the base for our
current aggregation in the re-render. This explains
the never-ending incrementing stats. This commit
addresses this bug by ensuring we don't re-use the
stats object between re-renders by creating a new copy
of the stats for every aggregation.

Fixes: #96186
Fixes: #68375

Release note (bug fix): stats columns in txns fingerprint overview
page does not continuously increment
xinhaoz added a commit to xinhaoz/cockroach that referenced this issue Mar 23, 2023
This commit addresses 2 issues on the txns overview page:
1. We were previously grouping txns by txn fingerprint
id, agg time, agg interval, and app name. This is from
a time when we wanted all these fields, but recently
we only want to aggregate on txn fingerprint id.
This commit changes the grouping to only the txn id.

2. Stats aggregation causing undesired data mutations:
We were seeing that in the txns fingerprint page,
stats columns would seemingly randomly continue to
increase while on the page (e.g. exec count, bytes
read). During stats aggregation after grouping by
the fields mentioned above, we were using the first
txn in the grouping  as the base object for stats
aggregation, meaning we inherited and mutated the
stats object of that txn. Since we aggregate on
every re-render, This meant that we were using the
result of any previous aggregations as the base for our
current aggregation in the re-render. This explains
the never-ending incrementing stats. This commit
addresses this bug by ensuring we don't re-use the
stats object between re-renders by creating a new copy
of the stats for every aggregation.

Fixes: cockroachdb#96186
Fixes: cockroachdb#68375

Release note (bug fix): stats columns in txns fingerprint overview
page does not continuously increment
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

Successfully merging a pull request may close this issue.

7 participants