Fix monitor timeout (#149)

* fix monitor timeout * fix login timeout and unit tests * pass monitor flag
aws · May 18, 2023 · 9c136a6 · 9c136a6
1 parent 6e80ed8
commit 9c136a6
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 79 deletions.
diff --git a/driver/connect.cc b/driver/connect.cc
@@ -326,7 +326,7 @@ class dbc_guard
   @return Standard SQLRETURN code. If it is @c SQL_SUCCESS or @c
   SQL_SUCCESS_WITH_INFO, a connection has been established.
 */
-SQLRETURN DBC::connect(DataSource *dsrc, bool failover_enabled)
+SQLRETURN DBC::connect(DataSource *dsrc, bool failover_enabled, bool is_monitor_connection)
 {
   SQLRETURN rc = SQL_SUCCESS;
   unsigned long flags;
@@ -385,7 +385,12 @@ SQLRETURN DBC::connect(DataSource *dsrc, bool failover_enabled)
       read_timeout = get_network_timeout(dsrc->network_timeout);
       write_timeout = read_timeout;
   }
-  else
+  else if (is_monitor_connection)
+  {
+      connect_timeout = get_network_timeout(dsrc->read_timeout);
+      read_timeout = get_network_timeout(dsrc->read_timeout);
+      write_timeout = get_network_timeout(dsrc->write_timeout);
+  } else
   {
       connect_timeout = get_connect_timeout(login_timeout);
       read_timeout = get_network_timeout(dsrc->read_timeout);

diff --git a/driver/connection_handler.cc b/driver/connection_handler.cc
@@ -56,11 +56,11 @@ CONNECTION_HANDLER::CONNECTION_HANDLER(DBC* dbc) : dbc{dbc} {}
 
 CONNECTION_HANDLER::~CONNECTION_HANDLER() = default;
 
-SQLRETURN CONNECTION_HANDLER::do_connect(DBC* dbc_ptr, DataSource* ds, bool failover_enabled) {
-    return dbc_ptr->connect(ds, failover_enabled);
+SQLRETURN CONNECTION_HANDLER::do_connect(DBC* dbc_ptr, DataSource* ds, bool failover_enabled, bool is_monitor_connection) {
+    return dbc_ptr->connect(ds, failover_enabled, is_monitor_connection);
 }
 
-CONNECTION_PROXY* CONNECTION_HANDLER::connect(std::shared_ptr<HOST_INFO> host_info, DataSource* ds) {
+CONNECTION_PROXY* CONNECTION_HANDLER::connect(std::shared_ptr<HOST_INFO> host_info, DataSource* ds, bool is_monitor_connection) {
 
     if (dbc == nullptr || host_info == nullptr) {
         return nullptr;
@@ -76,7 +76,7 @@ CONNECTION_PROXY* CONNECTION_HANDLER::connect(std::shared_ptr<HOST_INFO> host_in
 
     CONNECTION_PROXY* new_connection = nullptr;
     CLEAR_DBC_ERROR(dbc_clone);
-    const SQLRETURN rc = do_connect(dbc_clone, ds_to_use, ds_to_use->enable_cluster_failover);
+    const SQLRETURN rc = do_connect(dbc_clone, ds_to_use, ds_to_use->enable_cluster_failover, is_monitor_connection);
 
     if (rc == SQL_SUCCESS || rc == SQL_SUCCESS_WITH_INFO) {
         new_connection = dbc_clone->connection_proxy;

diff --git a/driver/connection_handler.h b/driver/connection_handler.h
@@ -51,8 +51,8 @@ class CONNECTION_HANDLER {
         CONNECTION_HANDLER(DBC* dbc);
         virtual ~CONNECTION_HANDLER();
 
-        virtual SQLRETURN do_connect(DBC* dbc_ptr, DataSource* ds, bool failover_enabled);
-        virtual CONNECTION_PROXY* connect(std::shared_ptr<HOST_INFO> host_info, DataSource* ds);
+        virtual SQLRETURN do_connect(DBC* dbc_ptr, DataSource* ds, bool failover_enabled, bool is_monitor_connection = false);
+        virtual CONNECTION_PROXY* connect(std::shared_ptr<HOST_INFO> host_info, DataSource* ds, bool is_monitor_connection = false);
         void update_connection(CONNECTION_PROXY* new_connection, const std::string& new_host_name);
 
     private:

diff --git a/driver/driver.h b/driver/driver.h
@@ -656,7 +656,7 @@ struct DBC
   void remove_desc(DESC *desc);
   SQLRETURN set_error(char *state, const char *message, uint errcode);
   SQLRETURN set_error(char *state);
-  SQLRETURN connect(DataSource *dsrc, bool failover_enabled);
+  SQLRETURN connect(DataSource *dsrc, bool failover_enabled, bool is_monitor_connection = false);
   void execute_prep_stmt(MYSQL_STMT *pstmt, std::string &query,
     MYSQL_BIND *param_bind, MYSQL_BIND *result_bind);
   void init_proxy_chain(DataSource *dsrc);

diff --git a/driver/monitor.cc b/driver/monitor.cc
@@ -226,12 +226,12 @@ bool MONITOR::connect() {
     } else {
         // cannot change login_timeout here because no access to dbc
         this->ds->read_timeout = timeout_sec;
+        this->ds->write_timeout = timeout_sec;
     }
 
-    this->ds->enable_cluster_failover = false;
     this->ds->enable_failure_detection= false;
 
-    this->connection_proxy = this->connection_handler->connect(this->host, this->ds);
+    this->connection_proxy = this->connection_handler->connect(this->host, this->ds, true);
     if (!this->connection_proxy) {
         return false;
     }

diff --git a/unit_testing/failover_handler_test.cc b/unit_testing/failover_handler_test.cc
@@ -115,9 +115,9 @@ TEST_F(FailoverHandlerTest, CustomDomain) {
         .Times(AtLeast(1));
     EXPECT_CALL(*mock_ts, set_cluster_id(_)).Times(0);
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     FAILOVER_HANDLER failover_handler(dbc, ds, mock_connection_handler, mock_ts, mock_metrics);
@@ -132,7 +132,7 @@ TEST_F(FailoverHandlerTest, CustomDomain) {
 TEST_F(FailoverHandlerTest, FailoverDisabled) {
     ds->enable_cluster_failover = false;
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false)).Times(1);
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false)).Times(1);
 
     FAILOVER_HANDLER failover_handler(dbc, ds, mock_connection_handler, mock_ts, mock_metrics);
     failover_handler.init_connection();
@@ -145,7 +145,7 @@ TEST_F(FailoverHandlerTest, IP_TopologyAvailable_PatternRequired) {
 
     ds_setattr_from_utf8(&ds->server, server);
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -159,7 +159,7 @@ TEST_F(FailoverHandlerTest, IP_TopologyNotAvailable) {
 
     ds_setattr_from_utf8(&ds->server, server);
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false))
@@ -181,9 +181,9 @@ TEST_F(FailoverHandlerTest, IP_Cluster) {
     ds_setattr_from_utf8(&ds->server, server);
     ds_setattr_from_utf8(&ds->host_pattern, host_pattern);
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -208,9 +208,9 @@ TEST_F(FailoverHandlerTest, IP_Cluster_ClusterID) {
     ds_setattr_from_utf8(&ds->host_pattern, host_pattern);
     ds_setattr_from_utf8(&ds->cluster_id, cluster_id);
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -232,9 +232,9 @@ TEST_F(FailoverHandlerTest, RDS_Cluster) {
     ds_setattr_from_utf8(&ds->server, server);
     ds->port = 1234;
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -255,9 +255,9 @@ TEST_F(FailoverHandlerTest, RDS_CustomCluster) {
 
     ds_setattr_from_utf8(&ds->server, server);
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -278,9 +278,9 @@ TEST_F(FailoverHandlerTest, RDS_Instance) {
 
     ds_setattr_from_utf8(&ds->server, server);
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -302,7 +302,7 @@ TEST_F(FailoverHandlerTest, RDS_Proxy) {
     ds_setattr_from_utf8(&ds->server, server);
     ds->port = 1234;
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -324,9 +324,9 @@ TEST_F(FailoverHandlerTest, RDS_ReaderCluster) {
     ds_setattr_from_utf8(&ds->server, server);
     ds->port = 1234;
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     EXPECT_CALL(*mock_ts, get_topology(_, false)).WillOnce(Return(topology));
@@ -352,7 +352,7 @@ TEST_F(FailoverHandlerTest, RDS_MultiWriterCluster) {
     ds->port = 1234;
     ds->enable_cluster_failover = true;
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false)).WillOnce(Return(SQL_SUCCESS));
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false)).WillOnce(Return(SQL_SUCCESS));
 
     auto multi_writer_topology = std::make_shared<CLUSTER_TOPOLOGY_INFO>();
     multi_writer_topology->add_host(writer_host);
@@ -389,10 +389,10 @@ TEST_F(FailoverHandlerTest, ReconnectWithFailoverSettings) {
     EXPECT_CALL(*mock_ts, set_cluster_instance_template(_))
         .Times(AtLeast(1));
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, false))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false))
         .WillOnce(Return(SQL_SUCCESS));
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, true))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, true, false))
         .WillOnce(Return(SQL_SUCCESS));
 
     FAILOVER_HANDLER failover_handler(dbc, ds, mock_connection_handler, mock_ts, mock_metrics);
@@ -490,7 +490,7 @@ TEST_F(FailoverHandlerTest, GetRdsClusterHostUrl) {
 TEST_F(FailoverHandlerTest, ConnectToNewWriter) {
     SQLCHAR server[] = "my-cluster-name.cluster-XYZ.us-east-2.rds.amazonaws.com";
 
-    EXPECT_CALL(*mock_connection_handler, do_connect(dbc, ds, _))
+    EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, _, false))
         .WillOnce(Return(SQL_SUCCESS))
         .WillOnce(Return(SQL_SUCCESS));
 

diff --git a/unit_testing/failover_reader_handler_test.cc b/unit_testing/failover_reader_handler_test.cc
@@ -223,7 +223,7 @@ TEST_F(FailoverReaderHandlerTest, BuildHostsList) {
 TEST_F(FailoverReaderHandlerTest, GetConnectionFromHosts_Failure) {
     EXPECT_CALL(*mock_ts, get_topology(_, true)).WillRepeatedly(Return(topology));
 
-    EXPECT_CALL(*mock_connection_handler, connect(_, nullptr)).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(_, nullptr, false)).WillRepeatedly(Return(nullptr));
 
     EXPECT_CALL(*mock_ts, mark_host_down(reader_a_host)).Times(1);
     EXPECT_CALL(*mock_ts, mark_host_down(reader_b_host)).Times(1);
@@ -247,8 +247,8 @@ TEST_F(FailoverReaderHandlerTest, GetConnectionFromHosts_Success_Reader) {
 
     EXPECT_CALL(*mock_reader_a_proxy, is_connected()).WillRepeatedly(Return(true));
 
-    EXPECT_CALL(*mock_connection_handler, connect(_, nullptr)).WillRepeatedly(Return(nullptr));    
-    EXPECT_CALL(*mock_connection_handler, connect(reader_a_host, nullptr)).WillRepeatedly(Return(mock_reader_a_proxy));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(_, nullptr, false)).WillRepeatedly(Return(nullptr));    
+    EXPECT_CALL(*mock_connection_handler, connect_impl(reader_a_host, nullptr, false)).WillRepeatedly(Return(mock_reader_a_proxy));
 
     // Reader C will not be used as it is put at the end. Will only try to connect to A and B
     EXPECT_CALL(*mock_ts, mark_host_up(reader_a_host)).Times(1);
@@ -274,8 +274,8 @@ TEST_F(FailoverReaderHandlerTest, GetConnectionFromHosts_Success_Writer) {
 
     EXPECT_CALL(*mock_writer_proxy, is_connected()).WillRepeatedly(Return(true));
 
-    EXPECT_CALL(*mock_connection_handler, connect(_, nullptr)).WillRepeatedly(Return(nullptr));
-    EXPECT_CALL(*mock_connection_handler, connect(writer_host, nullptr)).WillRepeatedly(Return(mock_writer_proxy));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(_, nullptr, false)).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(writer_host, nullptr, false)).WillRepeatedly(Return(mock_writer_proxy));
 
     EXPECT_CALL(*mock_ts, mark_host_up(writer_host)).Times(1);
 
@@ -309,9 +309,9 @@ TEST_F(FailoverReaderHandlerTest, GetConnectionFromHosts_FastestHost) {
     EXPECT_CALL(*mock_reader_a_proxy, is_connected()).WillRepeatedly(Return(true));
     EXPECT_CALL(*mock_reader_b_proxy, is_connected()).WillRepeatedly(Return(true));
 
-    EXPECT_CALL(*mock_connection_handler, connect(_, nullptr)).WillRepeatedly(Return(nullptr));
-    EXPECT_CALL(*mock_connection_handler, connect(reader_a_host, nullptr)).WillRepeatedly(Return(mock_reader_a_proxy));
-    EXPECT_CALL(*mock_connection_handler, connect(reader_b_host, nullptr)).WillRepeatedly(Invoke([&]() {
+    EXPECT_CALL(*mock_connection_handler, connect_impl(_, nullptr, false)).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(reader_a_host, nullptr, false)).WillRepeatedly(Return(mock_reader_a_proxy));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(reader_b_host, nullptr, false)).WillRepeatedly(Invoke([&]() {
             std::this_thread::sleep_for(std::chrono::milliseconds(5000));
             return mock_reader_b_proxy;
         }));
@@ -342,12 +342,12 @@ TEST_F(FailoverReaderHandlerTest, GetConnectionFromHosts_Timeout) {
     EXPECT_CALL(*mock_reader_b_proxy, is_connected()).WillRepeatedly(Return(true));
     EXPECT_CALL(*mock_reader_b_proxy, mock_connection_proxy_destructor());
 
-    EXPECT_CALL(*mock_connection_handler, connect(_, nullptr)).WillRepeatedly(Return(nullptr));
-    EXPECT_CALL(*mock_connection_handler, connect(reader_a_host, nullptr)).WillRepeatedly(Invoke([&]() {
+    EXPECT_CALL(*mock_connection_handler, connect_impl(_, nullptr, false)).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(reader_a_host, nullptr, false)).WillRepeatedly(Invoke([&]() {
             std::this_thread::sleep_for(std::chrono::milliseconds(5000));
             return mock_reader_a_proxy;
         }));
-    EXPECT_CALL(*mock_connection_handler, connect(reader_b_host, nullptr)).WillRepeatedly(Invoke([&]() {
+    EXPECT_CALL(*mock_connection_handler, connect_impl(reader_b_host, nullptr, false)).WillRepeatedly(Invoke([&]() {
             std::this_thread::sleep_for(std::chrono::milliseconds(5000));
             return mock_reader_b_proxy;
         }));
@@ -368,7 +368,7 @@ TEST_F(FailoverReaderHandlerTest, GetConnectionFromHosts_Timeout) {
 TEST_F(FailoverReaderHandlerTest, Failover_Failure) {
     EXPECT_CALL(*mock_ts, get_topology(_, true)).WillRepeatedly(Return(topology));
 
-    EXPECT_CALL(*mock_connection_handler, connect(_, nullptr)).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(_, nullptr, false)).WillRepeatedly(Return(nullptr));
 
     EXPECT_CALL(*mock_ts, mark_host_down(reader_a_host)).Times(AtLeast(1));
     EXPECT_CALL(*mock_ts, mark_host_down(reader_b_host)).Times(AtLeast(1));
@@ -404,10 +404,10 @@ TEST_F(FailoverReaderHandlerTest, Failover_Success_Reader) {
     EXPECT_CALL(*mock_reader_b_proxy, is_connected()).WillRepeatedly(Return(true));
     EXPECT_CALL(*mock_reader_b_proxy, mock_connection_proxy_destructor());
 
-    EXPECT_CALL(*mock_connection_handler, connect(_, nullptr)).WillRepeatedly(Return(nullptr));
-    EXPECT_CALL(*mock_connection_handler, connect(reader_a_host, nullptr)).WillRepeatedly(
+    EXPECT_CALL(*mock_connection_handler, connect_impl(_, nullptr, false)).WillRepeatedly(Return(nullptr));
+    EXPECT_CALL(*mock_connection_handler, connect_impl(reader_a_host, nullptr, false)).WillRepeatedly(
         Return(mock_reader_a_proxy));
-    EXPECT_CALL(*mock_connection_handler, connect(reader_b_host, nullptr)).WillRepeatedly(Invoke([=]() {
+    EXPECT_CALL(*mock_connection_handler, connect_impl(reader_b_host, nullptr, false)).WillRepeatedly(Invoke([=]() {
         std::this_thread::sleep_for(std::chrono::milliseconds(5000));
         return mock_reader_b_proxy;
     }));