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

Peer removal API #894

Merged
merged 2 commits into from
Aug 2, 2022
Merged

Peer removal API #894

merged 2 commits into from
Aug 2, 2022

Conversation

e-ivkov
Copy link
Contributor

@e-ivkov e-ivkov commented Aug 2, 2022

Relates to #874

Design Decisions

Do not shutdown peer if it receives its own removal from consensus. Peer should be later shutdown by script/admin. Otherwise it could interfere with Kubernetes cluster management (it can try to restart the peer).

@e-ivkov e-ivkov requested review from generall and agourlay August 2, 2022 10:49
@e-ivkov e-ivkov changed the title Remove peer api Peer removal API Aug 2, 2022
@e-ivkov e-ivkov merged commit 0a63445 into master Aug 2, 2022
@@ -323,6 +324,15 @@ impl<C: CollectionContainer> ConsensusState<C> {
self.persistent.write().insert_peer(peer_id, uri)
}

pub fn remove_peer(&self, peer_id: PeerId) -> Result<(), StorageError> {
if self.toc.peer_has_shards(peer_id) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we check if the peer is currently receiving a shard via a transfer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good point. I guess I will add it in the next PR then 😄

Also because of this discussion I noticed that we capture proxy shards in collection snapshot, which is interesting. I will need to recheck the logic here.

@agourlay agourlay mentioned this pull request Aug 5, 2022
2 tasks
@e-ivkov e-ivkov deleted the remove-peer-api branch August 29, 2022 09:08
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