-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix(pubsublite): improve handling of backend unavailability #3846
Conversation
- define ErrBackendUnavailable - mitigate stream initialization delays
19bdac1
to
75fdabe
Compare
51f23f8
to
d554104
Compare
pubsublite/internal/wire/settings.go
Outdated
@@ -82,7 +82,7 @@ var DefaultPublishSettings = PublishSettings{ | |||
DelayThreshold: 10 * time.Millisecond, | |||
CountThreshold: 100, | |||
ByteThreshold: 1e6, | |||
Timeout: 60 * time.Minute, | |||
Timeout: 72 * time.Hour, |
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.
This feels a little arbitrary. Is there an InfiniteFuture() equivalent on Go? Same comment on the ReceiveSettings.
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.
There isn't a defined infinite duration. It's an int64, so we can compute the max value, however I wanted to avoid depending on implementation details.
I changed this to 1 week, as it would be concerning if a client wasn't able to connect within that time. The original 3 days was to allow clients to cover a weekend.
ErrBackendUnavailable
to allow users to detect and handle backend unavailability during publish and subscribe.PublishSettings.Timeout
andReceiveSettings.Timeout
to 1 week.PublishSettings.BufferedByteLimit
to 10 GB.