dashbitco / broadway_cloud_pub_sub

A Broadway producer for Google Cloud Pub/Sub
Apache License 2.0
70 stars 24 forks source link

Reduces maximum number of ack ids per ack request #49

Closed monkybrain closed 4 years ago

monkybrain commented 4 years ago

We found that processing large batches would result in acknowledgement requests failing due to exceeded payload size (Request payload size exceeds the limit: 524288 bytes).

The problem seems to be the assumption that ack ids have a maximum size of 164 bytes. Perhaps Google changed this recently because we found that our ack ids were 184 bytes.

Our solution was simply to update the formula for calculating a safe limit and reduce the constant max_ack_ids_per_request to 2500. This seems to have solved the problem.

rockneurotiko commented 4 years ago

You need to change the test that checks that the maximum ackIds is being used 😄

monkybrain commented 4 years ago

@rockneurotiko thanks! 🤦Sorted!

m0ar commented 4 years ago

Nice, quick review! Thanks for a great lib guys :tada:

m0ar commented 3 years ago

@mcrumm Would you mind creating a release for this change? We still use a fork with the fix since it's not updated on hex.pm :pray: :revolving_hearts:

mcrumm commented 3 years ago

Hi @m0ar, thanks for the ping - done! :)

m0ar commented 3 years ago

@mcrumm :cupcake: