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.

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

changefeedccl: BenchmarkChangefeedTicks is skipped. #51842

Closed
jordanlewis opened this issue Jul 23, 2020 · 2 comments · Fixed by #99934
Closed

changefeedccl: BenchmarkChangefeedTicks is skipped. #51842

jordanlewis opened this issue Jul 23, 2020 · 2 comments · Fixed by #99934
Labels
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. skipped-test T-cdc

Comments

@jordanlewis
Copy link
Member

jordanlewis commented Jul 23, 2020 •
edited by cockroach-jira-scripts
Loading

As of #38211:

	// In PR #38211, we removed the polling based data watcher in changefeeds in
	// favor of RangeFeed. This benchmark worked by writing a bunch of data at
	// certain timestamps and manipulating clocks at runtime so the polling
	// grabbed a little of it at a time. There's fundamentally no way for this to
	// work with RangeFeed without a rewrite, but it's not being used for anything
	// right now, so the rewrite isn't worth it. We should fix this if we need to
	// start doing changefeed perf work at some point.

Jira issue: CRDB-4004
@jordanlewis jordanlewis added the C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. label Jul 23, 2020
@blathers-crl
Copy link

blathers-crl bot commented Mar 8, 2023

Hi @shermanCRL, please add branch-* labels to identify which branch(es) this release-blocker affects.

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

@shermanCRL shermanCRL added the branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 label Mar 8, 2023
@miretskiy
Copy link
Contributor

@shermanCRL pretty sure it's not a GA blocker given the fact that this so called benchmark never ran, and just a quick look of it makes me question its usefulness.

@miretskiy miretskiy mentioned this issue Mar 29, 2023
Merged
craig bot pushed a commit that referenced this issue Apr 3, 2023
@rafiss @knz
Merge #99934 #100342 #100345 #100484
6a3422b 
99934: changefeedccl: Remove skipped tests that decayed over time r=miretskiy a=miretskiy

Remove
Fixes #32232
Remove TestChangefeedNodeShutdown.  This test has been disabled since 2018; Other tests exist (e.g. `TestChangefeedHandlesDrainingNodes`) that verify restart behavior.

Fixes #51842
Remove BenchmarkChangefeedTicks benchmark.  This benchmark has been skipped since 2019.  Attempts could be made to revive it; however, this benchmark had a lot of code, which accomplished questionable goals. The benchmark itself was unrepresentative (by using dependency injection), too small to be meaningful (1000 rows), and most likely would be too noise and inconclusive.  We have added other micro benchmarks over time; and we conduct large scale testing, including with roachtests.

Release note: None

100342: upgrades: remove migration that waits for schema changes r=rafiss a=rafiss

We can also remove some skipped tests, since they no longer apply.

informs: #96751
Release note: None

100345: upgrades: unskip TestIsAtLeastVersionBuiltin r=rafiss a=rafiss

informs: #96751
Release note: None

100484: server,testutils: add some extra logging for TestStatusEngineStatsJson r=abarganier a=knz

Informs #99261

Release note: None
Epic: None

Co-authored-by: Yevgeniy Miretskiy <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Raphael 'kena' Poss <[email protected]>
@craig craig bot closed this as completed in f42c0f1 Apr 3, 2023
miretskiy pushed a commit to miretskiy/cockroach that referenced this issue Apr 12, 2023
changefeedccl: Remove skipped tests that decayed over time
4a1ec78 
Remove
Fixes cockroachdb#32232
Remove TestChangefeedNodeShutdown.  This test has been disabled since
2018; Other tests exist (e.g. `TestChangefeedHandlesDrainingNodes`) that verify
restart behavior.

Fixes cockroachdb#51842
Remove BenchmarkChangefeedTicks benchmark.  This benchmark has been
skipped since 2019.  Attempts could be made to revive it; however, this
benchmark had a lot of code, which accomplished questionable goals.
The benchmark itself was unrepresentative (by using dependency
injection), too small to be meaningful (1000 rows), and most likely
would be too noise and inconclusive.  We have added other micro
benchmarks over time; and we conduct large scale testing, including
with roachtests.

Release note: None
miretskiy pushed a commit to miretskiy/cockroach that referenced this issue Apr 12, 2023
changefeedccl: Remove skipped tests that decayed over time
553c481 
Remove
Fixes cockroachdb#32232
Remove TestChangefeedNodeShutdown.  This test has been disabled since
2018; Other tests exist (e.g. `TestChangefeedHandlesDrainingNodes`) that verify
restart behavior.

Fixes cockroachdb#51842
Remove BenchmarkChangefeedTicks benchmark.  This benchmark has been
skipped since 2019.  Attempts could be made to revive it; however, this
benchmark had a lot of code, which accomplished questionable goals.
The benchmark itself was unrepresentative (by using dependency
injection), too small to be meaningful (1000 rows), and most likely
would be too noise and inconclusive.  We have added other micro
benchmarks over time; and we conduct large scale testing, including
with roachtests.

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
branch-release-23.1 Used to mark GA and release blockers, technical advisories, and bugs for 23.1 C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. skipped-test T-cdc
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

changefeedccl: Remove skipped tests that decayed over time miretskiy/cockroach
4 participants
@jordanlewis @kenliu @miretskiy @shermanCRL