aws-sdk-go-v2 icon indicating copy to clipboard operation
aws-sdk-go-v2 copied to clipboard

SQS ReceiveMessage overrides message when used in batch

Open dgivoli opened this issue 3 years ago • 3 comments

Describe the bug

Not exactly a bug, more a pitfall - When using SQS ReceiveMessage with MaxNumberOfMesages > 1, the sdk returns a slice of Message objects. When iterating the slice we noticed we get the same Message object instance on each iteration, but each time filled with different data. While this works when messages are handled in series, this behavior caused us data overriding when we passed the message pointer to another goroutine through a channel. The solution of course was to pass the message by value, but this object pooling behavior seem quite risky (assuming it is intended)

Expected Behavior

Each message should have a new Message instance

Current Behavior

Messages share instance

Reproduction Steps

call Receive message with MaxNumberOfMesages > 1 and inspect the object address

Possible Solution

instantiate new message object for each Message

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

github.com/aws/aws-sdk-go-v2 v1.16.3
github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.10
github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.4

Compiler and Version used

go version go1.18.1 linux/amd64

Operating System and version

Ubuntu 22.04

dgivoli avatar Oct 04 '22 12:10 dgivoli

Hi @dgivoli

Can you please provide your code?

Thanks, Ran

RanVaknin avatar Oct 04 '22 18:10 RanVaknin

@RanVaknin It's a bit tricky because our code is wrapped with interfaces and split accross some goroutines. we are using the standard ReceiveMessage call. I'm re-writing some parts for this snippet

receiveMessageConfig := &sqs.ReceiveMessageInput{
 QueueUrl:              "queueUrl",
 AttributeNames:        []types.QueueAttributeName{"All"},
 MessageAttributeNames: []string{"All"},
 MaxNumberOfMessages:   10,
 VisibilityTimeout:     90,
 WaitTimeSeconds:       20
}
if output, err := s.client.ReceiveMessage(ctx, receiveMessageConfig); err != nil {
 //handle err
} else {
 for _, message := range output.Messages {
  someChannel <- &message
 }
}

dgivoli avatar Oct 04 '22 20:10 dgivoli

@dgivoli

Thanks for the code snippet.

The issue is not with the SDK. This is a classic Golang "gotcha" In your case message is not being not being overwritten with a new value because it is declared once per for loop code block, not once per iteration.

you can solve it like so:

for _, message := range output.Messages {
    message := message
    someChannel <- &message
}

Let me know if that helps.

Thanks, Ran~

RanVaknin avatar Oct 04 '22 21:10 RanVaknin

@RanVaknin Thanks. Very helpful. I do see now that there are a lot of discussions around this

dgivoli avatar Oct 05 '22 10:10 dgivoli

@dgivoli ,

My pleasure! 😄 I assume this solved the problem. I'm going to close this thread and If you're running into more problems, feel free to open another issue.

Thanks, Ran~

RanVaknin avatar Oct 05 '22 17:10 RanVaknin

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions[bot] avatar Oct 05 '22 17:10 github-actions[bot]