From c79e0ce06f1d2f0c6bddacd8782701dfaf19641d Mon Sep 17 00:00:00 2001 From: Raja Subramanian Date: Tue, 16 May 2023 14:08:17 +0530 Subject: [PATCH] Make signal close async. (#1711) * Make signal close async. Left notes about async close in code. Also reducing retry config timeout - Timeout to 7.5 seconds (making it 1/4th of current config) - max retry to 4 seconds - so, it can do 4 tries now in 7.5 seconds (with retries ending at 0.5 seconds, 1.5 seconds, 3.5 seconds, 7.5 seconds). The change of max to 4 seconds is not really needed, but it lined up with 7.5. So, made the change. * update comments a bit --- pkg/config/config.go | 4 ++-- pkg/routing/signal.go | 17 ++++++++++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index a9f61aad4..876e74773 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -375,9 +375,9 @@ func NewConfig(confString string, strictMode bool, c *cli.Context, baseFlags []c }, SignalRelay: SignalRelayConfig{ Enabled: false, - RetryTimeout: 30 * time.Second, + RetryTimeout: 7500 * time.Millisecond, MinRetryInterval: 500 * time.Millisecond, - MaxRetryInterval: 5 * time.Second, + MaxRetryInterval: 4 * time.Second, StreamBufferSize: 1000, }, Keys: map[string]string{}, diff --git a/pkg/routing/signal.go b/pkg/routing/signal.go index 56aa25005..af8392e48 100644 --- a/pkg/routing/signal.go +++ b/pkg/routing/signal.go @@ -255,7 +255,22 @@ func (s *signalMessageSink[SendType, RecvType]) Close() { } s.mu.Unlock() - <-s.Stream.Context().Done() + // NOTE: not waiting for stream context to be done. + // Waiting for stream context to be done is confirmation + // that the close message has been processed by the other side. + // In ideal conditions, waiting for it is a clean end. + // + // But, in cases where the remote side goes away abruptly, waiting + // for stream context to be done could block connection progress + // till the timeout hits. + // + // The abrupt case happens when one side of the signal + // relay is shut down due to scale down or a crash. + // + // Uncomment the following line to wait for close acknowledgement, + // but the system should be able to wait long enough (till timeout) + // without adverse impact if waiting for close acknowledgement. + //<-s.Stream.Context().Done() } func (s *signalMessageSink[SendType, RecvType]) IsClosed() bool {