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

kvserver: move checkForcedErr to kvserverbase #91734

Merged
merged 3 commits into from
Nov 11, 2022

Conversation

tbg
Copy link
Member

@tbg tbg commented Nov 11, 2022

This is a prereq to making log application stand-alone, since applying
entries means calling this method to determine whether to apply an
empty entry instead.

This necessitated moving the constructor for NotLeaseholderError where it belongs (roachpb) and
moving proposal evaluation reasons to somewhere else (I chose kvserverpb). While I was there I
also renamed their type to better reflect that these are command rejections.

Touches #75729.

Epic: CRDB-220
Release note: None

This is called from below-raft, so if we hope to apply log entries
offline in a contained context, we want to be able to call this without
pulling in the `kvserver` package.

Epic: CRDB-220
Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

These are referenced from checkForcedErr, and if we are to
apply proposals via a smaller library we can't have them
live in `kvserver` due to import conflicts.

The move is mechanical. I also threw in a mechanical rename
since the previous names weren't very descriptive.

Release note: None
@tbg tbg marked this pull request as ready for review November 11, 2022 15:57
@tbg tbg requested a review from a team as a code owner November 11, 2022 15:57
@tbg tbg requested a review from pav-kv November 11, 2022 15:57
pkg/roachpb/errors.go Show resolved Hide resolved
pkg/kv/kvserver/kvserverbase/forced_error.go Show resolved Hide resolved
pkg/kv/kvserver/kvserverbase/forced_error.go Outdated Show resolved Hide resolved
@pav-kv
Copy link
Collaborator

pav-kv commented Nov 11, 2022

The PR name and message seems to have picked up only the last commit. Try to write overviews?

This is a prereq to making log application stand-alone, since applying
entries means calling this method to determine whether to apply an
empty entry instead.

Touches cockroachdb#75729.

Epic: CRDB-220
Release note: None
@tbg
Copy link
Member Author

tbg commented Nov 11, 2022

Updated PR message as well.

@tbg
Copy link
Member Author

tbg commented Nov 11, 2022

bors r=pavelkalinnikov

TFTR!

@craig
Copy link
Contributor

craig bot commented Nov 11, 2022

Build succeeded:

@craig craig bot merged commit 5c76966 into cockroachdb:master Nov 11, 2022
@tbg tbg deleted the extract-forced-err branch November 12, 2022 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants