Skip to content
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

test ackDeadline #3394

Closed
wants to merge 1 commit into from
Closed

Conversation

dmitry-s
Copy link

This test reproduces an issue with ackDeadline. It publishes 2 messages, then it pools them and nacks one of them. The other one is not being nacked or acked. Then it tries to pull both messages again, in a loop.

The ackDeadline is set to 10 seconds, so I would expect to get both messages after that time. But the message that wasn't nacked or acked never makes it back.

What is interesting, if I enable debugging and put a break point inside the loop, I am able to get both messages.

I verified that ackDeadline works as expected in gcloud cli tool.

@dmitry-s dmitry-s requested a review from pongad as a code owner June 19, 2018 15:17
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 19, 2018
@pongad
Copy link
Contributor

pongad commented Jun 19, 2018

I think this is mostly expected. The client library will automatically extend (modack) the messages that hasn't been acked or nacked. So not getting the message again is more like a sign that automatic deadline extension is working. By default, the client will auto-extend messages for an hour. You can change this with Subscriber.Builder.setMaxAckExtensionPeriod.

@meltsufin
Copy link
Member

@pongad What was the reasoning behind introducing such a long default ack extension? It doesn't even seem to be documented anywhere, whereas it radically changes the expectations of the users of how the ack deadline works.

@meltsufin
Copy link
Member

@garrettjonesgoogle Friendly ping.

@garrettjonesgoogle
Copy link
Member

@pongad should take a look at your question first I think

@pongad
Copy link
Contributor

pongad commented Jul 20, 2018

Sorry I missed this earlier. I believe this was recommended by Pubsub team a long time ago. @kir-titievsky do you happen to know? I'll make a PR to document this.

@meltsufin @dmitry-s Could you tell us what kind of problem this is causing you? I believe you're the first people who want to see duplicates.

@kir-titievsky
Copy link

@pongad sorry, don't remember any more. I'm assuming the idea was to reduce duplicate deliveries.

@meltsufin
Copy link
Member

@pongad The main issue with this behavior is the element of surprise to the users who don't expect the ack deadline to be auto-extended for an hour by default. In the least, this needs to be documented.

@pongad
Copy link
Contributor

pongad commented Sep 4, 2018

The behavior is now documented. I'll close this for now, but please let us know if there's still confusion.

@pongad pongad closed this Sep 4, 2018
@meltsufin
Copy link
Member

@pongad Link(s)?

@pongad
Copy link
Contributor

pongad commented Sep 10, 2018

It should be in the client doc. Search for "ack management".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants