-
Notifications
You must be signed in to change notification settings - Fork 82
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
FIX #28 #29
base: master
Are you sure you want to change the base?
FIX #28 #29
Conversation
[ADD] logs
retry; | ||
|
||
% Redis explicitly say our slot mapping is incorrect, we need to refresh | ||
% it | ||
{error, <<"MOVED ", _/binary>>} -> | ||
logger:error("eredis_cluster:handle_transaction_result <--> {error, MOVED} Result = ~",[Result]), |
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.
I think there is an issue here? ~p
Fixes adrienmo#28 and obsoletes PR adrienmo#29. Retry is implemented for query, but not for query_noreply. Perhaps it should.
Fixes adrienmo#28 and obsoletes PR adrienmo#29. Retry is implemented for query, but not for query_noreply. Perhaps it should.
Fixes adrienmo#28 and obsoletes PR adrienmo#29. Retry is implemented for query, but not for query_noreply. Perhaps it should.
Fixes adrienmo#28 and obsoletes PR adrienmo#29. Retry is implemented for query, but not for query_noreply. Perhaps it should.
query(full, _Commands) -> | ||
{error, retry}; |
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.
Actually, eredis_cluster
is using poolboy:transaction/2
which is using poolboy:checkout/3
with Block = true. Checkout can only return full
when Block =:= false. Therefore, if the pool is full, the transaction fun never gets called so this PR just adds dead code.
Instead, when the pool doesn't have workers available, we get an exit:{timeout, {gen_server, call, _}}
exception from poolboy:transaction/2. This is turned into {error, no_connection}
by the try-catch in eredis_cluster_pool and then it triggers a refesh mappings and then a retry in eredis_cluster
.
Refresh mappings isn't necessary in this case. A retry without refresh mappings would suffice. We have done it in the Nordix fork.
FIX #28 and added logs