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

Flush messages with nack on subscriber close #1860

Closed
brent-statsig opened this issue Dec 6, 2023 · 2 comments
Closed

Flush messages with nack on subscriber close #1860

brent-statsig opened this issue Dec 6, 2023 · 2 comments
Assignees
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@brent-statsig
Copy link

brent-statsig commented Dec 6, 2023

Hey!

Problem

When closing the connection, there is no easy way to nack all messages remaining in the queue. This leads to our P99 delivery time of messages to inflate. It is fairly common for a pod to be pre-empted, which means there is a fairly short window to close
down processing, and it is not realistic to wait until all processing is complete.

In the event of a connection closing, it is extremely difficult to ensure that all messages are nack without building my own version of a lease manager to track messages.

I noticed in LeaseManager.close, it wipes the local state. Can this just be updated to nack everything, and then wipe?

/**
   * Removes ALL messages from inventory.
   * @private
   */
  clear(): void {
    const wasFull = this.isFull();

    this._pending = [];
    this._messages.clear();
    this.bytes = 0;

    if (wasFull) {
      process.nextTick(() => this.emit('free'));
    }

    this._cancelExtension();
  }
@brent-statsig brent-statsig added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Dec 6, 2023
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Dec 6, 2023
@feywind feywind self-assigned this Dec 18, 2023
@feywind
Copy link
Collaborator

feywind commented Dec 18, 2023

@brent-statsig Yeah, this is something we're planning to address. Right now there isn't really a consistent way to deal with queued ack/modack/nack messages on close. There are a few other issues here asking for similar things, and I need to go back and link all of those at some point.

@feywind
Copy link
Collaborator

feywind commented May 2, 2024

Closed in favour of #1917

@feywind feywind closed this as completed May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants