Skip to content

[release/5.0] DelegationRule.Dispose() unsets delegation property on the queue#28514

Merged
wtgodbe merged 3 commits into
release/5.0from
shirhatti/unsetdelegationrulebackport
Dec 9, 2020
Merged

[release/5.0] DelegationRule.Dispose() unsets delegation property on the queue#28514
wtgodbe merged 3 commits into
release/5.0from
shirhatti/unsetdelegationrulebackport

Conversation

@shirhatti

Copy link
Copy Markdown
Contributor

Backporting #28342 to 5.0

Description

When a new DelegationRule is created, SetDelegationProperty gets called to add a HttpServerDelegationProperty property flag to the RequestQueue owned/created by the process (i.e. not the RequestQueue the delegator is trying to delegate to). When a DelegationRule is disposed, this property flag is not unset. This keeps the delegator process linked to destination queue and prevents a new DelegationRule rule for the same from being created later on.

Customer impact

Backport requested by @NGloreous. Has been validated (and submitted) by him

Ideally as a consumer of this feature I want to be able to dynamically add/remove rules in production via config without the need to restart the process and this bug blocks this ability.

Regression

No. This is a new feature introduced in 5.0

Risk

Very low.

NGloreous and others added 3 commits December 8, 2020 09:25
…from the target URL group and allows the delegation rule to be added back later on in the processes lifetime
@ghost ghost added the area-servers label Dec 8, 2020
@shirhatti shirhatti changed the base branch from master to release/5.0 December 8, 2020 17:30
@shirhatti shirhatti added this to the 5.0.2 milestone Dec 8, 2020
@shirhatti shirhatti added the Servicing-consider Shiproom approval is required for the issue label Dec 8, 2020
@ghost

ghost commented Dec 8, 2020

Copy link
Copy Markdown

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@Tratcher

Tratcher commented Dec 8, 2020

Copy link
Copy Markdown
Member

Note #28342 is still pending.

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Dec 8, 2020
@wtgodbe

wtgodbe commented Dec 9, 2020

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 2 pipeline(s).

@wtgodbe

wtgodbe commented Dec 9, 2020

Copy link
Copy Markdown
Member

CI failures aren't from the most recent run, it's just being buggy. Merging.

@wtgodbe

wtgodbe commented Dec 9, 2020

Copy link
Copy Markdown
Member

Oh, never mind, someone needs to approve - @Tratcher @jkotalik @halter73 does this look good?

_sourceQueueUrlGroup.UnSetDelegationProperty(Queue, throwOnError: false);
}
catch (ObjectDisposedException) { /* Server may have been shutdown */ }
Queue.UrlGroup.Dispose();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why was the nullable check removed here?

@wtgodbe wtgodbe merged commit e47cbf6 into release/5.0 Dec 9, 2020
@wtgodbe wtgodbe deleted the shirhatti/unsetdelegationrulebackport branch December 9, 2020 21:33
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants