Skip to content

loqrecovery,admin: endpoints for loss of quorum recovery#91849

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
aliher1911:loq_recovery_half_online
Dec 6, 2022
Merged

loqrecovery,admin: endpoints for loss of quorum recovery#91849
craig[bot] merged 1 commit intocockroachdb:masterfrom
aliher1911:loq_recovery_half_online

Conversation

@aliher1911
Copy link
Copy Markdown
Contributor

@aliher1911 aliher1911 commented Nov 14, 2022

This commit adds necessary protobufs for half online loss of quorum recovery tooling.

Release note: None

Fixes #93037

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@aliher1911 aliher1911 self-assigned this Nov 14, 2022
@aliher1911 aliher1911 force-pushed the loq_recovery_half_online branch 3 times, most recently from 305cdc7 to 7572a43 Compare November 21, 2022 10:58
@aliher1911 aliher1911 force-pushed the loq_recovery_half_online branch 2 times, most recently from 1f66ed4 to b6f028b Compare December 4, 2022 17:52
@aliher1911 aliher1911 changed the title admin: endpoints for loss of quorum recovery loqrecovery,admin: endpoints for loss of quorum recovery Dec 5, 2022
@aliher1911 aliher1911 force-pushed the loq_recovery_half_online branch 4 times, most recently from 596e6e8 to 716c261 Compare December 6, 2022 10:10
@aliher1911 aliher1911 marked this pull request as ready for review December 6, 2022 10:13
@aliher1911 aliher1911 requested review from a team as code owners December 6, 2022 10:13
@aliher1911 aliher1911 requested review from a team and erikgrinaker December 6, 2022 10:13
Copy link
Copy Markdown
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

request *serverpb.RecoveryCollectReplicaInfoRequest,
server serverpb.Admin_RecoveryCollectReplicaInfoServer,
) error {
panic("To be implemented by #93040")
Copy link
Copy Markdown
Contributor

@erikgrinaker erikgrinaker Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return errors.AssertionFailedf, otherwise this is a trivial DoS vector that's accessible via the network.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, maybe we have a panic handler that would catch it. Either way, AssertionFailedf is more appropriate when we have an error return path.

This commit adds necessary protobufs and Admin rpc's for half online
loss of quorum recovery tooling.

Release note: None
@aliher1911 aliher1911 force-pushed the loq_recovery_half_online branch from 716c261 to 4f71924 Compare December 6, 2022 10:50
@aliher1911
Copy link
Copy Markdown
Contributor Author

TFTR
bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 6, 2022

Build succeeded:

@craig craig bot merged commit 184808f into cockroachdb:master Dec 6, 2022
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.

loqrecovery: Admin service rpc definitions

3 participants