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
Enforce uniqueness of replication stream #4448
Conversation
f257661
to
3d0eec3
Compare
@@ -5,7 +5,7 @@ linters: | |||
- goerr113 | |||
- errcheck | |||
- goimports | |||
- paralleltest | |||
# - paralleltest # missing the call to method parallel, but testify does not seem to work well with parallel test: https://github.com/stretchr/testify/issues/187 |
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.
@MichaelSnowden i am disabling the paralleltest linter since this will not work with testify, see above comments
* Unify stream implementations
h.streamReceiverMonitor.RegisterInboundStream(streamSender) | ||
streamSender.Start() | ||
defer streamSender.Stop() | ||
streamSender.Wait() |
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.
Will it blocked and timed out?
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.
the caller will use a background context
when the context is cancelled, the stream sender will be marked as "stopped" and this wait invocation will unblock
|
||
Stream BiDirectionStream[*adminservice.StreamWorkflowReplicationMessagesRequest, *adminservice.StreamWorkflowReplicationMessagesResponse] | ||
StreamReceiver struct { | ||
StreamReceiverImpl struct { |
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.
nit: streamReceiverImpl?
* Enforce uniqueness of replication stream within stream monitor for both sender side & receiver side * Consolidate replication stream implementations; move logic to replication package
* Enforce uniqueness of replication stream within stream monitor for both sender side & receiver side * Consolidate replication stream implementations; move logic to replication package
What changed?
Why?
Better stream lifecycle control
How did you test it?
UT
Potential risks
N/A
Is hotfix candidate?
N/A