MDEV-4820: Empty master does not give error for slave GTID position that does not exist in the binlog

The main bug here was the following situation:

Suppose we set up a completely new master2 as an extra multi-master to an
existing slave that already has a different master1 for domain_id=0. When the
slave tries to connect to master2, master2 will not have anything that slave
requests in domain_id=0, but that is fine as master2 is supposedly meant to
serve eg. domain_id=1. (This is MDEV-4485).

But suppose that master2 then actually starts sending events from
domain_id=0. In this case, the fix for MDEV-4485 was incomplete, and the code
would fail to give the error that the position requested by the slave in
domain_id=0 was missing from the binlogs of master2. This could lead to lost
events or completely wrong replication.

The patch for this bug fixes this issue.

In addition, it cleans up the code a bit, getting rid of the fake_gtid_hash in
the code. And the error message when slave and master have diverged due to
alternate future is clarified, as requested in the bug description.

1 files changed, 31 insertions, 18 deletions

diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc
index 4783fb763c8..c8a00cd78ae 100644
--- a/sql/rpl_gtid.cc
+++ b/sql/rpl_gtid.cc
@@ -1237,8 +1237,8 @@ rpl_binlog_state::append_pos(String *str)
 slave_connection_state::slave_connection_state()
 {
   my_hash_init(&hash, &my_charset_bin, 32,
-               offsetof(rpl_gtid, domain_id), sizeof(uint32), NULL, my_free,
-               HASH_UNIQUE);
+               offsetof(entry, gtid) + offsetof(rpl_gtid, domain_id),
+               sizeof(uint32), NULL, my_free, HASH_UNIQUE);
 }
 
 
@@ -1272,7 +1272,7 @@ slave_connection_state::load(char *slave_request, size_t len)
   char *p, *end;
   uchar *rec;
   rpl_gtid *gtid;
-  const rpl_gtid *gtid2;
+  const entry *e;
 
   reset();
   p= slave_request;
@@ -1281,27 +1281,28 @@ slave_connection_state::load(char *slave_request, size_t len)
     return 0;
   for (;;)
   {
-    if (!(rec= (uchar *)my_malloc(sizeof(*gtid), MYF(MY_WME))))
+    if (!(rec= (uchar *)my_malloc(sizeof(entry), MYF(MY_WME))))
     {
       my_error(ER_OUTOFMEMORY, MYF(0), sizeof(*gtid));
       return 1;
     }
-    gtid= (rpl_gtid *)rec;
+    gtid= &((entry *)rec)->gtid;
     if (gtid_parser_helper(&p, end, gtid))
     {
       my_free(rec);
       my_error(ER_INCORRECT_GTID_STATE, MYF(0));
       return 1;
     }
-    if ((gtid2= (const rpl_gtid *)
+    if ((e= (const entry *)
          my_hash_search(&hash, (const uchar *)(&gtid->domain_id), 0)))
     {
       my_error(ER_DUPLICATE_GTID_DOMAIN, MYF(0), gtid->domain_id,
-               gtid->server_id, (ulonglong)gtid->seq_no, gtid2->domain_id,
-               gtid2->server_id, (ulonglong)gtid2->seq_no, gtid->domain_id);
+               gtid->server_id, (ulonglong)gtid->seq_no, e->gtid.domain_id,
+               e->gtid.server_id, (ulonglong)e->gtid.seq_no, gtid->domain_id);
       my_free(rec);
       return 1;
     }
+    ((entry *)rec)->flags= 0;
     if (my_hash_insert(&hash, rec))
     {
       my_free(rec);
@@ -1357,30 +1358,42 @@ slave_connection_state::load(rpl_slave_state *state,
 }
 
 
+slave_connection_state::entry *
+slave_connection_state::find_entry(uint32 domain_id)
+{
+  return (entry *) my_hash_search(&hash, (const uchar *)(&domain_id), 0);
+}
+
+
 rpl_gtid *
 slave_connection_state::find(uint32 domain_id)
 {
-  return (rpl_gtid *) my_hash_search(&hash, (const uchar *)(&domain_id), 0);
+  entry *e= find_entry(domain_id);
+  if (!e)
+    return NULL;
+  return &e->gtid;
 }
 
 
 int
 slave_connection_state::update(const rpl_gtid *in_gtid)
 {
-  rpl_gtid *new_gtid;
+  entry *e;
   uchar *rec= my_hash_search(&hash, (const uchar *)(&in_gtid->domain_id), 0);
   if (rec)
   {
-    memcpy(rec, in_gtid, sizeof(*in_gtid));
+    e= (entry *)rec;
+    e->gtid= *in_gtid;
     return 0;
   }
 
-  if (!(new_gtid= (rpl_gtid *)my_malloc(sizeof(*new_gtid), MYF(MY_WME))))
+  if (!(e= (entry *)my_malloc(sizeof(*e), MYF(MY_WME))))
     return 1;
-  memcpy(new_gtid, in_gtid, sizeof(*new_gtid));
-  if (my_hash_insert(&hash, (uchar *)new_gtid))
+  e->gtid= *in_gtid;
+  e->flags= 0;
+  if (my_hash_insert(&hash, (uchar *)e))
   {
-    my_free(new_gtid);
+    my_free(e);
     return 1;
   }
 
@@ -1394,7 +1407,7 @@ slave_connection_state::remove(const rpl_gtid *in_gtid)
   uchar *rec= my_hash_search(&hash, (const uchar *)(&in_gtid->domain_id), 0);
 #ifndef DBUG_OFF
   bool err;
-  rpl_gtid *slave_gtid= (rpl_gtid *)rec;
+  rpl_gtid *slave_gtid= &((entry *)rec)->gtid;
   DBUG_ASSERT(rec /* We should never try to remove not present domain_id. */);
   DBUG_ASSERT(slave_gtid->server_id == in_gtid->server_id);
   DBUG_ASSERT(slave_gtid->seq_no == in_gtid->seq_no);
@@ -1423,8 +1436,8 @@ slave_connection_state::append_to_string(String *out_str)
   first= true;
   for (i= 0; i < hash.records; ++i)
   {
-    const rpl_gtid *gtid= (const rpl_gtid *)my_hash_element(&hash, i);
-    if (rpl_slave_state_tostring_helper(out_str, gtid, &first))
+    const entry *e= (const entry *)my_hash_element(&hash, i);
+    if (rpl_slave_state_tostring_helper(out_str, &e->gtid, &first))
       return 1;
   }
   return 0;