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

AJ-1485: Ignore redundant AvroUpsert messages. #2656

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

jladieu
Copy link
Contributor

@jladieu jladieu commented Dec 6, 2023

AJ-1485 Maintenance Ticket

Double deliveries can happen occasionally, per:
https://cloud.google.com/pubsub/docs/subscription-overview#default_properties

Rather than let such redundant messages get stuck in the queue, ack them to clear them out.

This is done as a response to a Rawls import pub/sub message alert to prevent future such messages from getting stuck in the queue.

@jladieu jladieu force-pushed the AJ-1485-ack-double-delivered-messages branch 2 times, most recently from 220ad76 to 565d3b2 Compare December 6, 2023 16:11
@jladieu
Copy link
Contributor Author

jladieu commented Dec 6, 2023

/jenkins retest

Copy link
Contributor

@davidangb davidangb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

Comment on lines +875 to +868
// Publish message on the request topic
services.gpsDAO.publishMessages(
importReadPubSubTopic,
List(MessageRequest(importUuid.toString, testAttributes(importUuid)))
)

// check that a pubsub message was acked.
eventually(Timeout(scaled(timeout)), Interval(scaled(interval))) {
services.gpsDAO.acks should not be empty
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking out loud … I poked at this a bit, and really wanted to find a way to assert that it is acking the second message - the duplicate one - but given the structure of MockGooglePubSubDAO I couldn't find a way to do that. I don't think it's worth refactoring a whole bunch of stuff to make that test work.

@jladieu jladieu force-pushed the AJ-1485-ack-double-delivered-messages branch from 0bd38c1 to 8aa6539 Compare December 6, 2023 20:16
Double deliveries can happen occasionally:
https://cloud.google.com/pubsub/docs/subscription-overview#default_properties

Rather than let such redundant messages get stuck in the queue, ack them
to clear them out.
@jladieu jladieu force-pushed the AJ-1485-ack-double-delivered-messages branch from 8aa6539 to ab354b0 Compare December 6, 2023 20:16
Copy link
Contributor

@calypsomatic calypsomatic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jladieu jladieu merged commit 92cd863 into develop Dec 6, 2023
12 checks passed
@jladieu jladieu deleted the AJ-1485-ack-double-delivered-messages branch December 6, 2023 21:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants