[ReplicatedLoglet] Implement remote sequencer find tail#2017
[ReplicatedLoglet] Implement remote sequencer find tail#2017muhamadazmy wants to merge 2 commits intorestatedev:mainfrom
Conversation
muhamadazmy
force-pushed
the
pr2017
branch
8 times, most recently
from
1c84db6 to
148b191
Compare
Summary: Implements a remote loglet append calls to leader sequencer
tillrohrmann
left a comment
tillrohrmann
left a comment
There was a problem hiding this comment.
Thanks for creating this PR @muhamadazmy. What happens if we lose the GetSequencerInfo or its response?
| .call( | ||
| &self.networking, | ||
| self.params.sequencer, | ||
| GetSequencerInfo { | ||
| header: CommonRequestHeader { | ||
| log_id: self.log_id, | ||
| loglet_id: self.params.loglet_id, | ||
| segment_index: self.segment_index, | ||
| }, | ||
| }, | ||
| ) | ||
| .await |
There was a problem hiding this comment.
What happens if the request or response get lost? Would sync_sequencer_tail get stuck?
There was a problem hiding this comment.
Yeah, that's definitely something that need to be handled. I have paused working on this draft PR in favour of #2019 and wasn't sure if I should continue on this one.
| ); | ||
| } | ||
| _ => { | ||
| unreachable!() |
There was a problem hiding this comment.
Maybe state why this case is unreachable. Maybe also don't use the wildcard. That way we'll see the places where a newly SequencerStatus variant needs to be handled.
There was a problem hiding this comment.
Good point. Thank you :)
| async fn sync_log_servers_tail(&self) -> Result<(), OperationError> { | ||
| todo!() | ||
| } |
| if self.sync_sequencer_tail().await.is_ok() { | ||
| return Ok(*self.known_global_tail.get()); | ||
| } | ||
|
|
||
| // otherwise we need to try to fetch this from the log servers. | ||
| self.sync_log_servers_tail().await?; | ||
| Ok(*self.known_global_tail.get()) |
There was a problem hiding this comment.
Is there value in racing these two variants?
There was a problem hiding this comment.
I am not sure I get your question here.
There was a problem hiding this comment.
Whether we should run both variants for obtaining the known global tail in parallel/concurrently?
There was a problem hiding this comment.
Thank you for clarification. Yeah, that's definitely a good idea.
2 participants
[ReplicatedLoglet] Implement remote sequencer find tail
Stack created with Sapling. Best reviewed with ReviewStack.