From b313c6d0e750c556676147bc35b357350ea42422 Mon Sep 17 00:00:00 2001 From: Johan Wikman Date: Tue, 7 May 2019 16:04:34 +0300 Subject: [PATCH 1/4] MXS-2474 Ignore attempts to re-register a housekeeper task It is an error to register the same task multiple times, but for a maintenance release it is simpler and less risky to simply ignore an attempt (that BLR does) to do that. Allowing a task to be registered anew causes behaviour akin to a leak. --- server/core/housekeeper.cc | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/server/core/housekeeper.cc b/server/core/housekeeper.cc index 665a51ceb..aeb544825 100644 --- a/server/core/housekeeper.cc +++ b/server/core/housekeeper.cc @@ -12,6 +12,7 @@ */ #include +#include #include #include #include @@ -205,7 +206,29 @@ void Housekeeper::stop() void Housekeeper::add(const Task& task) { std::lock_guard guard(m_lock); - m_tasks.push_back(task); + + auto i = std::find_if(m_tasks.begin(), m_tasks.end(), Task::NameMatch(task.name)); + if (i == m_tasks.end()) + { + m_tasks.push_back(task); + } + else + { + const Task& existing = *i; + + bool identical = false; + if (task.func == existing.func + && task.data == existing.data + && task.frequency == existing.frequency) + { + identical = true; + } + + MXS_INFO("Housekeeper task `%s` added anew, all settings %s identical. " + "Second attempt to add is ignored.", + identical ? "ARE" : "are NOT", + task.name.c_str()); + } } void Housekeeper::remove(std::string name) From 788dc429f82fd4be296e86a293cd2a9f3f98e7c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 9 May 2019 10:05:40 +0300 Subject: [PATCH 2/4] Do client callback on owning worker The callback should've been done on the worker that owns the DCB instead of the main worker. --- server/modules/routing/avrorouter/avro_client.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/routing/avrorouter/avro_client.cc b/server/modules/routing/avrorouter/avro_client.cc index 566b7b571..1ff7a2515 100644 --- a/server/modules/routing/avrorouter/avro_client.cc +++ b/server/modules/routing/avrorouter/avro_client.cc @@ -244,7 +244,7 @@ bool file_in_dir(const char* dir, const char* file) */ void AvroSession::queue_client_callback() { - auto worker = mxs::RoutingWorker::get(mxs::RoutingWorker::MAIN); + auto worker = static_cast(dcb->poll.owner); worker->execute([this]() { client_callback(); }, mxs::RoutingWorker::EXECUTE_QUEUED); From 59f2145c00cdfa905e854a14d42d2fcef610a34e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 9 May 2019 10:13:43 +0300 Subject: [PATCH 3/4] Allocate blr heartbeat buffer on correct worker The buffer was allocated on one worker and written on another. --- .../modules/routing/binlogrouter/blr_slave.cc | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/server/modules/routing/binlogrouter/blr_slave.cc b/server/modules/routing/binlogrouter/blr_slave.cc index ed77b0e38..8a008184d 100644 --- a/server/modules/routing/binlogrouter/blr_slave.cc +++ b/server/modules/routing/binlogrouter/blr_slave.cc @@ -243,7 +243,7 @@ static int blr_slave_send_columndef_with_status_schema(ROUTER_INSTANCE* router, int len, uint8_t seqno); static bool blr_send_slave_heartbeat(void* inst); -static int blr_slave_send_heartbeat(ROUTER_INSTANCE* router, +static void blr_slave_send_heartbeat(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave); static int blr_set_master_ssl(ROUTER_INSTANCE* router, const ChangeMasterConfig& config, @@ -6204,13 +6204,11 @@ static bool blr_send_slave_heartbeat(void* inst) sptr->heartbeat, (unsigned long)sptr->lastReply); - if (blr_slave_send_heartbeat(router, sptr)) - { - /* Set last event */ - sptr->lastEventReceived = HEARTBEAT_EVENT; - /* Set last time */ - sptr->lastReply = t_now; - } + blr_slave_send_heartbeat(router, sptr); + /* Set last event */ + sptr->lastEventReceived = HEARTBEAT_EVENT; + /* Set last time */ + sptr->lastReply = t_now; } sptr = sptr->next; @@ -6228,7 +6226,7 @@ static bool blr_send_slave_heartbeat(void* inst) * @param slave The current slave connection * @return Number of bytes sent or 0 in case of failure */ -static int blr_slave_send_heartbeat(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave) +static void send_heartbeat(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave) { REP_HEADER hdr; GWBUF* h_event; @@ -6255,10 +6253,7 @@ static int blr_slave_send_heartbeat(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave * * Total = 5 bytes + len */ - if ((h_event = gwbuf_alloc(MYSQL_HEADER_LEN + 1 + len)) == NULL) - { - return 0; - } + h_event = gwbuf_alloc(MYSQL_HEADER_LEN + 1 + len); /* The OK/Err byte is part of payload */ hdr.payload_len = len + 1; @@ -6306,11 +6301,15 @@ static int blr_slave_send_heartbeat(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave } /* Write the packet */ + MXS_SESSION_ROUTE_REPLY(slave->dcb->session, h_event); +} + +static void blr_slave_send_heartbeat(ROUTER_INSTANCE* router, ROUTER_SLAVE* slave) +{ mxs::RoutingWorker* worker = (mxs::RoutingWorker*)slave->dcb->poll.owner; - worker->execute([slave, h_event]() { - MXS_SESSION_ROUTE_REPLY(slave->dcb->session, h_event); + worker->execute([router, slave]() { + send_heartbeat(router, slave); }, mxs::RoutingWorker::EXECUTE_AUTO); - return 1; } /** From 567ad9b8b846743936660096c620fa3a686e0775 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20M=C3=A4kel=C3=A4?= Date: Thu, 9 May 2019 12:05:22 +0300 Subject: [PATCH 4/4] Fix galeramon regression The comparisons were wrong: strcasecmp returns 0 for equal strings. --- server/modules/monitor/galeramon/galeramon.cc | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/server/modules/monitor/galeramon/galeramon.cc b/server/modules/monitor/galeramon/galeramon.cc index f84ef38b6..3cff962de 100644 --- a/server/modules/monitor/galeramon/galeramon.cc +++ b/server/modules/monitor/galeramon/galeramon.cc @@ -215,37 +215,41 @@ void GaleraMonitor::update_server_status(MXS_MONITORED_SERVER* monitored_server) /* Node is in desync - lets take it offline */ if (strcmp(row[0], "wsrep_desync") == 0) { - if (strcasecmp(row[1],"YES") || strcasecmp(row[1],"ON") || strcasecmp(row[1],"1") || strcasecmp(row[1],"true")) - { - info.joined = 0; - } + if (strcasecmp(row[1], "YES") == 0 || strcasecmp(row[1], "ON") == 0 + || strcasecmp(row[1], "1") == 0 || strcasecmp(row[1], "true") == 0) + { + info.joined = 0; + } } /* Node rejects queries - lets take it offline */ if (strcmp(row[0], "wsrep_reject_queries") == 0) { - if (strcasecmp(row[1],"ALL") || strcasecmp(row[1],"ALL_KILL")) - { - info.joined = 0; - } + if (strcasecmp(row[1], "ALL") == 0 + || strcasecmp(row[1], "ALL_KILL") == 0) + { + info.joined = 0; + } } /* Node rejects queries - lets take it offline */ if (strcmp(row[0], "wsrep_sst_donor_rejects_queries") == 0) { - if (strcasecmp(row[1],"YES") || strcasecmp(row[1],"ON") || strcasecmp(row[1],"1") || strcasecmp(row[1],"true")) - { - info.joined = 0; - } + if (strcasecmp(row[1], "YES") == 0 || strcasecmp(row[1], "ON") == 0 + || strcasecmp(row[1], "1") == 0 || strcasecmp(row[1], "true") == 0) + { + info.joined = 0; + } } /* Node is not ready - lets take it offline */ if (strcmp(row[0], "wsrep_ready") == 0) { - if (strcasecmp(row[1],"NO") || strcasecmp(row[1],"OFF") || strcasecmp(row[1],"0") || strcasecmp(row[1],"false")) - { - info.joined = 0; - } + if (strcasecmp(row[1], "NO") == 0 || strcasecmp(row[1], "OFF") == 0 + || strcasecmp(row[1], "0") == 0 || strcasecmp(row[1], "false") == 0) + { + info.joined = 0; + } } if (strcmp(row[0], "wsrep_cluster_state_uuid") == 0 && row[1] && *row[1])