sqs: Clear depduplication cache when FifoQueue is cleared #8218

martin-walsh · 2023-04-28T03:49:00Z

localstack-bot

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

localstack-bot · 2023-04-28T03:49:20Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

martin-walsh · 2023-04-28T03:49:42Z

I have read the CLA Document and I hereby sign the CLA

martin-walsh · 2023-04-28T04:17:04Z

Can't get tests to run again...

martin-walsh · 2023-04-28T05:08:58Z

closed & re-opened to get circle-ci to run...

martin-walsh · 2023-04-28T05:45:27Z

Tests failing unrelated. Looks to be flaky as arm64 test passed

FAILED tests/integration/apigateway/test_apigateway_integrations.py::test_put_integration_responses

thrau

He Martin @martin-walsh, thanks a lot for the contribution. Great find!

The solution also looks good to me. I just have two questions regarding the test to clarify before we merge. Is the test covering what it should (see comment inline)? And also just to make sure: did you run the test against aws by running the test with TEST_TARGET=AWS_CLOUD, this is what the aws_validated marker indicates.

thrau · 2023-04-28T09:07:55Z

tests/integration/test_sqs.py

+        aws_client.sqs.send_message(
+            QueueUrl=queue_url,
+            MessageBody="message-1",
+            MessageGroupId=group_id,
+            MessageDeduplicationId=dedup_id,
+        )
+
+        aws_client.sqs.purge_queue(QueueUrl=queue_url)
+
+        aws_client.sqs.send_message(


In the original reproduction template of your issue #8211 there was an additional step here before the purge (a receive message call) - is this necessary here?

It isn't necessary. The issue reproduction step was just to illustrate that the message was pushed previously & could be retrieved. That is covered by other tests.

baermat

Thank you for your contribution! I agree with the points @thrau raised, apart from that this change looks good to me 👍

martin-walsh · 2023-04-29T01:29:20Z

He Martin @martin-walsh, thanks a lot for the contribution. Great find!

The solution also looks good to me. I just have two questions regarding the test to clarify before we merge. Is the test covering what it should (see comment inline)? And also just to make sure: did you run the test against aws by running the test with TEST_TARGET=AWS_CLOUD, this is what the aws_validated marker indicates.

~~Re aws_validated I copied an existing test and modified it. Are you asking me to remove this or test against aws directly ?~~

See comment below

martin-walsh · 2023-05-01T01:50:21Z

In the interest of time, as we are in opposite time zones, I tested against aws. Here is the result:

$ aws sqs create-queue --queue-name "martin-walsh-test-queue.fifo" --attributes FifoQueue=true
{
    "QueueUrl": "https://sqs.us-west-2.amazonaws.com/xxxxxxxxxxxx/martin-walsh-test-queue.fifo"
}
$ aws sqs send-message --queue-url https://sqs.us-west-2.amazonaws.com/xxxxxxxxxxxx/martin-walsh-test-queue.fifo --message-body "Testing 123" --message-group-id "ABC" --message-deduplication-id "123"
{
    "MD5OfMessageBody": "41884e32dd65188232ce22cde06a153d",
    "MessageId": "e0ed2d75-a4bd-4fb5-9b22-212604fc9383",
    "SequenceNumber": "18877567867944687616"
}

$ aws sqs receive-message --queue-url https://sqs.us-west-2.amazonaws.com/xxxxxxxxxxxx/martin-walsh-test-queue.fifo
{
    "Messages": [
        {
            "MessageId": "e0ed2d75-a4bd-4fb5-9b22-212604fc9383",
            "ReceiptHandle": "AQEBGIW+t+2suV1N3cYMZWuqszW32RHZh3RiRSJYBETW2wKFFdb6s2++HvXrWdoIF61PR9hRkuJr2WnvpZPjq6uVUL4pa64oKjMSxMCFKsXq/9+FDslxfpUJF1rPGvCyC024uQ5PDXVsrJvltBSVzfDYgKEbg6E86a1+udkWpSo2koN0FkKO16tNIf2e3TXkG6FeIUB2iTmY/Tj3XoIYkrekq1FnWdglfnHPDqpiHpFgX+D1dtjkLQB3UpL5Uf36wJ0EARG7v+pS4f/pKcpG7ujCTH7zJlmw+n0oLwoF8YVE7P8=",
            "MD5OfBody": "41884e32dd65188232ce22cde06a153d",
            "Body": "Testing 123"
        }
    ]
}

$ aws sqs purge-queue --queue-url https://sqs.us-west-2.amazonaws.com/xxxxxxxxxxxx/martin-walsh-test-queue.fifo
$ aws sqs send-message --queue-url https://sqs.us-west-2.amazonaws.com/xxxxxxxxxxxx/martin-walsh-test-queue.fifo --message-body "Testing 123" --message-group-id "ABC" --message-deduplication-id "123"
{
    "MD5OfMessageBody": "41884e32dd65188232ce22cde06a153d",
    "MessageId": "890a376a-1728-4e85-b689-4f86b3d247af",
    "SequenceNumber": "18877567877623023872"
}
$ aws sqs receive-message --queue-url https://sqs.us-west-2.amazonaws.com/xxxxxxxxxxxx/martin-walsh-test-queue.fifo
{
    "Messages": [
        {
            "MessageId": "890a376a-1728-4e85-b689-4f86b3d247af",
            "ReceiptHandle": "AQEB6GnGYZ5ol9fcl/JxBzt+PUKrlajKMv/wJE8XuRwYJHrSvYDpM6L3mLTErFDB/5iD+MpIjJjKYVRnR7XKG+AEzOZht8f7mSG2cihOV/+FNu0tBcqG4AsIbHnyYG/7dxTeCJDs+nUH3lOmuZzFv4YXeMOsuLqJjOyQSldxCJ0aLQQoGzEQnbBgNudwA7LXeFjhCzciGHthPkRoS9Gvvqx+/mztSDl9v6qPGyFIjGhT4r662DYgXt5P5Qbv73B4uuQDGbRD2CLrMwpJzJxcpkW+WjptXiYNAEyyPxjFaZI+Jxk=",
            "MD5OfBody": "41884e32dd65188232ce22cde06a153d",
            "Body": "Testing 123"
        }
    ]
}

It behaves as the change does, and satisfies the aws_validated marker.

martin-walsh · 2023-05-08T01:58:37Z

Hi @thrau & @baermat,

Did you guys get a chance to take a look again ?

martin-walsh · 2023-05-09T09:46:12Z

Brought in changes from upstream.

martin-walsh · 2023-05-10T07:35:33Z

I've also run the test with TEST_TARGET=AWS_CLOUD TEST_PATH="tests/integration/test_sqs.py::TestSqsProvider::test_purge_queue_clears_fifo_deduplication_cache" make test and it passes.

This is blocking upgrade to localstack v2, so please let me know if there's anything that needs to be done to get this over line.

thrau

Hey @martin-walsh , thanks for the contribution and thanks for double checking that the tests run on AWS! 👍

looks good to me, and welcome to the list of contributors :-)

thrau · 2023-05-10T15:31:05Z

the change should be in the latest image in a couple of hours

martin-walsh added 2 commits April 28, 2023 13:38

sqs: Clear depduplication cache when FifoQueue is cleared

5a7ce60

sqs: Add test for clearing of FifoQueue deduplication cache on purge

0382008

martin-walsh requested review from thrau and baermat as code owners April 28, 2023 03:49

localstack-bot reviewed Apr 28, 2023

View reviewed changes

localstack-bot added a commit that referenced this pull request Apr 28, 2023

@martin-walsh has signed the CLA from Pull Request #8218

cb8e572

fix lint errors in test

e2e3bd2

martin-walsh requested a review from localstack-bot April 28, 2023 04:07

martin-walsh closed this Apr 28, 2023

martin-walsh reopened this Apr 28, 2023

thrau reviewed Apr 28, 2023

View reviewed changes

baermat reviewed Apr 28, 2023

View reviewed changes

martin-walsh requested review from thrau and baermat May 1, 2023 11:14

martin-walsh added 4 commits May 9, 2023 18:54

Merge branch 'master' into master

04d3b98

Fix merge

38e8d9c

Merge branch 'localstack:master' into master

3e4eaae

sqs: fix python syntax in models.py

cad0a50

thrau mentioned this pull request May 10, 2023

clear deduplication cache in SQS fifo queues #8293

Closed

thrau approved these changes May 10, 2023

View reviewed changes

thrau merged commit 815c399 into localstack:master May 10, 2023
12 checks passed

thrau mentioned this pull request May 10, 2023

bug: SQS deduplication cache is not reset when queue is purged #8211

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sqs: Clear depduplication cache when FifoQueue is cleared #8218

sqs: Clear depduplication cache when FifoQueue is cleared #8218

martin-walsh commented Apr 28, 2023

localstack-bot left a comment

localstack-bot commented Apr 28, 2023 •

edited

martin-walsh commented Apr 28, 2023

martin-walsh commented Apr 28, 2023

martin-walsh commented Apr 28, 2023

martin-walsh commented Apr 28, 2023

thrau left a comment

thrau Apr 28, 2023

martin-walsh Apr 29, 2023

baermat left a comment

martin-walsh commented Apr 29, 2023 •

edited

martin-walsh commented May 1, 2023 •

edited

martin-walsh commented May 8, 2023

martin-walsh commented May 9, 2023

martin-walsh commented May 10, 2023

thrau left a comment

thrau commented May 10, 2023 •

edited

sqs: Clear depduplication cache when FifoQueue is cleared #8218

sqs: Clear depduplication cache when FifoQueue is cleared #8218

Conversation

martin-walsh commented Apr 28, 2023

localstack-bot left a comment

Choose a reason for hiding this comment

localstack-bot commented Apr 28, 2023 • edited

martin-walsh commented Apr 28, 2023

martin-walsh commented Apr 28, 2023

martin-walsh commented Apr 28, 2023

martin-walsh commented Apr 28, 2023

thrau left a comment

Choose a reason for hiding this comment

thrau Apr 28, 2023

Choose a reason for hiding this comment

martin-walsh Apr 29, 2023

Choose a reason for hiding this comment

baermat left a comment

Choose a reason for hiding this comment

martin-walsh commented Apr 29, 2023 • edited

martin-walsh commented May 1, 2023 • edited

martin-walsh commented May 8, 2023

martin-walsh commented May 9, 2023

martin-walsh commented May 10, 2023

thrau left a comment

Choose a reason for hiding this comment

thrau commented May 10, 2023 • edited

localstack-bot commented Apr 28, 2023 •

edited

martin-walsh commented Apr 29, 2023 •

edited

martin-walsh commented May 1, 2023 •

edited

thrau commented May 10, 2023 •

edited