Skip to content
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

When reconnecting redis, a callback data can be duplicated. #49

Open
dobeeisfree opened this issue Sep 2, 2019 · 0 comments
Open

When reconnecting redis, a callback data can be duplicated. #49

dobeeisfree opened this issue Sep 2, 2019 · 0 comments

Comments

@dobeeisfree
Copy link

dobeeisfree commented Sep 2, 2019

problem context

0. a client have two database index, so need to re_select()

the problem is here.

  1. when redis sentinel try to reconnect, called connection_disconnection_handler(...)

client::connection_disconnection_handler(network::redis_connection &) {

void
client::reconnect() {
/**
* increase the number of attempts to reconnect
*/
++m_current_reconnect_attempts;
/**
* We rely on the sentinel to tell us which redis server is currently the master.
*/
if (!m_master_name.empty() &&
!m_sentinel.get_master_addr_by_name(m_master_name, m_redis_server, m_redis_port, true)) {
if (m_connect_callback) {
m_connect_callback(m_redis_server, m_redis_port, connect_state::lookup_failed);
}
return;
}
/**
* Try catch block because the redis client throws an error if connection cannot be made.
*/
try {
connect(m_redis_server, m_redis_port, m_connect_callback, m_connect_timeout_ms, m_max_reconnects,
m_reconnect_interval_ms);
}
catch (...) {
}
if (!is_connected()) {
if (m_connect_callback) {
m_connect_callback(m_redis_server, m_redis_port, connect_state::failed);
}
return;
}
/**
* notify end
*/
if (m_connect_callback) {
m_connect_callback(m_redis_server, m_redis_port, connect_state::ok);
}
__CPP_REDIS_LOG(info, "client reconnected ok");
re_auth();
re_select();
resend_failed_commands();
try_commit();
}

  1. That can be successful, so internal m_commands pushed command_request by re_select() and m_buffer builded (by m_client.send(redis_cmd); )

client::unprotected_select(int index, const reply_callback_t &reply_callback) {
/**
* save the index of the database for reconnect attempts.
*/
m_database_index = index;
/**
* save command in the pipeline
*/
unprotected_send({"SELECT", std::to_string(index)}, reply_callback);
}

void
client::unprotected_send(const std::vector<std::string> &redis_cmd, const reply_callback_t &callback) {
m_client.send(redis_cmd);
m_commands.push({redis_cmd, callback});
}

  1. call next function resend_failed_commands()

m_commands is not empty, so ...

void
client::resend_failed_commands() {
if (m_commands.empty()) {
return;
}
/**
* dequeue commands and move them to a local variable
*/
std::queue<command_request> commands = std::move(m_commands);
while (!commands.empty()) {
/**
* Reissue the pending command and its callback.
*/
unprotected_send(commands.front().command, commands.front().callback);
commands.pop();
}
}

m_commands, this variable will still have the previously select(call by unprotected_select()) command

even if commands.pop(), this variable m_commandspushed again (by unprotected_send)

also, m_buffer have duplicate data..

"SELECT" cmd sended twice, so callback data can be weired.. (even if reconnecting successful) 🙀

  1. after try_commit() end of reconnect(), some commands request & response can pushed

but reply is not a current callback, the callback is previous old data when reconnecting (by re_select functions called)

solving problem

I suggest this code:

re_auth(); // => m_commands pushed
resend_failed_commands(); // build buffer from old command and m_commands pushed
re_select(); // build buffer from this new one!
try_commit();

original code:

re_auth();
re_select();
resend_failed_commands();
try_commit();

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant