nodejs-pubsub icon indicating copy to clipboard operation
nodejs-pubsub copied to clipboard

Lease management settings should be consolidated in one place

Open hongalex opened this issue 1 year ago • 2 comments

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.

hongalex avatar Apr 26 '24 01:04 hongalex

As discussed elsewhere, the settings can't be changed until a breaking major. The lease manager changes are planned for an internals cleanup effort.

feywind avatar Apr 29 '24 17:04 feywind

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 avatar May 02 '24 21:05 feywind

@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 avatar Apr 21 '25 10:04 mete89

@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.

feywind avatar Apr 30 '25 16:04 feywind

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.

feywind avatar Apr 30 '25 16:04 feywind