[Commits] 6a01ef3: MDEV-6368: assertion xid_seqno > trx_sys_cur_xid_seqno

Nirbhay Choubey nirbhay at mariadb.com
Tue May 3 01:23:12 EEST 2016


revision-id: 6a01ef38df59f2284d8468c9bc9681e837a625bd (mariadb-10.1.13-6-g6a01ef3)
parent(s): f8cdb9e7e8fb692f8eb3b9bb4792cd60653e0eb8
author: Nirbhay Choubey
committer: Nirbhay Choubey
timestamp: 2016-05-02 18:23:12 -0400
message:

MDEV-6368: assertion xid_seqno > trx_sys_cur_xid_seqno

- Validate the specified wsrep_start_position value by also
checking the return status of wsrep->sst_received. This also
ensures that changes in wsrep_start_position is not allowed
when the node is not in JOINING state.
- Do not allow decrease in seqno within same UUID.
- The initial checkpoint in SEs should be [0...:-1].

---
 .../sys_vars/r/wsrep_start_position_basic.result   | 10 ++-
 .../sys_vars/t/wsrep_start_position_basic.test     |  6 +-
 sql/wsrep_priv.h                                   |  4 +-
 sql/wsrep_sst.cc                                   | 93 +++++++++++++++++-----
 storage/innobase/trx/trx0sys.cc                    |  6 +-
 storage/xtradb/trx/trx0sys.cc                      |  6 +-
 6 files changed, 91 insertions(+), 34 deletions(-)

diff --git a/mysql-test/suite/sys_vars/r/wsrep_start_position_basic.result b/mysql-test/suite/sys_vars/r/wsrep_start_position_basic.result
index a49e613..036debb 100644
--- a/mysql-test/suite/sys_vars/r/wsrep_start_position_basic.result
+++ b/mysql-test/suite/sys_vars/r/wsrep_start_position_basic.result
@@ -17,10 +17,6 @@ SELECT @@global.wsrep_start_position;
 00000000-0000-0000-0000-000000000000:-1
 
 # valid values
-SET @@global.wsrep_start_position='00000000-0000-0000-0000-000000000000:-2';
-SELECT @@global.wsrep_start_position;
-@@global.wsrep_start_position
-00000000-0000-0000-0000-000000000000:-2
 SET @@global.wsrep_start_position='12345678-1234-1234-1234-123456789012:100';
 SELECT @@global.wsrep_start_position;
 @@global.wsrep_start_position
@@ -31,6 +27,12 @@ SELECT @@global.wsrep_start_position;
 00000000-0000-0000-0000-000000000000:-1
 
 # invalid values
+call mtr.add_suppression("WSREP: SST postion can't be set in past.*");
+SET @@global.wsrep_start_position='00000000-0000-0000-0000-000000000000:-2';
+ERROR 42000: Variable 'wsrep_start_position' can't be set to the value of '00000000-0000-0000-0000-000000000000:-2'
+SELECT @@global.wsrep_start_position;
+@@global.wsrep_start_position
+00000000-0000-0000-0000-000000000000:-1
 SET @@global.wsrep_start_position='000000000000000-0000-0000-0000-000000000000:-1';
 ERROR 42000: Variable 'wsrep_start_position' can't be set to the value of '000000000000000-0000-0000-0000-000000000000:-1'
 SET @@global.wsrep_start_position='12345678-1234-1234-12345-123456789012:100';
diff --git a/mysql-test/suite/sys_vars/t/wsrep_start_position_basic.test b/mysql-test/suite/sys_vars/t/wsrep_start_position_basic.test
index 3e57cfa..936646f 100644
--- a/mysql-test/suite/sys_vars/t/wsrep_start_position_basic.test
+++ b/mysql-test/suite/sys_vars/t/wsrep_start_position_basic.test
@@ -19,8 +19,6 @@ SELECT @@global.wsrep_start_position;
 
 --echo
 --echo # valid values
-SET @@global.wsrep_start_position='00000000-0000-0000-0000-000000000000:-2';
-SELECT @@global.wsrep_start_position;
 SET @@global.wsrep_start_position='12345678-1234-1234-1234-123456789012:100';
 SELECT @@global.wsrep_start_position;
 SET @@global.wsrep_start_position=default;
@@ -28,6 +26,10 @@ SELECT @@global.wsrep_start_position;
 
 --echo
 --echo # invalid values
+call mtr.add_suppression("WSREP: SST postion can't be set in past.*");
+--error ER_WRONG_VALUE_FOR_VAR
+SET @@global.wsrep_start_position='00000000-0000-0000-0000-000000000000:-2';
+SELECT @@global.wsrep_start_position;
 --error ER_WRONG_VALUE_FOR_VAR
 SET @@global.wsrep_start_position='000000000000000-0000-0000-0000-000000000000:-1';
 --error ER_WRONG_VALUE_FOR_VAR
diff --git a/sql/wsrep_priv.h b/sql/wsrep_priv.h
index 30dce78..a6e57e3 100644
--- a/sql/wsrep_priv.h
+++ b/sql/wsrep_priv.h
@@ -40,8 +40,8 @@ wsrep_cb_status wsrep_sst_donate_cb (void* app_ctx,
 extern wsrep_seqno_t local_seqno;
 
 // a helper function
-void wsrep_sst_received(wsrep_t*, const wsrep_uuid_t&, wsrep_seqno_t,
-                        const void*, size_t);
+bool wsrep_sst_received(wsrep_t*, const wsrep_uuid_t&, wsrep_seqno_t,
+                        const void*, size_t, bool const);
 /*! SST thread signals init thread about sst completion */
 void wsrep_sst_complete(const wsrep_uuid_t*, wsrep_seqno_t, bool);
 
diff --git a/sql/wsrep_sst.cc b/sql/wsrep_sst.cc
index 4f6cc51..44fdd03 100644
--- a/sql/wsrep_sst.cc
+++ b/sql/wsrep_sst.cc
@@ -264,42 +264,91 @@ void wsrep_sst_complete (const wsrep_uuid_t* sst_uuid,
   mysql_mutex_unlock (&LOCK_wsrep_sst);
 }
 
-void wsrep_sst_received (wsrep_t*            const wsrep,
+/*
+  If wsrep provider is loaded, inform that the new state snapshot
+  has been received. Also update the local checkpoint.
+
+  @param wsrep     [IN]               wsrep handle
+  @param uuid      [IN]               Initial state UUID
+  @param seqno     [IN]               Initial state sequence number
+  @param state     [IN]               Always NULL, also ignored by wsrep provider (?)
+  @param state_len [IN]               Always 0, also ignored by wsrep provider (?)
+  @param implicit  [IN]               Whether invoked implicitly due to SST
+                                      (true) or explicitly because if change
+                                      in wsrep_start_position by user (false).
+  @return false                       Success
+          true                        Error
+
+*/
+bool wsrep_sst_received (wsrep_t*            const wsrep,
                          const wsrep_uuid_t&       uuid,
                          wsrep_seqno_t       const seqno,
                          const void*         const state,
-                         size_t              const state_len)
+                         size_t              const state_len,
+                         bool                const implicit)
 {
-    wsrep_get_SE_checkpoint(local_uuid, local_seqno);
+  /*
+    To keep track of whether the local uuid:seqno should be updated. Also, note
+    that local state (uuid:seqno) is updated/checkpointed only after we get an
+    OK from wsrep provider. By doing so, the values remain consistent across
+    the server & wsrep provider.
+  */
+  bool do_update= false;
+
+  // Get the locally stored uuid:seqno.
+  wsrep_get_SE_checkpoint(local_uuid, local_seqno);
 
-    if (memcmp(&local_uuid, &uuid, sizeof(wsrep_uuid_t)) ||
-        local_seqno < seqno || seqno < 0)
+  if (memcmp(&local_uuid, &uuid, sizeof(wsrep_uuid_t)) ||
+      local_seqno < seqno)
+  {
+    do_update= true;
+  }
+  else if (local_seqno > seqno)
+  {
+    WSREP_WARN("SST position can't be set in past. Requested: %lld, Current: "
+               " %lld.", (long long)seqno, (long long)local_seqno);
+    /*
+      If we are here because of SET command, simply return true (error) instead of
+      aborting.
+    */
+    if (implicit)
     {
-        wsrep_set_SE_checkpoint(uuid, seqno);
-        local_uuid = uuid;
-        local_seqno = seqno;
+      WSREP_WARN("Can't continue.");
+      unireg_abort(1);
     }
-    else if (local_seqno > seqno)
+    else
     {
-        WSREP_WARN("SST postion is in the past: %lld, current: %lld. "
-                   "Can't continue.",
-                   (long long)seqno, (long long)local_seqno);
-        unireg_abort(1);
+      return true;
     }
+  }
 
 #ifdef GTID_SUPPORT
-    wsrep_init_sidno(uuid);
+  wsrep_init_sidno(uuid);
 #endif /* GTID_SUPPORT */
 
-    if (wsrep)
-    {
-        int const rcode(seqno < 0 ? seqno : 0);
-        wsrep_gtid_t const state_id = {
-            uuid, (rcode ? WSREP_SEQNO_UNDEFINED : seqno)
-        };
+  if (wsrep)
+  {
+    int const rcode(seqno < 0 ? seqno : 0);
+    wsrep_gtid_t const state_id= {uuid,
+      (rcode ? WSREP_SEQNO_UNDEFINED : seqno)};
+
+    wsrep_status_t ret= wsrep->sst_received(wsrep, &state_id, state,
+                                            state_len, rcode);
 
-        wsrep->sst_received(wsrep, &state_id, state, state_len, rcode);
+    if (ret != WSREP_OK)
+    {
+      return true;
     }
+  }
+
+  // Now is the good time to update the local state and checkpoint.
+  if (do_update) {
+    wsrep_set_SE_checkpoint(uuid, seqno);
+    local_uuid= uuid;
+    local_seqno= seqno;
+  }
+
+  return false;
 }
 
 // Let applier threads to continue
@@ -308,7 +357,7 @@ void wsrep_sst_continue ()
   if (sst_needed)
   {
     WSREP_INFO("Signalling provider to continue.");
-    wsrep_sst_received (wsrep, local_uuid, local_seqno, NULL, 0);
+    wsrep_sst_received (wsrep, local_uuid, local_seqno, NULL, 0, true);
   }
 }
 
diff --git a/storage/innobase/trx/trx0sys.cc b/storage/innobase/trx/trx0sys.cc
index 76c8613..1625e8d 100644
--- a/storage/innobase/trx/trx0sys.cc
+++ b/storage/innobase/trx/trx0sys.cc
@@ -349,10 +349,10 @@ void read_wsrep_xid_uuid(const XID* xid, unsigned char* buf)
             unsigned char xid_uuid[16];
             long long xid_seqno = read_wsrep_xid_seqno(xid);
             read_wsrep_xid_uuid(xid, xid_uuid);
-            if (!memcmp(xid_uuid, trx_sys_cur_xid_uuid, 8))
+            if (!memcmp(xid_uuid, trx_sys_cur_xid_uuid, 16) &&
+                (xid_seqno > -1))
             {
                 ut_ad(xid_seqno > trx_sys_cur_xid_seqno);
-                trx_sys_cur_xid_seqno = xid_seqno;
             }
             else
             {
@@ -411,6 +411,8 @@ void read_wsrep_xid_uuid(const XID* xid, unsigned char* buf)
                                       + TRX_SYS_WSREP_XID_MAGIC_N_FLD))
             != TRX_SYS_WSREP_XID_MAGIC_N) {
                 memset(xid, 0, sizeof(*xid));
+                long long seqno= -1;
+                memcpy(xid->data + 24, &seqno, sizeof(long long));
                 xid->formatID = -1;
                 trx_sys_update_wsrep_checkpoint(xid, sys_header, &mtr);
                 mtr_commit(&mtr);
diff --git a/storage/xtradb/trx/trx0sys.cc b/storage/xtradb/trx/trx0sys.cc
index 9b136cc..08bb74a 100644
--- a/storage/xtradb/trx/trx0sys.cc
+++ b/storage/xtradb/trx/trx0sys.cc
@@ -353,10 +353,10 @@ void read_wsrep_xid_uuid(const XID* xid, unsigned char* buf)
             unsigned char xid_uuid[16];
             long long xid_seqno = read_wsrep_xid_seqno(xid);
             read_wsrep_xid_uuid(xid, xid_uuid);
-            if (!memcmp(xid_uuid, trx_sys_cur_xid_uuid, 8))
+            if (!memcmp(xid_uuid, trx_sys_cur_xid_uuid, 16) &&
+                (xid_seqno > -1))
             {
                 ut_ad(xid_seqno > trx_sys_cur_xid_seqno);
-                trx_sys_cur_xid_seqno = xid_seqno;
             }
             else
             {
@@ -415,6 +415,8 @@ void read_wsrep_xid_uuid(const XID* xid, unsigned char* buf)
                                       + TRX_SYS_WSREP_XID_MAGIC_N_FLD))
             != TRX_SYS_WSREP_XID_MAGIC_N) {
                 memset(xid, 0, sizeof(*xid));
+                long long seqno= -1;
+                memcpy(xid->data + 24, &seqno, sizeof(long long));
                 xid->formatID = -1;
                 trx_sys_update_wsrep_checkpoint(xid, sys_header, &mtr);
                 mtr_commit(&mtr);


More information about the commits mailing list