diff --git a/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.cc b/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.cc index 9fee175b8634..94ebe57f9c7e 100644 --- a/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.cc +++ b/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.cc @@ -1756,6 +1756,15 @@ static void push_msg_3p(site_def const *site, pax_machine *p, pax_msg *msg, STRLIT(pax_op_to_str(msg->op))); } +/* A reserved synode is only ours while we still hold the node index it was + reserved under. A view change that renumbers us hands that slot to another + node, which may reserve it as well. */ +bool_t reservation_is_stale(synode_no msgno) { + site_def const *site = find_site_def(msgno); + node_no me = site ? get_nodeno(site) : VOID_NODE_NO; + return me != VOID_NODE_NO && me != msgno.node; +} + /* Brand client message with unique ID */ static void brand_client_msg(pax_msg *msg, synode_no msgno) { assert(!synode_eq(msgno, null_synode)); @@ -2520,6 +2529,13 @@ static int proposer_task(task_arg arg) { brand_client_msg(ep->client_msg->p, ep->msgno); + /* Only a locally allocated synode carries our own node index; a remote or + global allocation carries the allocating leader's, by design. */ + if (ep->synode_allocation == synode_allocation_type::local && + reservation_is_stale(ep->msgno)) { + GOTO(retry_new); + } + for (;;) { /* Loop until the client message has been learned */ /* Get a Paxos instance to send the client message */ @@ -2530,6 +2546,13 @@ static int proposer_task(task_arg arg) { goto retry_new; } + /* Checked again after wait_for_cache: that call can suspend, and a view + change during the wait leaves the reservation stale. */ + if (ep->synode_allocation == synode_allocation_type::local && + reservation_is_stale(ep->msgno)) { + GOTO(retry_new); + } + assert(ep->p); if (ep->client_msg->p->force_delivery) ep->p->force_delivery = ep->client_msg->p->force_delivery; diff --git a/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.h b/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.h index c83ac928b2b8..ebdd2cae95f3 100644 --- a/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.h +++ b/plugin/group_replication/libmysqlgcs/src/bindings/xcom/xcom/xcom_base.h @@ -70,6 +70,8 @@ void *xcom_thread_main(void *cp); synode_no incr_synode(synode_no synode); +bool_t reservation_is_stale(synode_no msgno); + synode_no decr_synode(synode_no synode); char *dbg_pax_msg(pax_msg const *p); diff --git a/unittest/gunit/libmysqlgcs/CMakeLists.txt b/unittest/gunit/libmysqlgcs/CMakeLists.txt index 27d9535d84c5..fbc87757e566 100644 --- a/unittest/gunit/libmysqlgcs/CMakeLists.txt +++ b/unittest/gunit/libmysqlgcs/CMakeLists.txt @@ -75,6 +75,7 @@ SET(GCS_XCOM_TESTS xcom/gcs_xcom_xcom_transport xcom/gcs_xcom_communication_protocol_changer xcom/gcs_xcom_xcom_cache + xcom/gcs_xcom_stale_reservation xcom/gcs_xcom_control_interface xcom/gcs_xcom_view_identifier xcom/gcs_message_stage_fragmentation diff --git a/unittest/gunit/libmysqlgcs/xcom/gcs_xcom_stale_reservation-t.cc b/unittest/gunit/libmysqlgcs/xcom/gcs_xcom_stale_reservation-t.cc new file mode 100644 index 000000000000..ca19847bf5b2 --- /dev/null +++ b/unittest/gunit/libmysqlgcs/xcom/gcs_xcom_stale_reservation-t.cc @@ -0,0 +1,96 @@ +/* Copyright (c) 2026, Oracle and/or its affiliates. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License, version 2.0, + as published by the Free Software Foundation. + + This program is designed to work with certain software (including + but not limited to OpenSSL) that is licensed under separate terms, + as designated in a particular file or component or in included license + documentation. The authors of MySQL hereby grant you an additional + permission to link the program and your derivative works with the + separately licensed software that they have either included with + the program or referenced in the documentation. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License, version 2.0, for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ + +#include +#include +#include "gcs_base_test.h" + +#include "site_def.h" +#include "xcom_base.h" + +namespace xcom_stale_reservation_unittest { + +/* Bug#121063: a synode reserved under one node index must be recognised as no + longer ours once a view change gives this member a different index. */ +class XcomStaleReservation : public GcsBaseTest { + protected: + void SetUp() override { + /* The view in force when the synode is reserved: this member is node 1. */ + char const *names[]{"127.0.0.1:12341", "127.0.0.1:12342", + "127.0.0.1:12343"}; + node_address *na = new_node_address(3, names); + + site_def *before = new_site_def(); + init_site_def(3, na, before); + before->start = synode_no{1, 10, 0}; + before->nodeno = 1; + push_site_def(before); + delete_node_address(3, na); + } + + /* The view installed while the reservation is held: the first member is + gone, so this one is now node 0. */ + void install_new_view() { + char const *names[]{"127.0.0.1:12342", "127.0.0.1:12343"}; + node_address *na = new_node_address(2, names); + + site_def *after = new_site_def(); + init_site_def(2, na, after); + after->start = synode_no{1, 20, 0}; + after->nodeno = 0; + push_site_def(after); + delete_node_address(2, na); + } + + void TearDown() override { free_site_defs(); } +}; + +/* Under the view it was taken in, the reservation is ours. */ +TEST_F(XcomStaleReservation, reservation_under_the_current_index_is_fresh) { + install_new_view(); + synode_no const reserved{1, 15, 1}; + ASSERT_FALSE(reservation_is_stale(reserved)); +} + +/* After the renumbering, the same index belongs to another node. */ +TEST_F(XcomStaleReservation, reservation_outliving_a_renumbering_is_stale) { + synode_no const reserved{1, 25, 1}; + ASSERT_FALSE(reservation_is_stale(reserved)); + install_new_view(); + ASSERT_TRUE(reservation_is_stale(reserved)); +} + +/* A slot that carries the index this member holds now is usable. */ +TEST_F(XcomStaleReservation, slot_matching_the_new_index_is_fresh) { + install_new_view(); + synode_no const reserved{1, 25, 0}; + ASSERT_FALSE(reservation_is_stale(reserved)); +} + +/* Without a site there is nothing to compare against. */ +TEST_F(XcomStaleReservation, unknown_site_is_not_reported_stale) { + synode_no const other_group{99, 25, 0}; + ASSERT_FALSE(reservation_is_stale(other_group)); +} + +} // namespace xcom_stale_reservation_unittest