Scooletz / QueueBatch

WebJobs/Azure Functions trigger providing batches of Azure Storage Queues messages directly to your function
Apache License 2.0
41 stars 4 forks source link

Make MarkAsProcessed thread safe #10

Closed jeremy001181 closed 5 years ago

jeremy001181 commented 5 years ago

Good point! thanks, I will make an update accordingly and probably add some tests around this scenario as well.

On Thu, 20 Dec 2018 at 08:13, Marcin Kral notifications@github.com wrote:

@krlm commented on this pull request.

In src/QueueBatch/Impl/MessageBatch.cs https://github.com/Scooletz/QueueBatch/pull/10#discussion_r243184195:

     {

var tasks = new Task[messages.Length];

  • var messagesToDelete = processed.ToList();

ConcurrentBag accepts duplicates, which is not a problem for Delete I guess, however ReleaseMessage and MoveToPoisonQueue doesn't look like idempotent operations, so what do you think about:

var messagesToDelete = new HashSet(processed);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Scooletz/QueueBatch/pull/10#pullrequestreview-186884518, or mute the thread https://github.com/notifications/unsubscribe-auth/ACO9hZsefblyaQLdkhBehSZMS_dfOK30ks5u60a8gaJpZM4ZaCkt .

-- Zhongming Chen

Scooletz commented 5 years ago

Oh my, I needed to unroll the comment as it was done in the original branch I suppose :-) Is this Work In Progress then, @jeremy001181 ?

krlm commented 5 years ago

@Scooletz sorry for confusion, I've deleted my original comment because I got tricked by diff, actually @jeremy001181 change is not relevant for ReleaseMessage and MoveToPoisonQueue. However, using a hashset/distinct there might help to avoid superfluous calls to queue client.

Scooletz commented 5 years ago

Replaced by #11