Skip to content

Commit

Permalink
BUG#24764113: TRANSACTION_CONTEXT_EVENT::WRITE_SET_LEN HAS NOT ENOUGH…
Browse files Browse the repository at this point in the history
… INTEGER SIZE

On Group Replication transactions are executed optimistically and
then, at commit time, are checked for conflicts. If there are
conflicts, in order to maintain consistency across the group, some
transactions will be rolled back.
In order to detect conflicts, at commit time, each transaction data
and write sets are sent to all members. It was discovered that
serialization of the write sets to the network format was limited by
a 16 bits integer, which is not enough to hold the size of big
transactions write sets. This was causing write sets to be trim and
conflict detection unable to find conflicts.

In order to fix the above issue, now we do use a 32 bit integer on
Transaction_context_log_event write set length which can hold big
transactions write sets.
Since we are already changing Transaction_context_log_event, we did
also fix the size of thread_id field from 64 to 32 bits.
  • Loading branch information
nacarvalho committed Oct 4, 2016
1 parent a8844a1 commit fa5bea7
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 21 deletions.
22 changes: 11 additions & 11 deletions libbinlogevents/include/control_events.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (c) 2014, 2015, Oracle and/or its affiliates. All rights reserved.
/* Copyright (c) 2014, 2016, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -1106,7 +1106,7 @@ class Previous_gtids_event : public Binary_log_event
<tr>
<td>thread_id</td>
<td>long long type variable</td>
<td>4 byte integer</td>
<td>The identifier for the thread executing the transaction.</td>
</tr>
Expand Down Expand Up @@ -1178,7 +1178,7 @@ class Transaction_context_event : public Binary_log_event

virtual ~Transaction_context_event();

static const char *read_data_set(const char *pos, uint16_t set_len,
static const char *read_data_set(const char *pos, uint32_t set_len,
std::list<const char*> *set);

static void clear_set(std::list<const char*> *set);
Expand All @@ -1190,7 +1190,7 @@ class Transaction_context_event : public Binary_log_event

protected:
const char *server_uuid;
long long int thread_id;
uint32_t thread_id;
bool gtid_specified;
const unsigned char *encoded_snapshot_version;
uint32_t encoded_snapshot_version_length;
Expand All @@ -1202,16 +1202,16 @@ class Transaction_context_event : public Binary_log_event

// 1 byte length.
static const int ENCODED_SERVER_UUID_LEN_OFFSET= 0;
// 8 bytes length.
// 4 bytes length.
static const int ENCODED_THREAD_ID_OFFSET= 1;
// 1 byte length.
static const int ENCODED_GTID_SPECIFIED_OFFSET= 9;
static const int ENCODED_GTID_SPECIFIED_OFFSET= 5;
// 4 bytes length
static const int ENCODED_SNAPSHOT_VERSION_LEN_OFFSET= 10;
// 2 bytes length.
static const int ENCODED_WRITE_SET_ITEMS_OFFSET= 14;
// 2 bytes length.
static const int ENCODED_READ_SET_ITEMS_OFFSET= 16;
static const int ENCODED_SNAPSHOT_VERSION_LEN_OFFSET= 6;
// 4 bytes length.
static const int ENCODED_WRITE_SET_ITEMS_OFFSET= 10;
// 4 bytes length.
static const int ENCODED_READ_SET_ITEMS_OFFSET= 14;

// The values mentioned on the next class's constants is the length of the
// data that will be copied in the buffer.
Expand Down
14 changes: 7 additions & 7 deletions libbinlogevents/src/control_events.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -707,15 +707,15 @@ Transaction_context_event(const char *buffer, unsigned int event_len,
uint8_t server_uuid_len= (static_cast<unsigned int>
(data_head[ENCODED_SERVER_UUID_LEN_OFFSET]));

uint16_t write_set_len= 0;
uint32_t write_set_len= 0;
memcpy(&write_set_len, data_head + ENCODED_WRITE_SET_ITEMS_OFFSET,
sizeof(write_set_len));
write_set_len= le16toh(write_set_len);
write_set_len= le32toh(write_set_len);

uint16_t read_set_len= 0;
uint32_t read_set_len= 0;
memcpy(&read_set_len, data_head + ENCODED_READ_SET_ITEMS_OFFSET,
sizeof(read_set_len));
read_set_len= le16toh(read_set_len);
read_set_len= le32toh(read_set_len);

encoded_snapshot_version_length= 0;
memcpy(&encoded_snapshot_version_length,
Expand All @@ -724,7 +724,7 @@ Transaction_context_event(const char *buffer, unsigned int event_len,
encoded_snapshot_version_length= le32toh(encoded_snapshot_version_length);

memcpy(&thread_id, data_head + ENCODED_THREAD_ID_OFFSET, sizeof(thread_id));
thread_id= (uint64_t) le64toh(thread_id);
thread_id= (uint32_t) le32toh(thread_id);
gtid_specified= (int8_t) data_head[ENCODED_GTID_SPECIFIED_OFFSET];

const char *pos = data_head + TRANSACTION_CONTEXT_HEADER_LEN;
Expand Down Expand Up @@ -766,11 +766,11 @@ Transaction_context_event(const char *buffer, unsigned int event_len,
value.
*/
const char* Transaction_context_event::read_data_set(const char *pos,
uint16_t set_len,
uint32_t set_len,
std::list<const char*> *set)
{
uint16_t len= 0;
for (int i= 0; i < set_len; i++)
for (uint32_t i= 0; i < set_len; i++)
{
memcpy(&len, pos, 2);
len= le16toh(len);
Expand Down
6 changes: 3 additions & 3 deletions sql/log_event.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13497,11 +13497,11 @@ bool Transaction_context_log_event::write_data_header(IO_CACHE* file)
char buf[Binary_log_event::TRANSACTION_CONTEXT_HEADER_LEN];

buf[ENCODED_SERVER_UUID_LEN_OFFSET] = (char) strlen(server_uuid);
int8store(buf + ENCODED_THREAD_ID_OFFSET, thread_id);
int4store(buf + ENCODED_THREAD_ID_OFFSET, thread_id);
buf[ENCODED_GTID_SPECIFIED_OFFSET] = gtid_specified;
int4store(buf + ENCODED_SNAPSHOT_VERSION_LEN_OFFSET, get_snapshot_version_size());
int2store(buf + ENCODED_WRITE_SET_ITEMS_OFFSET, write_set.size());
int2store(buf + ENCODED_READ_SET_ITEMS_OFFSET, read_set.size());
int4store(buf + ENCODED_WRITE_SET_ITEMS_OFFSET, write_set.size());
int4store(buf + ENCODED_READ_SET_ITEMS_OFFSET, read_set.size());
DBUG_RETURN(wrapper_my_b_safe_write(file, (const uchar *) buf,
Binary_log_event::TRANSACTION_CONTEXT_HEADER_LEN));
}
Expand Down

0 comments on commit fa5bea7

Please sign in to comment.