-
Notifications
You must be signed in to change notification settings - Fork 9
feat: Simulator Add Publisher Support for EndOfStream
Message
#1293
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
base: main
Are you sure you want to change the base?
feat: Simulator Add Publisher Support for EndOfStream
Message
#1293
Conversation
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
Signed-off-by: Mustafa Uzun <mustafa.uzun@limechain.tech>
if (!publishStreamGrpcClient.streamBlock(nextBlock, publishStreamResponseConsumer)) { | ||
// TODO: how we would simulate starting a new stream? Creating a new instance of publishStreamGrpcClient? | ||
// The same for blockStreamManager because we would want to start from before the failed block for example? | ||
publishStreamGrpcClient.shutdown(); | ||
if (publishStreamResponseAtomicReference.get().getEndStream().getStatus().equals(Code.SUCCESS)) { | ||
publishStreamGrpcClient.init(); | ||
continue; | ||
} else if (publishStreamResponseAtomicReference.get().getEndStream().getStatus().equals(Code.TIMEOUT)) { | ||
// TODO: The source MUST start a new stream before the failed block. | ||
|
||
} | ||
// TODO: handle other statuses here | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am commenting on these lines to visualise two examples.
My questions are more general.
- How would we want to simulate ending and starting a new stream?
- We can shutdown current
publishStreamGrpcClient
and create new instance of it - Also in case of SUCCESS we have
The Publisher MAY start a new stream beginning with the next block.
in docs. So we would want to test only stoping the stream and also test stopping and starting new stream on SUCCESS
So we should control that through some config? - Also in case of TIMEOUT we want to start a new stream before the failed block. Here we can shutdown current
publishStreamGrpcClient
and create a new instance of it. We also need to create a new instance of blockstreamManager because we want to change the starting point of block streaming. Currently this to be achieved some refactoring is needed because we are starting fromsimulatorStartupData
As you see this is only for 2 statuses. There is more to cover. So it can get very complicated and messy
@jsync-swirlds @ata-nas @AlfredoG87 What approach do you think we need to take?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe a bit of refactor is needed to make this work correctly.
- Replace the singleton
publishStreamGrpcClient
with a non-singleton owned by a new class (perhaps "publishClientManager").- This new class would also own and manage the
Handler
.
- This new class would also own and manage the
- The handler signals it's client manager when any
EndStream
message is received from the block node. - The client manager calls stop/end/etc... on handler and client, then creates a new client and new handler, passing in the next block to stream (based on the latest block field of the
EndStream
message).
This mirrors what a real publisher would (should?) do.
For the SUCCESS
case, we should have a config in the simulator for first and last block to stream for this run of the simulator (max_long
as last block for "stream forever"). If the last block is less than or equal to the latest block field in the EndStream
message, then the simulator just exits; otherwise it starts a new client for the next block, as above.
We shouldn't try to figure out testing scenarios in the simulator; that's what suites do. The simulator just needs to behave as much like a real publisher as possible while remaining simple and very easy to change or extend. We should offer mechanisms for a suite to make behavior match a particular scenario, but not so much as to make the simulator excessively complex.
EndOfStream
Message
Reviewer Notes
Note
This is WIP
Add support in the block stream simulator, publisher mode, for receiving an EndStream message from the block node and correctly handling that response.
Related Issue(s)
Fixes #1109