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

Check if a container or table exists before trying to create it #9555

Merged
merged 4 commits into from
Sep 26, 2023

Conversation

liliankasem
Copy link
Member

Issue describing the changes in this PR

Resolves multiple GH issues where customers are seeing an excessive amount of "409 The specified container already exists" errors in their logs:

warn: Azure.Core[8]
      Error response [d35f480a-744d-4871-ac64-20693baf387a] 409 The specified container already exists. (00.0s)
      Server:Windows-Azure-Blob/1.0 Microsoft-HTTPAPI/2.0
      x-ms-request-id:6ee968fc-c01e-00a4-5f07-ec787b000000
      x-ms-client-request-id:d35f480a-744d-4871-ac64-20693baf387a
      x-ms-version:2021-08-06
      x-ms-error-code:ContainerAlreadyExists
      Date:Wed, 20 Sep 2023 21:15:31 GMT
      Content-Length:230
      Content-Type:application/xml

CreateIfNotExistsAsync throws an exception if the container exists. An issue was opened for this in the sdk repo but was closed unresolved. This was an intention change and not a bug (more details). However, it still does lead to excessive logs.

The change may only need to happen for the blob container but figured better safe and do it for all usage of CreateIfNotExistsAsync.

Relevant issues:

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • My changes do not require diagnostic events changes
    • Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
  • I have added all required tests (Unit tests, E2E tests)

Additional information

Additional PR information

@liliankasem liliankasem requested a review from a team as a code owner September 20, 2023 21:43
@mathewc
Copy link
Member

mathewc commented Sep 25, 2023

Is the issue you referenced for 409s talking about conflicts coming from storage operations performed by the Host (i.e. code in this repo), or by code in lower levels? I ask because most of the storage operations performed by the Host are done in the WebJobs SDK, and looking through all the CreateIfNotExistsAsync calls (e.g. here) they all seem to be using the existing pattern of just relying on the storage SDK method.

So I'm wondering whether the changes made in this PR will actually address the customer issue? I.e. we're using CreateIfNotExistsAsync all over the place, down through transitive dependencies.

@liliankasem
Copy link
Member Author

Is the issue you referenced for 409s talking about conflicts coming from storage operations performed by the Host (i.e. code in this repo), or by code in lower levels? I ask because most of the storage operations performed by the Host are done in the WebJobs SDK, and looking through all the CreateIfNotExistsAsync calls (e.g. here) they all seem to be using the existing pattern of just relying on the storage SDK method.

So I'm wondering whether the changes made in this PR will actually address the customer issue? I.e. we're using CreateIfNotExistsAsync all over the place, down through transitive dependencies.

That's a good point, a couple years ago we made a similar change but for the lease acquisition for a similar reason, and I think we missed the container creation flow. I think we should change this in the WebJobs SDK too if we can. I did some testing locally and this did reduce some of the 409 noise.

@liliankasem
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liliankasem liliankasem merged commit 65dcbe8 into dev Sep 26, 2023
@liliankasem liliankasem deleted the liliankasem/409errorlog branch September 26, 2023 21:57
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.

3 participants