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

Allow for dynamic port usage for local rpc endpoint if 17071 in use #1307

Merged
merged 2 commits into from
Apr 9, 2020

Conversation

ConnorMcMahon
Copy link
Contributor

Our initial implementation of Local HTTP for out-of-proc languages always uses the port 17071. This can cause issues with slots, as well as caused some noisy telemetry when hosts would shut down occasionally.

We now check to see if the default port is available before attempting to bind to it. There is still some degree of race condition if the extensions are instantiated before the port is being listened to, but in general the OS should provide some protections against this.

@ConnorMcMahon
Copy link
Contributor Author

I still want to do some end-to-end testing for this PR in Functions V2 and V3, and make sure it doesn't break anything in V1.

Copy link
Member

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

Looks great! Just a couple tiny suggestions.

@cgillum cgillum added this to the v2.2.1 milestone Apr 7, 2020
Copy link
Contributor

@davidmrdavid davidmrdavid left a comment

Choose a reason for hiding this comment

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

It looks good to me, thank you for this change.

Our initial implementation of Local HTTP for out-of-proc languages always uses the port 17071. This can cause issues with slots, as well as caused some noisy telemetry when hosts would shut down occasionally.

We now check to see if the default port is available before attempting to bind to it. There is still some degree of race condition if the extensions are instantiated before the port is being listened to, but in general the OS should provide some protections against this.
@ConnorMcMahon ConnorMcMahon force-pushed the AddPortDiscoveryLocalRpc branch from bdf181f to 44bf3fb Compare April 8, 2020 17:06
@ConnorMcMahon
Copy link
Contributor Author

Tested in FunctionsV1 and FunctionsV3 to be sure neither broke (Functions V1 due to conditional compilation, FunctionsV3 to verify no synchronous calls in HTTP pipeline).

Also tested end-to-end in Durable JS, and verified using RPC URI with a different port.

@ConnorMcMahon ConnorMcMahon merged commit b9c1bc7 into dev Apr 9, 2020
@ConnorMcMahon ConnorMcMahon deleted the AddPortDiscoveryLocalRpc branch April 9, 2020 19:08
@CastleArg
Copy link

is there any way of getting my hands on this fix? I've had a durable function go down without warning and cannot start even after deleting and recreating the app because of this port in use error

@cgillum
Copy link
Member

cgillum commented May 29, 2020

Related: #1273

ConnorMcMahon pushed a commit that referenced this pull request Jun 6, 2020
ConnorMcMahon pushed a commit that referenced this pull request Jun 12, 2020
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.

4 participants