From 6547a55eb7b1536241bbb062810aab988aec4c50 Mon Sep 17 00:00:00 2001 From: alexr-bq <72045206+alexr-bq@users.noreply.github.com> Date: Wed, 17 May 2023 19:36:39 -0700 Subject: [PATCH] Is multi writer removal (#152) * Add AWS Authentication parameters to DSN UI (#115) * Refactor MYSQL_PROXY (#118) * EFM PROXY * complete EFM PROXY * fix unit tests * fix include * generate node keys after setting next proxy for EFM_PROXY * fix unit tests * stop monitoring init() and options() * replace MYSQL_MONITOR_PROXY with MYSQL_PROXY * fix unit tests * stop monitoring ping() * stop monitoring ssl_set and option4 * stop monitoring autocommit * comment out aws code * stop monitoring client_find_plugin * newline at eof * free monitor connection before getting a new one * fix unit tests * test * disable failover and monitoring for monitor thread * fix memory leak in unit test * needs connection handler for monitor * add null check in monitor service start monitoring * fix env handle still used after deleting * comment out mysql_library_init * move connection handler to its own header file * Further refactor MYSQL_PROXY (#119) * create DUMMY_PROXY to handle mysql library calls, refactor MYSQL_PROXY to only forward proxy call * remove commented include * include thread header in dummy proxy * rename mysql_proxy to connection_proxy * rename dummy proxy to mysql proxy * remove secrets manager proxy as it should be a different PR * include in alphabetical order * AWS SDK helper class (#123) * Implement IAM Authentication (#120) * Add Authentication Parameters to ConnectionStringBuilder (#124) * Secrets Manager Proxy (#121) * new secrets_manager files * working secrets manager client * add cache for secrets manager proxy * first unit test for secrets manager proxy * added more unit tests * fix unique lock missing template arguments * fix multiple calls to aws api in secrets manager unit tests * fix multiple aws init/shutdown api calls in integration tests * fix calling init/shutdownApi in windows * verbose log for windows community tests * wrap initapi and shutdownapi inside secrets manager proxy contructor/destructor * move init/shutdown aws api to alloc/free env handle * move init/shutdown aws api to alloc/free env handle * test * test * fix static variables * fix atomic int init * fix unit tests * renaming * cleanup * introduce aws sdk helpoer * address comments * nit * fix rebase * fix rebase * set error * fix build * better error message * Secrets manager integration test (#127) * create secrets in java util * simple sm test * simplify setup * fix integration * comment out toxiporxy * comment out toxiporxy * sm test not inherit from base anymore * fix build * fix set error message, fix dsn not picked up * add {} in connection string * fix curly braces * fix log * fix build * more logging * more logging * fix test * better error handling * uncomment * address comments * revert * IAM Authentication Integration Tests (#126) * Refactor Integration Tests (#128) * Fix installer (#129) * fix minor version exceeding 255, remove lib folder for installer * remove setup lib from installer * add aws sdk component in wix * build aws sdk on release action * workflow dispatch * bundle aws sdk when doing cpack on mac and linux * fix mac installer aws dependencies * Adjust DSN config UI for AWS Authentication (#130) * Documentation for AWS Authentication (#131) * rename windows installer (#132) * Update license (#133) * update license * ignore file change in license * update license in build script * Parse Region from Secret if User does not provide Region (#134) * Fix SQLCancel not going through proxy (#135) * fix SQLCancel not going through proxy * cleanup ds during dbc clone * spacing * Fix typo in doc * Implement ENABLE_STRICT_READER_FAILOVER user parameter (#136) * Override change_user() in IAM/Secrets Manager proxy (#137) * override change_user() in IAM/Secrets Manager proxy * better naming * Fix failover timeout not obeyed (#138) * replace async future with packed_task and thread to avoid blocking future destructor * fix unit tests * add template arguments for std::packaged_task * missing template arguments * use thread pool to run failover and wait for all when driver unloads, fix logger * increase thread pool size * fix reader failover where failed result overwrite successful result * not stop thread pool on free env handle * redirect maven central and stop thread pool in teardown * maven central * add thread id to logs * move future valid check before wait for * move future valid check before wait_for() for writer failover * mark logger as extern * test diable logs * test failover_integration_test first * test * debug gh action * make env var available for debug session * debug in docker * refactor integration tests to use shared pointer for rds client * make rds client back to non static * fix build * reset rds client in teardown * Revert "refactor integration tests to use shared pointer for rds client" This reverts commit 2ec1ae5e95f2257166836b439e9ae4d189084317. * disable remote debug * fix build * debug * specifically call thread pool stop in myodbc_end * move failover thread pool inside env * fix unit test * fix build * cleanup * China endpoint support (#140) * china endpoint support * nit * Verify Writer Cluster Connections (#139) * Fix shadowing member variables causing null pointer exception (#148) * fix some shadowing member variables casuing null pointer exception * extend perf tests aws credentials duration from 3 to 4 hours * workaround mysqlclient * fix zlib not found * fix mac build * bump mysql version * fix mac build * bump github action runner macos versiooon * update download link * fix build * install openssl1.1 for macos13 * debug * debug * fall back to macos 11 * change macos version to 12 * fix build * fix build * Fix monitor timeout (#149) * fix monitor timeout * fix login timeout and unit tests * pass monitor flag * rename a component that may have name collision issue on mac (#151) * Remove is_multi_writer flag and check for last updated writer * Fix tests * Remove uneeded line --------- Co-authored-by: justing-bq <62349012+justing-bq@users.noreply.github.com> Co-authored-by: Yan Wang <68562925+yanw-bq@users.noreply.github.com> Co-authored-by: justing-bq Co-authored-by: Alex Rehnby-Martin --- driver/cluster_topology_info.cc | 3 +-- driver/cluster_topology_info.h | 1 - driver/failover_handler.cc | 4 +--- driver/topology_service.cc | 22 +++++++++++-------- unit_testing/failover_handler_test.cc | 31 --------------------------- unit_testing/topology_service_test.cc | 19 +++++++--------- 6 files changed, 23 insertions(+), 57 deletions(-) diff --git a/driver/cluster_topology_info.cc b/driver/cluster_topology_info.cc index f995fa6cc..303d6fc19 100644 --- a/driver/cluster_topology_info.cc +++ b/driver/cluster_topology_info.cc @@ -49,8 +49,7 @@ CLUSTER_TOPOLOGY_INFO::CLUSTER_TOPOLOGY_INFO() { CLUSTER_TOPOLOGY_INFO::CLUSTER_TOPOLOGY_INFO(const CLUSTER_TOPOLOGY_INFO& src_info) : current_reader{src_info.current_reader}, last_updated{src_info.last_updated}, - last_used_reader{src_info.last_used_reader}, - is_multi_writer_cluster{src_info.is_multi_writer_cluster} { + last_used_reader{src_info.last_used_reader} { for (auto host_info_source : src_info.writers) { writers.push_back(std::make_shared(*host_info_source)); //default copy } diff --git a/driver/cluster_topology_info.h b/driver/cluster_topology_info.h index fe50f2f97..90d840370 100644 --- a/driver/cluster_topology_info.h +++ b/driver/cluster_topology_info.h @@ -58,7 +58,6 @@ class CLUSTER_TOPOLOGY_INFO { std::shared_ptr get_reader(int i); std::vector> get_writers(); std::vector> get_readers(); - bool is_multi_writer_cluster = false; private: int current_reader = -1; diff --git a/driver/failover_handler.cc b/driver/failover_handler.cc index 94966301a..5989d3e11 100644 --- a/driver/failover_handler.cc +++ b/driver/failover_handler.cc @@ -521,8 +521,7 @@ bool FAILOVER_HANDLER::is_failover_enabled() { return (dbc != nullptr && ds != nullptr && ds->enable_cluster_failover && m_is_cluster_topology_available && - !m_is_rds_proxy && - !m_is_multi_writer_cluster); + !m_is_rds_proxy); } bool FAILOVER_HANDLER::is_rds() { return m_is_rds; } @@ -537,7 +536,6 @@ void FAILOVER_HANDLER::initialize_topology() { current_topology = topology_service->get_topology(dbc->connection_proxy, false); if (current_topology) { - m_is_multi_writer_cluster = current_topology->is_multi_writer_cluster; m_is_cluster_topology_available = current_topology->total_hosts() > 0; MYLOG_DBC_TRACE(dbc, "[FAILOVER_HANDLER] m_is_cluster_topology_available=%s", diff --git a/driver/topology_service.cc b/driver/topology_service.cc index 4ef132b8e..265dee136 100644 --- a/driver/topology_service.cc +++ b/driver/topology_service.cc @@ -279,27 +279,31 @@ std::shared_ptr TOPOLOGY_SERVICE::query_for_topology(CONN topology_info = std::make_shared(); std::map> instances; MYSQL_ROW row; - int writer_count = 0; + std::shared_ptr latest_writer = nullptr; while ((row = connection_proxy->fetch_row(result))) { std::shared_ptr host_info = create_host(row); if (host_info) { - // Only mark the first/latest writer as true writer if (host_info->is_host_writer()) { - if (writer_count > 0) { - host_info->mark_as_writer(false); + // Only mark the latest writer as true writer. Ignore other writers (possible stale records, multi-writer not supported) + // Date in lexographic order, so str comparison works for most recent date + if (!latest_writer || host_info->last_updated > latest_writer->last_updated) { + latest_writer = host_info; } - writer_count++; } - // Add to topology if instance not seen before - if (!TOPOLOGY_SERVICE::does_instance_exist(instances, host_info)) { + else if (!TOPOLOGY_SERVICE::does_instance_exist(instances, host_info)) { + // Add readers to topology if instance not seen before topology_info->add_host(host_info); } } } + if (latest_writer && !TOPOLOGY_SERVICE::does_instance_exist(instances, latest_writer)) + { + topology_info->add_host(latest_writer); + } + connection_proxy->free_result(result); - topology_info->is_multi_writer_cluster = writer_count > 1; - if (writer_count == 0) { + if (!latest_writer) { // No writer found MYLOG_TRACE(logger, dbc_id, "[TOPOLOGY_SERVICE] The topology query returned an " "invalid topology - no writer instance detected"); diff --git a/unit_testing/failover_handler_test.cc b/unit_testing/failover_handler_test.cc index d4a72434b..832408cfb 100644 --- a/unit_testing/failover_handler_test.cc +++ b/unit_testing/failover_handler_test.cc @@ -345,37 +345,6 @@ TEST_F(FailoverHandlerTest, RDS_ReaderCluster) { EXPECT_TRUE(failover_handler.is_failover_enabled()); } -TEST_F(FailoverHandlerTest, RDS_MultiWriterCluster) { - SQLCHAR server[] = "my-cluster-name.cluster-XYZ.us-east-2.rds.amazonaws.com"; - - ds_setattr_from_utf8(&ds->server, server); - ds->port = 1234; - ds->enable_cluster_failover = true; - - EXPECT_CALL(*mock_connection_handler, do_connect_impl(dbc, ds, false, false)).WillOnce(Return(SQL_SUCCESS)); - - auto multi_writer_topology = std::make_shared(); - multi_writer_topology->add_host(writer_host); - multi_writer_topology->add_host(reader_host); - multi_writer_topology->is_multi_writer_cluster = true; - - EXPECT_CALL(*mock_ts, get_topology(_, false)) - .WillOnce(Return(multi_writer_topology)); - EXPECT_CALL(*mock_ts, set_cluster_instance_template(_)).Times(AtLeast(1)); - EXPECT_CALL(*mock_ts, - set_cluster_id(StrEq( - "my-cluster-name.cluster-XYZ.us-east-2.rds.amazonaws.com:1234"))) - .Times(AtLeast(1)); - - FAILOVER_HANDLER failover_handler(dbc, ds, mock_connection_handler, mock_ts, mock_metrics); - failover_handler.init_connection(); - - EXPECT_TRUE(failover_handler.is_rds()); - EXPECT_FALSE(failover_handler.is_rds_proxy()); - EXPECT_TRUE(failover_handler.is_cluster_topology_available()); - EXPECT_FALSE(failover_handler.is_failover_enabled()); -} - TEST_F(FailoverHandlerTest, ReconnectWithFailoverSettings) { SQLCHAR server[] = "10.10.10.10"; SQLCHAR host_pattern[] = "?.my-custom-domain.com"; diff --git a/unit_testing/topology_service_test.cc b/unit_testing/topology_service_test.cc index 6db9d5035..bc21b5372 100644 --- a/unit_testing/topology_service_test.cc +++ b/unit_testing/topology_service_test.cc @@ -51,8 +51,8 @@ namespace { char* reader2[4] = { "replica-instance-2", "Replica", "2020-09-15 17:51:53.0", "13.5" }; char* writer[4] = { "writer-instance", WRITER_SESSION_ID, "2020-09-15 17:51:53.0", "13.5" }; char* writer1[4] = { "writer-instance-1", WRITER_SESSION_ID, "2020-09-15 17:51:53.0", "13.5" }; - char* writer2[4] = { "writer-instance-2", WRITER_SESSION_ID, "2020-09-15 17:51:53.0", "13.5" }; - char* writer3[4] = { "writer-instance-3", WRITER_SESSION_ID, "2020-09-15 17:51:53.0", "13.5" }; + char* writer2[4] = { "writer-instance-2", WRITER_SESSION_ID, "2020-09-15 17:51:54.0", "13.5" }; + char* writer3[4] = { "writer-instance-3", WRITER_SESSION_ID, "2020-09-15 17:51:55.0", "13.5" }; } // namespace class TopologyServiceTest : public testing::Test { @@ -101,7 +101,6 @@ TEST_F(TopologyServiceTest, TopologyQuery) { std::shared_ptr topology = ts->get_topology(mock_proxy); ASSERT_NE(nullptr, topology); - EXPECT_FALSE(topology->is_multi_writer_cluster); EXPECT_EQ(3, topology->total_hosts()); EXPECT_EQ(2, topology->num_readers()); @@ -116,7 +115,7 @@ TEST_F(TopologyServiceTest, TopologyQuery) { EXPECT_EQ("13.5", writer_host->replica_lag); } -TEST_F(TopologyServiceTest, MultiWriter) { +TEST_F(TopologyServiceTest, StaleRecord) { EXPECT_CALL(*mock_proxy, query(StrEq(RETRIEVE_TOPOLOGY_SQL))) .WillRepeatedly(Return(0)); EXPECT_CALL(*mock_proxy, fetch_row(_)) @@ -128,18 +127,17 @@ TEST_F(TopologyServiceTest, MultiWriter) { std::shared_ptr topology = ts->get_topology(mock_proxy); ASSERT_NE(nullptr, topology); - EXPECT_TRUE(topology->is_multi_writer_cluster); - EXPECT_EQ(3, topology->total_hosts()); - EXPECT_EQ(2, topology->num_readers()); // 2 writers are marked as readers + EXPECT_EQ(1, topology->total_hosts()); + EXPECT_EQ(0, topology->num_readers()); std::shared_ptr writer_host = topology->get_writer(); ASSERT_NE(nullptr, writer_host); - EXPECT_EQ("writer-instance-1.XYZ.us-east-2.rds.amazonaws.com", writer_host->get_host()); + EXPECT_EQ("writer-instance-3.XYZ.us-east-2.rds.amazonaws.com", writer_host->get_host()); EXPECT_EQ(1234, writer_host->get_port()); - EXPECT_EQ("writer-instance-1", writer_host->instance_name); + EXPECT_EQ("writer-instance-3", writer_host->instance_name); EXPECT_EQ(WRITER_SESSION_ID, writer_host->session_id); - EXPECT_EQ("2020-09-15 17:51:53.0", writer_host->last_updated); + EXPECT_EQ("2020-09-15 17:51:55.0", writer_host->last_updated); // Only latest updated writer is counted EXPECT_EQ("13.5", writer_host->replica_lag); } @@ -155,7 +153,6 @@ TEST_F(TopologyServiceTest, DuplicateInstances) { std::shared_ptr topology = ts->get_topology(mock_proxy); ASSERT_NE(nullptr, topology); - EXPECT_TRUE(topology->is_multi_writer_cluster); EXPECT_EQ(1, topology->total_hosts()); EXPECT_EQ(0, topology->num_readers());