internal/state: add ctx deadline exceeded check on membership consistency check#609
internal/state: add ctx deadline exceeded check on membership consistency check#609louiseschmidtgen wants to merge 1 commit intocanonical:v2from
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens the membership consistency check loop to abort promptly when the provided context has already expired or been canceled, improving behavior on the v2 (LTS) branch without broader API changes.
Changes:
- Add an early
ctx.Err()check at the start of theCheckMembershipConsistencyretry loop to return immediately when the context is done. - Preserve the existing 5-second default timeout behavior when no deadline is provided, and maintain existing retry-and-backoff semantics.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
louiseschmidtgen
force-pushed
the
KU-5062/ctx-v2
branch
from
dc921c5 to
e76bcff
Compare
| } | ||
|
|
||
| for { | ||
| ctxErr := ctx.Err() |
There was a problem hiding this comment.
The more I think about this, I am less confident that this copilot comment is actually useful.
If we call getMembershipData with an already expired context, we still get Failed to gather membership data for consistency check: context deadline exceeded which is explanatory enough IMHO.
So it's not that we loose anything around the fact we are trying to perform the consistency check. It's even written in the error message.
Let's just fix e76bcff#r2762736434 and we should be good to go. Fine for you?
There was a problem hiding this comment.
I was proposing to again drop this ctxErr := ctx.Err() ... section as it looks to me the copilot comment wasn't that useful as we would still have a nice and descriptive error message returned by getMembershipData.
…ency check Signed-off-by: Louise K. Schmidtgen <louise.schmidtgen@canonical.com>
louiseschmidtgen
force-pushed
the
KU-5062/ctx-v2
branch
from
e76bcff to
b13352f
Compare
| } | ||
|
|
||
| for { | ||
| ctxErr := ctx.Err() |
There was a problem hiding this comment.
I was proposing to again drop this ctxErr := ctx.Err() ... section as it looks to me the copilot comment wasn't that useful as we would still have a nice and descriptive error message returned by getMembershipData.
2 participants
Add ctx deadline exceeded check on membership consistency check
This PR adds a ctx deadline exceeded check in the membership consistency check loop on the v2 branch.
Nice to know: In v3 we pass the request's context, however, we do not want to make any drastic changes to the LTS branch.