Description:
Background:
Binlog_sender writes events from the binary log to a buffer, then sends the buffer to the slave. The buffer is kept between events. Before an event is written to the buffer, if the event is bigger than the current size of the buffer, the buffer is reallocated. To keep reallocations few, it grows to at least twice the size. To prevent that the buffer stays big forever just because there was one big event, the buffer shrinks when events are small again. To prevent that the buffer grows and shrinks often, it shrinks to half the size if 100 events in sequence are smaller than half the buffer size.
Part of the logic in Binlog_sender::read_event relevant to this bug is as follows:
1. pre-allocate as many bytes as needed (call reset_transmit_packet which calls grow_packet()).
2. read the event buffer into *event_ptr.
3. read the event into the buffer (call Log_event::read_log_event).
4. use event_ptr.
This was the logic before BUG#22109863 was fixed.
Problem:
P1. The buffer does not shrink to half the size if 100 events in sequence are smaller than half the buffer size. Due to a typo in the code, the buffer shrinks to 4096 bytes when 100 events in sequence are smaller than 4096 bytes. So if one event in 100 is 4097 bytes big, the buffer will never shrink.
P2. In BUG#22109863, freed memory was accessed because Log_event::read_log_event would resize the packet. The reason for resizing the packet is that String::mem_realloc may reallocate even when it is not needed, because it determines whether to realloc based on a rounded-up string length:
bool String::mem_realloc(size_t alloc_length, bool force_on_heap)
{
size_t len= ALIGN_SIZE(alloc_length + 1);
[...]
if (m_alloced_length < len)
[...]
if (!(new_ptr= static_cast<char*>
(my_realloc(STRING_PSI_MEMORY_KEY,
However, BUG#22109863 was fixed in a different way: it allocates one extra byte. This works because the buffer size is always a multiple of 8 and ALIGN_SIZE rounds up to the nearest multiple of 8. Namely, if the buffer length is greater than the event length, then the event length plus one rounded up to the nearest multiple of 8 is less than or equal to the buffer length, so then mem_realloc will not try to realloc. And if the buffer length is smaller than or equal to the event length, grow_packet will resize it. However, it is only because of problem P1 that the buffer is a multiple of 8. The buffer can grow using String::mem_realloc and shrink using String::shrink. String::shrink does not round to multiples of 8, but it is only called with the value 4096 which happens to be a multiple of 8. However, after we fix P1, String::shrink will be called with numbers that are not multiples of 8. Then, when the buffer size is not a multiple of 8 (say it is 4100), if the event length is one less than the buffer size, grow_packet will not realloc the buffer but String::mem_realloc will, because the event rounded up will be bigger than the buffer size (in this case 4104). So BUG#22109863 will come back. Luckily, BUG#22109863 changed so that the event pointer is read only after the call Log_event::read_log_event, so there will not be any read of freed memory. However, there will still be unnecessary reallocs, both in grow_packet (because it allocates one byte too much) and in String::mem_realloc (because it uses the rounded-up alloc_length to determine if reallocation is needed).
How to repeat:
--source include/master-slave.inc
CREATE TABLE t1 (a TEXT(1000000));
--echo # INSERT 900k string
--disable_query_log
--let $big= `SELECT REPEAT('a', 900000)`
eval INSERT INTO t1 VALUES ('$big');
--let $i= 0
--let $medium= `SELECT REPEAT('b', 4096)`
while ($i < 200) {
--echo # INSERT 4k string
eval INSERT INTO t1 VALUES ('$medium');
--inc $i
}
--enable_query_log
# Grep for cur_buffer_size in the debug trace to see that the buffer
# size never shrinks.
Suggested fix:
1. Compute Binlog_sender::m_new_shrink_size correctly
2. Don't allocate unnecessarily in String::mem_realloc
3. Don't allocate extra byte in Binlog_sender::grow_packet
Description: Background: Binlog_sender writes events from the binary log to a buffer, then sends the buffer to the slave. The buffer is kept between events. Before an event is written to the buffer, if the event is bigger than the current size of the buffer, the buffer is reallocated. To keep reallocations few, it grows to at least twice the size. To prevent that the buffer stays big forever just because there was one big event, the buffer shrinks when events are small again. To prevent that the buffer grows and shrinks often, it shrinks to half the size if 100 events in sequence are smaller than half the buffer size. Part of the logic in Binlog_sender::read_event relevant to this bug is as follows: 1. pre-allocate as many bytes as needed (call reset_transmit_packet which calls grow_packet()). 2. read the event buffer into *event_ptr. 3. read the event into the buffer (call Log_event::read_log_event). 4. use event_ptr. This was the logic before BUG#22109863 was fixed. Problem: P1. The buffer does not shrink to half the size if 100 events in sequence are smaller than half the buffer size. Due to a typo in the code, the buffer shrinks to 4096 bytes when 100 events in sequence are smaller than 4096 bytes. So if one event in 100 is 4097 bytes big, the buffer will never shrink. P2. In BUG#22109863, freed memory was accessed because Log_event::read_log_event would resize the packet. The reason for resizing the packet is that String::mem_realloc may reallocate even when it is not needed, because it determines whether to realloc based on a rounded-up string length: bool String::mem_realloc(size_t alloc_length, bool force_on_heap) { size_t len= ALIGN_SIZE(alloc_length + 1); [...] if (m_alloced_length < len) [...] if (!(new_ptr= static_cast<char*> (my_realloc(STRING_PSI_MEMORY_KEY, However, BUG#22109863 was fixed in a different way: it allocates one extra byte. This works because the buffer size is always a multiple of 8 and ALIGN_SIZE rounds up to the nearest multiple of 8. Namely, if the buffer length is greater than the event length, then the event length plus one rounded up to the nearest multiple of 8 is less than or equal to the buffer length, so then mem_realloc will not try to realloc. And if the buffer length is smaller than or equal to the event length, grow_packet will resize it. However, it is only because of problem P1 that the buffer is a multiple of 8. The buffer can grow using String::mem_realloc and shrink using String::shrink. String::shrink does not round to multiples of 8, but it is only called with the value 4096 which happens to be a multiple of 8. However, after we fix P1, String::shrink will be called with numbers that are not multiples of 8. Then, when the buffer size is not a multiple of 8 (say it is 4100), if the event length is one less than the buffer size, grow_packet will not realloc the buffer but String::mem_realloc will, because the event rounded up will be bigger than the buffer size (in this case 4104). So BUG#22109863 will come back. Luckily, BUG#22109863 changed so that the event pointer is read only after the call Log_event::read_log_event, so there will not be any read of freed memory. However, there will still be unnecessary reallocs, both in grow_packet (because it allocates one byte too much) and in String::mem_realloc (because it uses the rounded-up alloc_length to determine if reallocation is needed). How to repeat: --source include/master-slave.inc CREATE TABLE t1 (a TEXT(1000000)); --echo # INSERT 900k string --disable_query_log --let $big= `SELECT REPEAT('a', 900000)` eval INSERT INTO t1 VALUES ('$big'); --let $i= 0 --let $medium= `SELECT REPEAT('b', 4096)` while ($i < 200) { --echo # INSERT 4k string eval INSERT INTO t1 VALUES ('$medium'); --inc $i } --enable_query_log # Grep for cur_buffer_size in the debug trace to see that the buffer # size never shrinks. Suggested fix: 1. Compute Binlog_sender::m_new_shrink_size correctly 2. Don't allocate unnecessarily in String::mem_realloc 3. Don't allocate extra byte in Binlog_sender::grow_packet