-
Notifications
You must be signed in to change notification settings - Fork 495
feat: add connection timeout configuration for Redis operations #987
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
Conversation
|
LGTM @arkodg thoughts? |
|
Looks like it's failing on precommit. Possible to run |
README.md
Outdated
|
|
||
| Use the following environment variables to configure timeouts: | ||
|
|
||
| 1. `REDIS_TIMEOUT`: sets the timeout for Redis connection and I/O operations. Default: `10s` |
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.
can you point out the defaults from the lib docs or code
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.
updated |
|
hi @collin-lee , @arkodg please help me review it again |
Signed-off-by: notdu <huudutg@gmail.com>
arkodg
left a comment
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.
LGTM thanks
|
@collin-lee It seems like this PR needs approval from reviewer who have write access |
|
hi @collin-lee, This PR still cannot be merged due to two workflows awaiting approval. This workflow requires approval from a maintainer |
|
@notdu - merged |
fix: #978 - Add REDIS_TIMEOUT first; implement the circuit breaker in a separate PR.
Introduces configurable timeouts for Redis connection and I/O operations via environment variables
REDIS_TIMEOUTandREDIS_PERSECOND_TIMEOUT, defaulting to 10 seconds. Updates the Redis client implementation to utilize these timeouts, enhancing resource management during unresponsive Redis instances.Also updates documentation in README.md to reflect these changes.
Test result: