flowControl.maxMessages is being ignored

[sjb933]

flowControl.maxMessages is seemingly being ignored. I've created a small focused test repo to highlight the issue.

Environment details

• OS: Mac OSX
• Node.js version: 8.16.2
• npm version: 6.4.1
• @google-cloud/pubsub version: 1.1.2

Steps to reproduce

See: https://github.com/sjb933/gcloud-pubsub-maxMessage-bug

1. Clone project
2. npm install
3. Open test.ts and populate GOOGLE_PROJECT_ID with a proper project ID which you have authenticated access to
4. Execute the test with npm start
5. Observe the output

Observations:

Expected: 10 messages published, only 2 process concurrently.

Actual: 10 messages published, all 10 process concurrently.

[sjb933]

The output produced from my test script:

> [email protected] start /Users/shane/Code/pubsub-bug
> ts-node test.ts

Concurrency within tolerances
Concurrency within tolerances
ERROR: Concurrently processing 10 messages (flowControl.maxMessages = 2)
ERROR: Concurrently processing 10 messages (flowControl.maxMessages = 2)
Concurrency within tolerances
Deleting test topic: test-topic-029095cb

The messages in my script are ACK'd on a timeout and there should only ever be 2 running concurrently.

[callmehiphop]

@sjb933 the backend doesn't actually let us configure how many messages it sends per batch, so unless the allowExcessMessages flag is set to false you'll get more. Can you try out the following config?

subscription.setOptions({
  flowControl: {
    allowExcessMessages: false,
    maxMessages: 2
  }
});

[sjb933]

@callmehiphop confirmed, allowExcessMessages: false resolves the issue. I'm in the process of updating to newer libs and this was revealed in our consuming code's tests.

Was this a new option, or did it perhaps default to false before?

Thanks you for the quick support!

[callmehiphop]

I think it shipped with v0.23.0 which was released back in January. Before that we had behavior that mirrored setting it to false.

[mnahkies]

@sjb933 the backend doesn't actually let us configure how many messages it sends per batch, so unless the allowExcessMessages flag is set to false you'll get more. Can you try out the following config?

@callmehiphop seems like this has recently changed (#1041) - does this mean we no longer require the allowExcessMessages: false option?

[lucasjohnston]

Can confirm that you still need allowExcessMessages: false to use maxMessages.
The example for Lease Management in the docs should probably reflect this :)

https://cloud.google.com/pubsub/docs/pull#synchronous_pull_with_lease_management