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

Poison blob queue message in multi storage account scenario #804

Closed
Fabian-Schmidt opened this issue Aug 30, 2016 · 11 comments
Closed

Poison blob queue message in multi storage account scenario #804

Fabian-Schmidt opened this issue Aug 30, 2016 · 11 comments
Assignees
Milestone

Comments

@Fabian-Schmidt
Copy link

Hi,
I have setup two storage accounts in the connectionStrings. First one is setup as AzureWebJobsStorage and a second as AzureWebJobsMyToken.

In the second account a blob trigger is configured via an attribute on the class [StorageAccount("MyToken")].
In case this blob trigger fails multiple times a poison blob message is generated in the queue webjobs-blobtrigger-poison of the first storage account.
My expectation was that the same storage account, as used for the trigger, is used for the poison message.

Thanks,
Fabian

@mathewc
Copy link
Member

mathewc commented Aug 30, 2016

Thanks for reporting. What version of the SDK are you using?

@Fabian-Schmidt
Copy link
Author

Microsoft.Azure.WebJobs Version 1.1.2.0
Microsoft.Azure.WebJobs.Extensions Version 1.0.1.0
Microsoft.Azure.WebJobs.Host Version 1.1.2.0

@mamaso
Copy link
Contributor

mamaso commented Sep 13, 2016

@christopheranderson @mathewc Unfortunately this scenario and #797 are basically incompatible (if we add the poison queue to the secondary storage account it cannot be a blob-only storage account).

Which issue do you guys want to prioritize?

I think that supporting blob-only storage unlocks more options. We could do a more complex fix where we put the poison queue on the secondary storage account unless if it is blob-only, but I'm not sure it's worth the complexity & inconsistent behavior.

@lindydonna
Copy link
Contributor

@mamaso We were thinking of excluding blob only storage accounts for RC, since it requires perf testing as well. Does that simplify the decision?

@mathewc
Copy link
Member

mathewc commented Sep 14, 2016

@mamaso My thought was that the queue we use for poison blobs would live in the primary storage account. That's the same place the trigger queue (our internal implementation detail) we create for blob triggers lives. We scan for blobs and add items to the trigger queue, which the shared blob listener is monitoring to trigger functions. Can't the poison queue live in the same place?

@mamaso
Copy link
Contributor

mamaso commented Sep 14, 2016

Yep, I think it's fine for the primary storage account to have all the queues. Perhaps the answer is to document the poison queue behavior better since this didn't match the user's expectation of where it would be created.

@mamaso mamaso closed this as completed Sep 20, 2016
@mathewc
Copy link
Member

mathewc commented Sep 20, 2016

What is our behavior for poison queues for QueueTrigger functions in the context of secondary storage accounts? We want to ensure that we have a consistent story.

@mathewc mathewc reopened this Sep 20, 2016
@mamaso
Copy link
Contributor

mamaso commented Sep 20, 2016

Just tested, QueueTrigger will create the poison queue in the secondary account, so there's some inconsistency here.

@mathewc
Copy link
Member

mathewc commented Sep 20, 2016

Yeah, that's what I thought. We'll need to drive towards a consistent model for all of these.

@mathewc mathewc assigned mathewc and unassigned mamaso Sep 23, 2016
@mathewc
Copy link
Member

mathewc commented Sep 26, 2016

The start of a fix is in a branch I just pushed blobfix. I will resume that work when I have time - the fix needs more improvements.

@mathewc
Copy link
Member

mathewc commented Jan 6, 2017

Fixed in 56b5302. By default the poison queue will no be colocated in the account where the blob container lives. This is configurable via JobJostConfiguration.Blobs.CentralizedPoisonQueue. Preview build with these changes can be gotten on our myget feed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants