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

[Clarification]: Hedging contexts #1727

Closed
peter-csala opened this issue Oct 24, 2023 · 3 comments · Fixed by #1749
Closed

[Clarification]: Hedging contexts #1727

peter-csala opened this issue Oct 24, 2023 · 3 comments · Fixed by #1749

Comments

@peter-csala
Copy link
Contributor

peter-csala commented Oct 24, 2023

Is your feature request related to a specific problem? Or an existing feature?

While I was playing with hedging strategy I realized that we have PrimaryContext and ActionContext on OnHedgingArguments<TResult> struct.

It is unclear:

  • what is difference between the two
    • I did not find the documentation comment really helpful
  • when to use which
    • hedging documentation does not even mention primary only the ActionContext

Describe the solution you'd like

I think we could improve on the documentation comments also on the hedging documentation page.

Additional context

No response

@martintmk
Copy link
Contributor

Because of the nature of hedging strategy, there might be multiple parallel actions being executed. To avoid race conditions we "duplicate" the original context passed to the strategy (primary) and provide it as an argument to action generator.

When hedging finishes we merge the context that was used for accepted action and merge it back to original context.

@peter-csala
Copy link
Contributor Author

It seems like we have some sort of fork-join model (or fan-out + fan-in). And that raises a lots of questions:

  • Are we talking about shallow or deep copy?
  • What happens during the merger if the primary and the action context have the same key with different values?
  • What happens during the merger if the action context contains a soft-delete (the value is set to null)?
  • Since action generator is called before the OnHedging, will the changes on the ActionContext from the action generator be available in the OnHedging delegate?
  • ...

So, my main point here is that without proper documentation i think it is easy to misuse these contexts.

Because V8 is already out we could not perform a renaming but RequestContext and HedgedRequestContext might be a better name IMHO. (For me the Primary prefix indicates that there could/should be a Secondary as well.)

@martintmk martintmk mentioned this issue Oct 29, 2023
4 tasks
@martintmk
Copy link
Contributor

@peter-csala I have created a #1749 to improve docs regarding the contexts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants