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

rangefeed: remove non-mux rangefeed code entirely #125666

Closed
wenyihu6 opened this issue Jun 13, 2024 · 1 comment · Fixed by #125610
Closed

rangefeed: remove non-mux rangefeed code entirely #125666

wenyihu6 opened this issue Jun 13, 2024 · 1 comment · Fixed by #125610
Assignees
Labels
A-kv-rangefeed Rangefeed infrastructure, server+client

Comments

@wenyihu6
Copy link
Contributor

wenyihu6 commented Jun 13, 2024

Describe the problem

In 24.1, to simplify the implementation of #110432, we removed any cluster
setting for enabling non-mux rangefeeds, making them always enabled. But we
retained the non-mux rangefeed code there for test coverage in mixed-version
clusters. Given that we are going to 24.2, we are now more comfortable with
removing the tests and deprecating non-mux rangefeed code entirely.

Jira issue: CRDB-39543

@wenyihu6 wenyihu6 added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jun 13, 2024
Copy link

blathers-crl bot commented Jun 13, 2024

Hi @wenyihu6, please add branch-* labels to identify which branch(es) this C-bug affects.

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

@wenyihu6 wenyihu6 self-assigned this Jun 13, 2024
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
Previously, we introduced a stuck rangefeed canceler to automatically restart
single rangefeeds if they haven’t received KV updates for some time. This patch
removes that feature as it was only available in non-mux rangefeed (which will
soonly be disabled). Given that we have seen few reports of stuck rangefeeds
now, we are more comfortable with not adding this feature to mux rangefeeds.

Part of: cockroachdb#125666
Release note: the cluster setting kv.rangefeed.range_stuck_threshold has been
removed.
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
This patch adapts the test TestInternalClientAdapterRunsInterceptors to mux
rangefeed so that we don’t lose test coverage when removing non-mux rangefeed
code in the future commits.

Part of: cockroachdb#125666
Release note: none
@wenyihu6 wenyihu6 added A-kv-rangefeed Rangefeed infrastructure, server+client and removed C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. labels Jun 14, 2024
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
This patch removes tests that use non-mux rangefeed code (which will soonly be
removed). Note that all non-mux rangefeed tests have corresponding mux rangefeed
tests, so we are not losing test coverage here.

Part of: cockroachdb#125666
Release note: none
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
This patch adapts the test TestInternalClientAdapterRunsInterceptors to mux
rangefeed so that we don’t lose test coverage when removing non-mux rangefeed
code in the future commits.

Part of: cockroachdb#125666
Release note: none
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
This patch removes tests that use non-mux rangefeed code (which will soonly be
removed). Note that all non-mux rangefeed tests have corresponding mux rangefeed
tests, so we are not losing test coverage here.

Part of: cockroachdb#125666
Release note: none
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
In 22.2, mux rangefeeds were introduced to reduce the number of rpc streams from
one per replica to one per client for all rangefeeds on a node.

In 24.1, to simplify the implementation of cockroachdb#110432, we removed any cluster
setting for enabling non-mux rangefeeds, making them always enabled. But we
retained the non-mux rangefeed code there for test coverage in mixed-version
clusters. Given that we are going to 24.2, we are now more comfortable with removing
the tests. This patch completely removes client and server code for non-mux
rangefeed.

Resolves: cockroachdb#125666
Release note: none
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
This patch removes tests that use non-mux rangefeed code (which will soonly be
removed). Note that all non-mux rangefeed tests have corresponding mux rangefeed
tests, so we are not losing test coverage here.

Part of: cockroachdb#125666
Release note: none
craig bot pushed a commit that referenced this issue Jun 14, 2024
125663: rangefeed: remove stuck rangefeed canceler r=nvanbenschoten a=wenyihu6

Previously, we introduced a stuck rangefeed canceler to automatically restart
single rangefeeds if they haven’t received KV updates for some time. This patch
removes that feature as it was only available in non-mux rangefeed (which will
soonly be removed). Given that we have seen few reports of stuck rangefeeds
now, we are comfortable with not adding this feature to mux rangefeeds.

Part of: #125666
Release note: the cluster setting kv.rangefeed.range_stuck_threshold has been
removed.

Co-authored-by: Wenyi Hu <[email protected]>
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
This patch adapts the test TestInternalClientAdapterRunsInterceptors to mux
rangefeed so that we don’t lose test coverage when removing non-mux rangefeed
code in the future commits.

Part of: cockroachdb#125666
Release note: none
wenyihu6 added a commit to wenyihu6/cockroach that referenced this issue Jun 14, 2024
This patch removes tests that use non-mux rangefeed code (which will soonly be
removed). Note that all non-mux rangefeed tests have corresponding mux rangefeed
tests, so we are not losing test coverage here.

Part of: cockroachdb#125666
Release note: none
craig bot pushed a commit that referenced this issue Jun 14, 2024
125665: rangefeed: remove tests for non-mux rangefeed r=nvanbenschoten a=wenyihu6

**rpc: extend TestInternalClientAdapterRunsInterceptors to mux rangefeed**

This patch adapts the test TestInternalClientAdapterRunsInterceptors to mux
rangefeed so that we don’t lose test coverage when removing non-mux rangefeed
code in the future commits.

Part of: #125666
Release note: none

---

**rangefeed: remove tests for non-mux rangefeed**

This patch removes tests that use non-mux rangefeed code (which will soonly be
removed). Note that all non-mux rangefeed tests have corresponding mux rangefeed
tests, so we are not losing test coverage here.

Part of: #125666
Release note: none

Co-authored-by: Wenyi Hu <[email protected]>
craig bot pushed a commit that referenced this issue Jun 18, 2024
125610: rangefeed: remove non-muxrangefeed code r=nvanbenschoten a=wenyihu6

In 22.2, mux rangefeeds were introduced to reduce the number of rpc streams from
one per replica to one per client for all rangefeeds on a node.

In 24.1, to simplify the implementation of #110432, we removed any cluster
setting for enabling non-mux rangefeeds, making them always enabled. But we
retained the non-mux rangefeed code there for test coverage in mixed-version
clusters. Given that we are going to 24.2, we are now more comfortable with removing
the tests. This patch completely removes client and server code for non-mux
rangefeed.

Resolves: #125666
Release note: none

125694: compose: TestComposeCompare improvements r=rafiss a=rafiss

This PR accomplishes:
- properly link libgeos
- preserve logs on failure
- run tests as current user 

fixes #125286
fixes #125027
fixes #125392
fixes #124379

Release note: None

125847: acceptance/compose: add docker health check r=rafiss a=rail

Previously, docker compose based GSS tests sometimes failed, because the cockroach container took some time to bring the server up. This doesn't happen with `--insecure` tests, such as `flyway`.

This PR adds a health check in order to ensure the cockroach container is healthy before running any tests.

Fixes: #125003
Fixes: #125089
Fixes: #125413
Release note: None

Co-authored-by: Wenyi Hu <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
@craig craig bot closed this as completed in 294e45d Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-rangefeed Rangefeed infrastructure, server+client
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant