Add option to process messages in parallel #2
Conversation
cmd/root.go
Outdated
| @@ -97,18 +100,30 @@ func startGohaqd(cmd *cobra.Command, args []string) { | |||
| QueueUrl: q.QueueUrl, | |||
| WaitTimeSeconds: aws.Int64(20), | |||
| } | |||
| sem = make(chan *sqs.Message) | |||
There was a problem hiding this comment.
@ApsOps This channel will have only one message at a time?
Instead of this approach I think we should go with buffered channel.
sem = make(chan *sqs.Message, parallelRequests)
There was a problem hiding this comment.
@ameykpatil I had taken that approach first, but changed it since it doesn't really help. Consumers are almost always slower than producers here.
And it increases the chances of same messages being consumed buy other instances as well. If messages stay in buffer for longer than the SQS visibility timeout, they'd be picked by other gohaqd instances as well.
Does this make sense?
| resp, err := svc.ReceiveMessage(msgparams) | ||
| if err != nil { | ||
| log.Fatalf(err.Error()) | ||
| } | ||
|
|
||
| for _, msg := range resp.Messages { | ||
| sem <- msg |
There was a problem hiding this comment.
@ApsOps Would this be blocking if sem already has a message?
Buffered channel should eliminate direct dependency between pollSqs & startConsumer.
There was a problem hiding this comment.
@ameykpatil We want this to block, like I explained in the above comment.
cmd/root.go
Outdated
| for i := 0; i < parallelRequests; i++ { | ||
| go startConsumer(q.QueueUrl) | ||
| } | ||
|
|
||
| for { |
There was a problem hiding this comment.
Can you put this for loop above the other one?
Looks logically aligned. First poll then process.
There was a problem hiding this comment.
@ameykpatil We can't. Since this is an infinite loop, the following part won't be reachable.
for i := 0; i < parallelRequests; i++ {
go startConsumer(q.QueueUrl)
}
cmd/root.go
Outdated
| } | ||
| } | ||
|
|
||
| func pollSQS(queueURL *string) { | ||
| func pollSQS() { |
There was a problem hiding this comment.
Also good to add function level comments.
There was a problem hiding this comment.
@ameykpatil Added some comments.
No description provided.