Define and use notWaitForRetryTimeout #82
Conversation
|
Is there a way to avoid these Travis CI build's errors? |
|
LGTM |
|
Thanks, @ariesdevil ! |
| @@ -346,7 +353,7 @@ func (client *Client) newConn() (*conn, error) { | |||
| ) | |||
|
|
|||
| cn.configHashFunction(int(hashFunctionMapping[client.hashFunc])) | |||
| if client.retryTimeout > 0 { | |||
| if client.retryTimeout > 0 || client.notWaitForRetryTimeout { | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Oh.. I see. Why not just change client.retryTimeout > 0 to client.retryTimeout >= 0 ?
There was a problem hiding this comment.
If client.ConfigTimeout (golibmc.RetryTimeout, ~) is not called, client.RetryTimeout is 0, but in this case it was set to 5 which is default value until this PR was merged It was.
If I make a fix like > =, we have implemented another function, as it will cause a change to the user that used to do so.
There was a problem hiding this comment.
If client.ConfigTimeout (golibmc.RetryTimeout, ~) is not called, client.RetryTimeout is 0, but in this case it was set to 5 which is default value until this PR was merged It was.
I don't understand why you said it is 0. If not called, it should be 5 (#define MC_DEFAULT_RETRY_TIMEOUT 5). After the PR is merged, it's still 5.
It looks like because of a bug introduced in 3b595db#diff-b21c9a32b7563e26c49d7f215bc5a656L584 . After that commit, the .ConfigTimeout function is not working anymore if called after the connection is established since timeout values won't be passed into the C++ layer. Actually the .ConfigTimeout has never been used in the golang code anymore, so how did you guys override the timeout values?
I think changing this to>= is acceptable and compatible with previous releases as the default behaviour is not changed at all.
There was a problem hiding this comment.
When implementing a connection pool, I fixed ConfigTimeout so that C. client_config will be called at the time when a new connection is created.
There was a problem hiding this comment.
@jumpeiMano how about init client.RetryTimeout to MC_DEFAULT_RETRY_TIMEOUT? and "just change client.retryTimeout > 0 to client.retryTimeout >= 0"。I also think notWaitForRetryTimeout is not necessary。
ChangeLog: - use pickle protocol version 2 (#52) - fix rvalue reference (#53) - Fix tests (#55) - golibmc_test: Fix fragile test case (#56) - Use connection pool in golibmc(again) (#57) - golibmc Quit ret err (#59) - cmake: Incorporate gtest in a standard way (#58) - Rename refactor (#60) - sync rapidjson/itoa.h upstream fix (#65) - split cppcheck (#67) - split FSM_GET_BYTES_CAS (#68) - Avoid noisy recv_err when broadcast quit (#73) - Try to support updating some servers without affecting others (#74) - make normalize_key cpdef & add warnings for unpickle unmarshal fail (#66) - fix fcntl usage when set socket nonblock (#69) - minor fix (#61) - add condition log macro (#77) - Define and use notWaitForRetryTimeout (#82) - upgrade travis to gcc 7 (#91) - add func errCodeToString (#79) - Introduce a reconnect mechanism in waitPoll (#88)
Reference: #81