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

Fail and delay batch items #6

Merged
merged 3 commits into from
Mar 28, 2024
Merged

Fail and delay batch items #6

merged 3 commits into from
Mar 28, 2024

Conversation

yaroslav-tykhonchuk
Copy link
Collaborator

Fail

When only failed items left -> update message with these items.
When flush is triggered by timeout -> update message with both failed and not processed items.

Delay

When one or more items are delayed -> update message with failed and not processed items with a minimum visibility timeout present.

We consider a situation when a batch is triggered by timeout together with a visibility timeout a very rare situation. So we don't care that other items in a batch are affected by a visibility timeout of another item.

FlushIfEmpty(remaining);
}

public void Delay(BatchItemId itemId, TimeSpan delayTime)
Copy link
Member

Choose a reason for hiding this comment

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

I am unsure about exposing such a specific API in a general-purpose queue. It smells a bit.
Maybe it's better to implement delays in place where they are needed. I.e. by putting data that need to be delayed as a separate message into the queue.

Imagine a case when we receive a batch of mentions that we want to delay (e.g. some news from the provider). And those mentions have different delay times. We will try to process same queue message several times then (as each time, we will delay the rest of the unprocessed mentions). Eventually, the queue message might go to quarantine because of exceeding the dequeue count.

I would leave delay functionality to library clients and try to keep the batch queue as simple as possible (only cover cases that are a must for all the scenarios).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think the concept of Delay is a specific case in a general-purpose queue. What we do here is basically use the visibility timeout feature of the AzureQueue. But the implementation of Delay in my case is really not thought well-enough and needs a lot more details, so I will move it to the clients.

@yaroslav-tykhonchuk yaroslav-tykhonchuk merged commit 9eb7689 into main Mar 28, 2024
2 checks passed
yaroslav-tykhonchuk added a commit that referenced this pull request Apr 11, 2024
Implement Fail functionality of batch items
@yaroslav-tykhonchuk yaroslav-tykhonchuk deleted the fail-delay branch April 11, 2024 12:29
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.

2 participants