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

Skip HostIdValidator checks in placeholder mode #10789

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

mathewc
Copy link
Member

@mathewc mathewc commented Feb 3, 2025

We're seeing cases in production where placeholder specializations are failing with host ID collision errors. Here's how this bug manifests itself:

  • In placeholder mode the first time the host ID is accessed during startup code here in the HostId provider will have scheduled the HostId validation for 5 seconds later (cold start optimization).
  • this validation isn't supposed to run in placeholder mode since it requires a storage connection, and code here on the validation path is supposed to prevent that. However, because of the coldstart delay, the validation can run after specialization with the app environment and storage connection, but the closure has captured the placeholder mode host ID e.g. functionsv4node18placeholdertemp.
  • Due to race conditions between specialization, host ID access and validation, a blob entry can be written for the placeholder ID pointing to the actual host name (e.g. myapp.azurewebsites.net), rather than the placeholder hostname (e.g. functionsv4node18placeholdertemplatesite.azurewebsites.net).
  • Depending on timing, this can cause the validation check here to fail due to an expected hostname mismatch.

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

  • Backporting to the in-proc branch is not required
    • TODO: Will backport to in-proc
  • 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)

@mathewc mathewc requested a review from a team as a code owner February 3, 2025 22:57
@mathewc mathewc changed the title Skip HostIdValidator checks in placehlder mode Skip HostIdValidator checks in placeholder mode Feb 3, 2025
@mathewc mathewc force-pushed the hostid-validator-fix branch from 9721c75 to 902d2ba Compare February 4, 2025 00:39
@@ -168,6 +173,105 @@ await TestHelpers.Await(() =>
Assert.NotSame(GetCachedTimeZoneInfo(), _originalTimeZoneInfoCache);
}

[Fact]
public async Task StandbyModeE2E_Dotnet_HostIdValidator_DoesNotRunInPlaceholderMode()
Copy link
Member Author

Choose a reason for hiding this comment

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

This test reproduces the issue 100% of the time. I couldn't get a reliable repro since a race condition is involved, so as the comments below state, I resorted to forcing the condition via a direct blob write.

@mathewc mathewc requested review from fabiocav and soninaren February 4, 2025 17:15
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.

1 participant