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

dobeeisfree · 2019-09-02T18:04:23Z

problem context

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

the problem is here.

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

cpp_redis/sources/core/client.cpp

Line 330 in d1fc9c7

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

cpp_redis/sources/core/client.cpp

Lines 433 to 482 in d1fc9c7

    
           	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(); 
        
           	}

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

cpp_redis/sources/core/client.cpp

Lines 1949 to 1958 in d1fc9c7

    
           	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); 
        
           	}

cpp_redis/sources/core/client.cpp

Lines 194 to 198 in d1fc9c7

    
           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}); 
        
           }

call next function resend_failed_commands()

m_commands is not empty, so ...

cpp_redis/sources/core/client.cpp

Lines 308 to 327 in d1fc9c7

    
           	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) 🙀

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:

cpp_redis/sources/core/client.cpp

Lines 478 to 481 in d1fc9c7

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

cpp-redis / cpp_redis

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

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

dobeeisfree commented Sep 2, 2019 •

edited

cpp-redis / cpp_redis

Join GitHub today

GitHub is where the world builds software

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

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

Comments

dobeeisfree commented Sep 2, 2019 • edited

problem context

solving problem

Essential cookies

Always active

Analytics cookies

dobeeisfree commented Sep 2, 2019 •

edited