nodejs-pubsub
nodejs-pubsub copied to clipboard
Lease management settings should be consolidated in one place
Currently, controlling min/max ack deadline is done in SubscriberOptions, but configuring the total extension (for a message's ack deadline) is in FlowControlOptions.
These are related settings and should be in the same place.
Separately, flow control options being in a the lease-manager.ts file seems strange, since lease management should only concern itself with modacks rather than flow/concurrency control.
As discussed elsewhere, the settings can't be changed until a breaking major. The lease manager changes are planned for an internals cleanup effort.
Kinda looks like the lease manager separation is going to help with this issue, too:
https://github.com/googleapis/nodejs-pubsub/issues/1213
So I might do that sooner rather than later.
@feywind I see so many issues around maxMessages. Is it working now or buggy? If it works can you send a working sample please? On my end it never respects the maxMessages parameter.
@mete89 What is the behaviour you're seeing? It should be functioning. e.g. if you set maxMessages to 1, it should only deliver one message at a time, pausing the stream before any further deliveries. This doc comment may hold some answers:
* @property {boolean} [allowExcessMessages=true] PubSub delivers messages in
* batches with no way to configure the batch size. Sometimes this can be
* overwhelming if you only want to process a few messages at a time.
* Setting this option to false will make the client manage any excess
* messages until you're ready for them. This will prevent them from being
* redelivered and make the maxMessages option behave more predictably.
Setting that to false ends up activating the client library flow control, so if the Pub/Sub server hands you more than maxMessages, the library will buffer them. (At the cost of extra memory on your client.)
Edit: Also, you're right, we have no samples around this... I'll put in a bug for it. The complicating factor there is that we'd probably need to add it for all languages.
Unrelated, the original issue topic is closed out by: https://github.com/googleapis/nodejs-pubsub/pull/2024
I'll leave this open for discussing the other thing.