feat(pubsub): add publisher flow control support #4292
Conversation
…into pubsub-publish-fc
…into pubsub-publish-fc
google-cla
bot
added
the
cla: yes
switch flowController to struct not pointer fix more comments fix more comments
hongalex
force-pushed
the
pubsub-publish-fc
branch
from
c4b5270
to
c3c0141
Compare
pubsub/topic.go
Outdated
| FlowControlSettings: FlowControlSettings{ | ||
| MaxOutstandingMessages: 1000, | ||
| MaxOutstandingBytes: 10 * MaxPublishRequestBytes, | ||
| LimitExceededBehavior: FlowControlBlock, |
There was a problem hiding this comment.
With BufferedByteLimit, what was the behavior by default? Was it to block? Either way, we should probably make sure that we don't introduce blocking due to MaxOutstandingMessages. That means we may either want to set MaxOutstandingMessages effectively to infinity or set the LimitExceededBehavior to Ignore by default.
There was a problem hiding this comment.
The default behavior of BufferedByteLimit is returning an error. Yeah, I think in retrospect, MaxOutstandingMessages should be disabled by default. Right now, I have some logic determining if these values have been set by the user, but that's probably too complicated.
There was a problem hiding this comment.
So should we change this in that case?
There was a problem hiding this comment.
What's the default for other languages? I defaulted to block because I thought that makes more sense, but consistency is probably more important.
There was a problem hiding this comment.
Ok it seems like the default is ignore for Java, and I think I'll stick with this.
pubsub/topic.go
Outdated
| FlowControlSettings: FlowControlSettings{ | ||
| MaxOutstandingMessages: 1000, | ||
| MaxOutstandingBytes: 10 * MaxPublishRequestBytes, | ||
| LimitExceededBehavior: FlowControlBlock, |
There was a problem hiding this comment.
So should we change this in that case?
hongalex
changed the title
This PR adds publisher flow control support, similar to the subscriber flow control. This is different in that the settings are part of
topic.PublishSettings.FlowControlSettings. Similar to the subscriber flow control, these settings are locked-in on the firstPublishcall and subsequent modifications will not have any effect. In theFlowControlSettings.LimitExceededBehavior = FlowControlSignalErrormode, oversized messages will result in an error from callingpublishResult.Get, but oversized messages in other modes (Block/Ignore), will treat the message as the largest possible message size.FlowControlSettings.MaxOutstandingByteshas a small overlap with the existingPublishSettings.BufferedByteLimit, which specifies an upper bound of bytes to keep in memory before throwing an error. This PR addresses this by adding a deprecation notice toBufferedByteLimit, but both will continue to be supported until a major version release. IfMaxOutstandingBytesis passed in, it will overrideBufferedByteLimit.The integration tests are largely based on the Java implementation. One difference between this and the Ruby/Python/Java implementations is that the semaphores enforce the queueing system. Specifically, large acquires will stay at head of line which avoids deadlocking.
Fixes #3210