Skip to content

pubsub: reject expired and duplicate messages#3743

Merged
pongad merged 4 commits intogoogleapis:masterfrom
pongad:pubsub-old-msg
Oct 18, 2018
Merged

pubsub: reject expired and duplicate messages#3743
pongad merged 4 commits intogoogleapis:masterfrom
pongad:pubsub-old-msg

Conversation

@pongad
Copy link
Contributor

@pongad pongad commented Sep 27, 2018

CC @csainty

This should let us clear through backlogs more quickly.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 27, 2018
csainty
csainty reviewed Sep 27, 2018
View reviewed changes
...-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java
if (ackHandler.totalExpiration.isBefore(now())) {
// Message expired while waiting. We don't extend these messages anymore,
// so it was probably sent to someone else. Don't work on it.
// Don't nack it either, because we'd be nacking someone else's message.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

csainty
csainty reviewed Sep 27, 2018
View reviewed changes
...-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java

// forget removes from pendingMessages; this is OK, concurrent maps can
// handle concurrent iterations and modifications.
entry.getValue().forget();

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@csainty
Copy link

csainty commented Sep 27, 2018

Generally like it, nice clean approach 👍

Won't handle a 0 extension period though will it? Since totalExpiration will be set to receive time

final Subscriber subscriber = Subscriber.newBuilder(subscriptionName, (msg, ack) -> {
    // do something
  })
  .setMaxAckExtensionPeriod(Duration.ZERO)
  .build();

@pongad
Copy link
Contributor Author

pongad commented Sep 27, 2018

That's right. This won't handle the case where max ack extension is zero. I'm a little concerned by this; this essentially changes the behavior from "we won't extend these" to "we'll never work on these". I'll make a small fix for this.

@csainty
Copy link

csainty commented Sep 28, 2018

👍
I pulled this morning and ran my sample project with the build, hits exactly-once consumption. Thanks for looking in to it.

I think this will fix #3383 and #2465
Also make it easier to configure for https://github.com/GoogleCloudPlatform/google-cloud-java/issues/3152 by tweaking the max deadline so all the messages don't pull to a single subscriber and stay there, as currently reducing the max deadline to prevent this gets you in to double processing sadness.

@csainty csainty mentioned this pull request Sep 28, 2018
Closed
@pongad pongad mentioned this pull request Sep 28, 2018
Closed
@pongad
Copy link
Contributor Author

pongad commented Sep 28, 2018

Don't merge this yet. Someone from pubsub should take a look.

kamalaboulhosn
kamalaboulhosn reviewed Oct 1, 2018
View reviewed changes
...-clients/google-cloud-pubsub/src/main/java/com/google/cloud/pubsub/v1/MessageDispatcher.java
@Override
public void run() {
try {
if (ackHandler.totalExpiration.plusSeconds(messageDeadlineSeconds.get()).isBefore(now())) {

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@pongad pongad mentioned this pull request Oct 5, 2018
Closed
@pongad pongad requested a review from a team October 18, 2018 00:46
@pongad pongad merged commit db9d244 into googleapis:master Oct 18, 2018
@pongad pongad deleted the pubsub-old-msg branch October 18, 2018 21:27
@JesseLovelace JesseLovelace mentioned this pull request Oct 25, 2018
Merged
This was referenced Dec 4, 2018
Closed
Closed
suztomo pushed a commit that referenced this pull request Mar 9, 2026
@renovate-bot
chore(deps): update dependency com.google.cloud:google-cloud-bigtable…
0700175 
… to v2.57.3 (#3743)
lqiu96 pushed a commit that referenced this pull request Mar 20, 2026
@renovate-bot
chore(deps): update dependency com.google.cloud:google-cloud-bigtable…
b0c6436 
… to v2.57.3 (#3743)
chingor13 pushed a commit that referenced this pull request Mar 24, 2026
@diegomarquezp
fix(hermetic-build): use correct image name in templated graalvm jobs (…
c71223d 
…#3743)

Fixes a typo in two image references on templates.
chingor13 pushed a commit that referenced this pull request Mar 24, 2026
@release-please
chore(main): release 2.56.0 (#3715)
741d398 
🤖 I have created a release *beep* *boop*
---


<details><summary>2.56.0</summary>

##
[2.56.0](googleapis/sdk-platform-java@v2.55.1...v2.56.0)
(2025-04-18)


### Features

* Selective gapic generation phase II
([#3730](googleapis/sdk-platform-java#3730))
([d4bafa0](googleapis/sdk-platform-java@d4bafa0))


### Bug Fixes

* **hermetic-build:** use correct image name in templated graalvm jobs
([#3743](googleapis/sdk-platform-java#3743))
([c71223d](googleapis/sdk-platform-java@c71223d))
* plumb mtls endpoint to TransportChannelProvider
([#3673](googleapis/sdk-platform-java#3673))
([d7daf89](googleapis/sdk-platform-java@d7daf89))


### Dependencies

* add opentelemetry gcp-resources to shared deps
([#3722](googleapis/sdk-platform-java#3722))
([4b94c7d](googleapis/sdk-platform-java@4b94c7d))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chingor13 chingor13 chingor13 approved these changes

@JesseLovelace JesseLovelace Awaiting requested review from JesseLovelace

+3 more reviewers

@csainty csainty csainty left review comments

@kamalaboulhosn kamalaboulhosn kamalaboulhosn approved these changes

@loverme12 loverme12 loverme12 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pongad @csainty @chingor13 @kamalaboulhosn @loverme12 @googlebot