Make Aerospike connection idle timeout configurable #118
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mansinahar
reviewed
Nov 8, 2022
config/backends.go
Outdated
| Password string `mapstructure:"password"` | ||
| MaxReadRetries int `mapstructure:"max_read_retries"` | ||
| MaxWriteRetries int `mapstructure:"max_write_retries"` | ||
| ConnectionIdleTimeout int `mapstructure:"connection_idle_timeout"` |
There was a problem hiding this comment.
@guscarreon Can we rename this to be connection_idle_timeout_sec to be clear about the time unit? Also, can you please add a comment for this config field along the lines of:
// Please set this to a value lower than the `proto-fd-idle-ms` (converted to seconds) value set in your Aerospike Server.
// This is to avoid having race conditions where the server closes the connection but the client still tries to use it.
// If set to a value less than or equal to 0, Aerospike Client's default value will be used which is 55 seconds.
There was a problem hiding this comment.
Makes sense. Great comment wording. Modified
config/backends.go
Outdated
| } | ||
|
|
||
| if cfg.ConnectionIdleTimeout > 0 { | ||
| log.Infof("config.backend.aerospike.connection_idle_timeout: %d. Will substitute Aerospike's default 55 seconds.", cfg.ConnectionIdleTimeout) |
There was a problem hiding this comment.
I believe we can skip this part of the log message: Will substitute Aerospike's default 55 seconds. as when someone's overriding that value, the value they have set is what we should log. Logging the default value that it will substitute won't help much IMHO.
mansinahar
previously approved these changes
Nov 8, 2022
AlexBVolcy
previously approved these changes
Nov 9, 2022
guscarreon
dismissed stale reviews from AlexBVolcy and mansinahar
via
97b5006
mansinahar
approved these changes
Nov 10, 2022
AlexBVolcy
approved these changes
Nov 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One of the Aerospike's client policy parameters is the maximum idle timeout for a connection. This pull requests makes Prebid Cache to be able to configure said paramenter.