-
Notifications
You must be signed in to change notification settings - Fork 492
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
IdempotentRequestCache
: Remove request timeout
#9581
IdempotentRequestCache
: Remove request timeout
#9581
Conversation
…ementation and move it outside
…remove-request-timeout
using CancellationTokenSource timeoutCts = new(RequestTimeout); | ||
using CancellationTokenSource linkedCts = CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have deliberately copy&pasted these two lines to ConfirmConnectionAsync
and other methods because if I were to use a method it would be very generic method and probably harder to understand.
That can be still done if you find it better.
responseTcs.SetException(!timeoutCts.IsCancellationRequested | ||
? e | ||
: new InvalidOperationException("DeadLock prevention timeout kicked in!", e)); | ||
responseTcs.SetException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main change. The rationale behind this change is: Whatever request timeout we choose, it's certainly not good enough for application-wide use.
So then we can just leaving it up to users of Idempotency Request Cache to specify a timeout they need OR don't specify any timeout at all (block fetching shows that we might wait very long time).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cACK
…IdempotentRequestCache-remove-request-timeout `IdempotentRequestCache`: Remove request timeout
Depends on #9577
This is an idea how to fix: #9469 (comment)
The idea is simply to remove
private static TimeSpan RequestTimeout { get; } = TimeSpan.FromMinutes(5);
fromIdempotencyRequestCache
and move it simply to the callers that need it.