Fix Indefinite Wait #4
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where channels could wait indefinitely due to dropped Redis invalidation notifications. The fix implements a timeout mechanism to prevent indefinite waits and improves context handling.
- Replaced simple channels with lockEntry struct containing context and cancel function for better lifecycle management
- Added automatic timeout mechanism using context.WithTimeout to prevent indefinite waits
- Improved error handling and context cancellation throughout the codebase
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cacheaside.go | Core implementation changes replacing channel-based locks with context-based lockEntry system and timeout handling |
| cacheaside_test.go | Added test case to verify proper parent context cancellation behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| defer cancel() | ||
| rca.unlock(toCtx, key, lockVal) | ||
| // Best effort unlock - errors are non-fatal as lock will expire | ||
| _ = rca.unlock(toCtx, key, lockVal) |
There was a problem hiding this comment.
The error from unlock is being silently discarded. Consider logging the error for debugging purposes while maintaining the non-fatal behavior.
| defer wg.Done() | ||
| delKeyLua.ExecMulti(ctx, rca.client, stmts...) | ||
| // Best effort unlock - errors are non-fatal as locks will expire | ||
| _ = delKeyLua.ExecMulti(ctx, rca.client, stmts...) |
There was a problem hiding this comment.
Similar to the single unlock, the error from unlockMulti is being silently discarded. Consider logging the error for debugging purposes while maintaining the non-fatal behavior.
2 participants
Uh oh!
There was an error while loading. Please reload this page.