changefeedccl: remove deprecated poller #38211
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
|
@rolandcrosby Any objection to removing the |
|
@danhhz Sounds good to me. |
tbg
approved these changes
Jun 18, 2019
There was a problem hiding this comment.
Reviewed 13 of 13 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @danhhz)
pkg/ccl/changefeedccl/bench_test.go, line 215 at r1 (raw file):
ctx, cancel := context.WithCancel(ctx) go func() { _ = poller.RunUsingRangefeeds(ctx) }()
Is poller now a misnomer? Not that I'd have a better suggestion.
danhhz
force-pushed
the
cdc_poller
branch
3 times, most recently
from
5b4a6aa to
c890311
Compare
We switched the default to push-based rangefeeds in 19.1. This removes the old pull-based poller fallback entirely. Details of the removal: - The relevant code is removed - Several poller-related hacks are removed - The changefeed.run.push.enabled telemetry metric is removed - The changefeed.push.enabled cluster setting is removed - The poller subtest is removed from each changefeedccl test - The cdc/poller roachtest is skipped on 19.2+ - TestValidations is removed, it's redundant with the much better quality TestChangefeedNemeses Note that the table history still does some polling, but switching this to RangeFeed will cause an unacceptable increase in the commit-to-emit latency of rows. This bit of polling will be removed as part of cockroachdb#36289. This commit also leaves the structure of the changefeed code mostly unchanged. There is an opportunity for cleanup here, but this also will wait for after cockroachdb#36289. Closes cockroachdb#36914 Release note: None
danhhz
commented
Jun 26, 2019
There was a problem hiding this comment.
While I was fixing up the tests, I found some more cleanups in poller (including removing the last usage of EnableTimeBoundIteratorOptimization!)
Reviewable status:
pkg/ccl/changefeedccl/bench_test.go, line 215 at r1 (raw file):
Previously, tbg (Tobias Grieger) wrote…
Is
pollernow a misnomer? Not that I'd have a better suggestion.
Yeah, ditto. Maybe something will come to me by the time I do #36289
tbg
approved these changes
Jun 26, 2019
There was a problem hiding this comment.
Reviewed 10 of 10 files at r2.
Reviewable status:
|
Thanks for the review! bors r=tbg |
Build failed |
|
bors r=tbg |
Build failed |
|
Flake is #38368 bors r=tbg |
craig bot
pushed a commit
that referenced
this pull request
Jun 26, 2019
38211: changefeedccl: remove deprecated poller r=tbg a=danhhz We switched the default to push-based rangefeeds in 19.1. This removes the old pull-based poller fallback entirely. Details of the removal: - The relevant code is removed - Several poller-related hacks are removed - The changefeed.run.push.enabled telemetry metric is removed - The changefeed.push.enabled cluster setting is removed - The poller subtest is removed from each changefeedccl test - The cdc/poller roachtest is skipped on 19.2+ - TestValidations is removed, it's redundant with the much better quality TestChangefeedNemeses Note that the table history still does some polling, but switching this to RangeFeed will cause an unacceptable increase in the commit-to-emit latency of rows. This bit of polling will be removed as part of #36289. This commit also leaves the structure of the changefeed code mostly unchanged. There is an opportunity for cleanup here, but this also will wait for after #36289. Closes #36914 Release note: None 38418: storage: fix null pointer dereference in AdminMerge r=jeffrey-xiao a=jeffrey-xiao Fixes #38427. I noticed that `TestRepartitioning` was failing under stress on master with the following error: ``` panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x20894f6] ... /usr/lib/go-1.12/src/runtime/panic.go:522 +0x1b5 github.com/cockroachdb/cockroach/pkg/storage.(*Replica).AdminMerge.func1(0xc00441e750, 0xc002a83e60, 0xc00020a880) /home/cockroach/go/src/github.com/cockroachdb/cockroach/pkg/storage/replica_command.go:536 +0x226 ... ``` The logic for retrieving the RHS descriptor before and after #38302 is not the same in the case where `rightDesc` does not exist. This PR changes it so the behavior is consistent. It seems possible that the following branch can evaluate true, but I might be missing something here. ```go // Verify that the two ranges are mergeable. if !bytes.Equal(origLeftDesc.EndKey, rightDesc.StartKey) { // Should never happen, but just in case. return errors.Errorf("ranges are not adjacent; %s != %s", origLeftDesc.EndKey, rightDesc.StartKey) } ``` 38435: sql: Fixing interleave check for loose index scan r=rohany a=rohany Fixing the interleave check for the loose index scan while interleave support is in progress. Co-authored-by: Daniel Harrison <[email protected]> Co-authored-by: Jeffrey Xiao <[email protected]> Co-authored-by: Rohan Yadav <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We switched the default to push-based rangefeeds in 19.1. This removes
the old pull-based poller fallback entirely.
Details of the removal:
quality TestChangefeedNemeses
Note that the table history still does some polling, but switching this
to RangeFeed will cause an unacceptable increase in the commit-to-emit
latency of rows. This bit of polling will be removed as part of #36289.
This commit also leaves the structure of the changefeed code mostly
unchanged. There is an opportunity for cleanup here, but this also will
wait for after #36289.
Closes #36914
Release note: None