Skip to content

changefeedccl: remove fixed latency checks from pts benchmark test #154409

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 1, 2025
Merged

changefeedccl: remove fixed latency checks from pts benchmark test #154409

merged 1 commit into from
Oct 1, 2025

Conversation

aerfrei
Copy link
Contributor

@aerfrei aerfrei commented Sep 29, 2025 •
edited
Loading

Removes the fixed threshold checks for the
changefeed_stage_pts_manage_latency and
changefeed_stage_pts_manage_error_latency metrics in the
cdc/multi-table-pts-benchmark roachtests. These thresholds were
too rigid and caused noisy failures.

Issue #154447 has been filed to track the goal of monitoring these metrics
via roachperf instead.

Fixes: #152900
Fixes: #152903
Fixes: #153017
Fixes: #154120
Fixes: #154122
Fixes: #154132
Fixes: #154302
Fixes: #154357
Epic: CRDB-1421
Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@aerfrei aerfrei force-pushed the pts_remove_perf_threshold branch 4 times, most recently from aba7a00 to acfcc5a Compare September 30, 2025 15:03
@aerfrei aerfrei changed the title remove threshold check changefeedccl: remove fixed latency checks from pts benchmark test Sep 30, 2025
@aerfrei
Copy link
Contributor Author

aerfrei commented Sep 30, 2025 •
edited
Loading

I kicked off a roachtest run here

Previous run on master failed with "failed to create IAP token source: failed to get token: impersonate: status code 403" here. Perhaps this was because I ran a stress run and not a nightly.

@aerfrei aerfrei requested a review from KeithCh September 30, 2025 20:45
@aerfrei aerfrei marked this pull request as ready for review September 30, 2025 20:45
KeithCh
KeithCh approved these changes Sep 30, 2025
View reviewed changes
@aerfrei
changefeedccl: remove fixed latency checks from pts benchmark test
598c8d5 
Removes the fixed threshold checks for the
`changefeed_stage_pts_manage_latency` and
`changefeed_stage_pts_manage_error_latency` metrics in the
cdc/multi-table-pts-benchmark roachtests. These thresholds were
too rigid and caused noisy failures.

Issue cockroachdb#154447 has been filed to track the goal of monitoring these metrics
via roachperf instead.

Fixes: cockroachdb#152900
Fixes: cockroachdb#152903
Fixes: cockroachdb#153017
Fixes: cockroachdb#154120
Fixes: cockroachdb#154122
Fixes: cockroachdb#154132
Fixes: cockroachdb#154302
Fixes: cockroachdb#154357
Epic: CRDB-1421
Release note: None
@aerfrei aerfrei force-pushed the pts_remove_perf_threshold branch from acfcc5a to 598c8d5 Compare September 30, 2025 21:06
@aerfrei aerfrei requested a review from asg0451 October 1, 2025 13:28
@aerfrei
Copy link
Contributor Author

aerfrei commented Oct 1, 2025

bors r=asg0451,KeithCh

@andyyang890 andyyang890 added backport-25.4.x Flags PRs that need to be backported to 25.4 and removed backport-25.4.x Flags PRs that need to be backported to 25.4 labels Oct 1, 2025
@andyyang890
Copy link
Collaborator

Will we need to backport this to 25.4? I was originally thinking yes but maybe not after #154478 merges.

@craig
Copy link
Contributor

craig bot commented Oct 1, 2025

Build succeeded:

@craig craig bot merged commit 1c18131 into cockroachdb:master Oct 1, 2025
23 checks passed
@aerfrei
Copy link
Contributor Author

aerfrei commented Oct 1, 2025

Will we need to backport this to 25.4? I was originally thinking yes but maybe not after #154478 merges.

Yes, I think we will since this test was failing occasionally but less frequently when the flag was off.

@aerfrei
Copy link
Contributor Author

aerfrei commented Oct 1, 2025

blathers backport 25.4

This was referenced Oct 1, 2025
Closed
Closed
Closed
Closed
@blathers-crl blathers[crl]
Copy link

blathers-crl bot commented Oct 1, 2025

Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches.

Issue #154357: branch-release-25.4.

Issue #152900: branch-release-25.4.

Issue #152903: branch-release-25.4.

Issue #153017: branch-release-25.4.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@aerfrei aerfrei mentioned this pull request Oct 1, 2025
Merged
@aerfrei aerfrei added the backport-25.4.x Flags PRs that need to be backported to 25.4 label Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asg0451 asg0451 asg0451 approved these changes

@KeithCh KeithCh KeithCh approved these changes

Assignees

No one assigned

Labels

backport-25.4.x Flags PRs that need to be backported to 25.4 target-release-26.1.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

roachtest: cdc/multi-table-pts-benchmark/per-table-pts=true/num-tables=50000/num-ranges=1 failed roachtest: cdc/multi-table-pts-benchmark/per-table-pts=false/num-tables=50000/num-ranges=1 failed roachtest: cdc/multi-table-pts-benchmark/per-table-pts=true/num-tables=5000/num-ranges=10 failed roachtest: cdc/multi-table-pts-benchmark/per-table-pts=false/num-tables=50000/num-ranges=1 failed roachtest: cdc/multi-table-pts-benchmark/per-table-pts=true/num-tables=50000/num-ranges=1 failed
roachtest: cdc/multi-table-pts-benchmark/per-table-pts=true/num-tables=5000/num-ranges=10 failed roachtest: cdc/multi-table-pts-benchmark/per-table-pts=true/num-tables=500/num-ranges=10 failed roachtest: cdc/multi-table-pts-benchmark/per-table-pts=true/num-tables=500/num-ranges=10 failed

5 participants

@aerfrei @cockroach-teamcity @andyyang890 @asg0451 @KeithCh