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

Cannot update `ackDeadline` via `setOptions()`

Open WesCossick opened this issue 4 years ago • 9 comments

Environment details

  • OS: Distroless
  • Node.js version: v12
  • npm version: N/A
  • @google-cloud/pubsub version: 2.6.0

Steps to reproduce

  1. Create a subscription by calling:
const subscription = topic.subscription('example-name', {
    ackDeadline: 300,
});

if (!(await subscription.exists())[0]) {
    await subscription.create({
        ackDeadlineSeconds: 300,
    });
}
  1. Later, attempt to update the ackDeadline by calling:
const subscription = topic.subscription('example-name', {
    ackDeadline: 60,
});

subscription.setOptions({
    ackDeadline: 60,
});
  1. See that in the Google Cloud Platform console, the deadline does not change:
Screen Shot 2020-10-22 at 2 17 42 PM

Other notes

The .setMetadata() call works just fine, at least with retryPolicy values.

WesCossick avatar Oct 22 '20 19:10 WesCossick

@WesCossick Thanks for the report. There was some discussion the other day, actually, about how the Node libraries for Pub/Sub sort of confuse the options for local library code, and the options passed on to the service. This might be an example of that. There is some intent to make this clearer, but that's probably going to be a bit. I'll put this on my list to look at, and maybe if there's not a bug there, we can at least make the usage clearer. (And if there is, I can work on a fix.)

feywind avatar Oct 23 '20 21:10 feywind

I agree that this library often intermingles local and cloud configuration settings in a way that can be confusing. In this case, though, I'm pretty sure ackDeadline is an option passed on to the Pub/Sub service. Whatever the option is first set to is what is displayed in the Google Cloud Platform console. Attempts to later change that value never update what's displayed in the console, unless the subscription is deleted and recreated.

WesCossick avatar Oct 26 '20 13:10 WesCossick

@WesCossick setOptions function will set the ackDeadline locally for specific request it doesn't update the actual ackDeadline saved in cloud.

Here is the Issue where user want to specify the ackDeadline for time being.

To update the ackDeadline you don't need to delete and recreate the subscription you can use setMetadata to update it.

await subscription.setMetadata({
    ackDeadlineSeconds: 60
});

laljikanjareeya avatar Oct 27 '20 09:10 laljikanjareeya

@laljikanjareeya Thanks for that. It does sound like the right answer to me, upon second glance.

I do think there's definitely room for improvement in the method names and docs around options vs metadata.

feywind avatar Oct 27 '20 23:10 feywind

@feywind definitely we need to improve the documentation on setOptions I will make PR shortly, setMetadata method is consistent across all the library.

laljikanjareeya avatar Oct 28 '20 09:10 laljikanjareeya

@laljikanjareeya When you say "locally for specific request," what do you mean by that? How does that affect the behavior differently than the cloud setting?

WesCossick avatar Oct 28 '20 17:10 WesCossick

I was trying to explain that if you have created two instance and you are setting ackDeadline using setOption to one of them subscription will not affect to second one.

 const subscription1 = topic.subscription('example-name')
 const subscription2 = topic.subscription('example-name')

subscription1.setOptions({
    ackDeadline: 60,
});

In above example subscription2 will use the ackDeadline that has been set on console.

laljikanjareeya avatar Oct 30 '20 10:10 laljikanjareeya

After a discussion we had today about this, I think I have a better idea what's going on here, and we're going to try to improve it in the future. There's a client-side ack deadline and a separate server-side concept that is called the same thing. The client side one is used for determining how long to modAck, and the client apparently tells the server to change its value as part of that. So it's possible the updated value from setMetadata is simply being overwritten again by the client.

All of which is to say that this is probably more complicated than it looks at first glance, and it's something that's (hopefully) going to get fixed with a future revision of the client library APIs.

feywind avatar Feb 16 '22 21:02 feywind

@kamalaboulhosn I'm probably explaining this poorly :D Feel free to throw in any clarification for that comment.

feywind avatar Feb 16 '22 21:02 feywind

Ping, this is still on the radar.

feywind avatar Apr 03 '23 19:04 feywind

setMetadata is the correct method to use.

kamalaboulhosn avatar Jun 20 '23 18:06 kamalaboulhosn