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

Feature idea: multi-bulkhead #507

Closed
bartelink opened this issue Sep 29, 2018 · 2 comments
Closed

Feature idea: multi-bulkhead #507

bartelink opened this issue Sep 29, 2018 · 2 comments

Comments

@bartelink
Copy link
Contributor

bartelink commented Sep 29, 2018

I'm looking to implement a refinement to Bulkhead (which would likely wrap normal Bulkheads) that facilitates governing by queueing and/or load-shed if the present set of in-flight requests in aggregate breach parallelism/max queueing thresholds.

I'm posting this here in the hope there will be either prior art, or someone with expertise in this space will tell me why it's a fundamentally broken idea.

An example might be that I define something like:

.BulkheadAsyncMulti(
    facetSelector: ctx => new ctx["type"]=="VIP") ? null : ctx["ClientId"],
    factory: () => Policy.BulkheadAsync(maxParallel: 100, maxQueued: 100))

which internally maintains some form of ConcurrentDictionary<string,BulkheadPolicy> with the following constraints:

  • every request gets passed to the facetSelector, which gets to decide:
    a) to blindly let it through uncounted [trusting an outer Bulkhead to adequately restrict request flow]
    b) OR the id of a sub-bulkhead from which the request should be allocated/queued/shed, and the weight to be allocated
  • factory means inner bulkhead rules can be tuned per facet
  • same callbacks as a normal Bulkhead, with an extra facet argument
  • if 2 come in concurrently, needs to ensure limits are upheld
  • needs to purge out entries that drop to zero automatically (perhaps based on a lock free construct, backed up by a timer, perhaps KISS like this?)
  • runaway state can be shielded against by putting a bulkhead outside to limit/queue/shed overall active calls
  • no prioritization, etc. required or desired - this is purely about making sure flow per defined bucketing of in-flight requests is kept constrained in the interests of fairness

Consumer code will go somewhere in here.

My current thinking is that this scheme is not sufficiently general in nature to warrant consideration as a Polly feature. the generalization of having a factory suggests this can be used to make policies other than Bulkhead be adaptive for cases where e.g. some clients should not be and/or should be more throttled than others and would allow special case code to be expressed by passing arguments in the Context, where they are easier to test, rather than sprinkling the special cases around the codebase.

My current plan is to implement this within my wrapper lib and see how well it generalizes beyond just being used for multi-bulkheads. Subsequently, it can always be plucked into Polly if it a) it proves its worth and b) others are interested.

...but I'm happy to be told I'm wrong!

@bartelink bartelink changed the title Feature request: multi-bulkhead Feature idea: multi-bulkhead Sep 29, 2018
@reisenberger
Copy link
Member

Umbrella or aggregating bulkhead: Not a broken idea. PolicyWrap is intentionally free-form enough to allow you to use the same type of Policy more than once in a wrap.

Policy selector: I can see mileage for this as a concept generalised outside the multi-bulkhead scenario. We built something like this already in the Polly-HttpClientFactory integration, a dynamic, execution-time Policy selector and configuration overloads also to make that play easily with PolicyRegistry. For HttpClientFactory the key discriminating input parameter was the HttpRequestMessage, but if we took this concept back into Polly that would naturally be Context. So we could probably have signatures like:

Policy.Selector(Func<Context, ISomePolicyInterfaceVariant> selector) // might need an overload for each return type ISyncPolicy, ISyncPolicy<TResult>, IAsyncPolicy and IAsyncPolicy<TResult>
Policy.Selector(Func<Context, IReadOnlyPolicyRegistry<TKey>, ISomePolicyInterfaceVariant> selector)

Interested in community feedback - interest in this? other possible uses for it?

@bartelink
Copy link
Contributor Author

I've implemented this in jet/CallPolly#17 - I'm intending to have it prove its worth over there and/or wait for others to have similar needs that we can generalize from before implementing a real solution in this context so am closing this for now. Thanks again @reisenberger for helping me think this through!

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

No branches or pull requests

2 participants