| Bug #37466 | Adding field to Log_event header not backward compatible | ||
|---|---|---|---|
| Submitted: | 18 Jun 2008 0:48 | Modified: | 17 Oct 2008 16:50 |
| Reporter: | Justin Tolmer | ||
| Status: | In progress | ||
| Category: | Server: Replication | Severity: | S3 (Non-critical) |
| Version: | 5.1.23-rc | OS: | Any |
| Assigned to: | Mats Kindahl | Target Version: | |
| Triage: | Triaged: D5 (Feature request) | ||
[18 Jun 2008 12:28]
Susanne Ebrecht
Many thanks for writing a bug report. We will analyse this.
[24 Jun 2008 22:47]
Justin Tolmer
Found another instance of this problem in the code in fake_rotate_event. This function generates a Rotate_Log_event, which is supposed to have a header frozen at 19 bytes and so the function should be using LOG_EVENT_MINIMAL_HEADER_LEN instead of LOG_EVENT_HEADER_LEN.
[12 Aug 2008 15:38]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/51416 2669 Mats Kindahl 2008-08-12 Bug #37466 Adding field to Log_event header not backward compatible In order to be able to extend the common header, the length of the common header is given in the format description log event. This will allow any server reading an event to find the location of the post- header and data even if the common header is longer than expected by just adding the size of the common header given in the format description log event. However, in order to be read correctly, the common header of both the format description log event and rotate event has to be fixed at LOG_EVENT_MINIMAL_HEADER_LEN, which is 19. The value of LOG_ EVENT_MINIMAL_HEADER_LEN may not change. If the common header is extended with new data, the header will grow beyond LOG_EVENT_MINIMAL_HEADER_LEN. However, the code for the format description and rotate event reads the size from the format description log event instead of just using the value LOG_EVENT_MINIMAL_HEADER_LEN, this subsequently will cause them to read bad data, leading to a crash or data corruption of some other form. This patch replace the read of the common header length from the format description log event with the constant LOG_EVENT_MINIMAL_HEADER_LEN in order to allow the common header to be extended with new fields.
[12 Aug 2008 15:39]
Mats Kindahl
Patch committed towards 5.1 even though it will be merged with 6.0 and not pushed to 5.1.
[6 Oct 2008 21:58]
Justin Tolmer
The patch doesn't cover the change needed to fake_rotate_event in file sql/sql_repl.cc.
[7 Oct 2008 12:34]
Andrei Elkin
Justin, hello. According to fake_rotate_event() definition the fake rotate is created having LOG_EVENT_MINIMAL_HEADER_LEN bytes in the common header and then no changes are needed in this part. Could you please explain what is the change you meant for the fake rotate? Regards, Andrei
[7 Oct 2008 13:16]
Bugs System
A patch for this bug has been committed. After review, it may be pushed to the relevant source trees for release in the next version. You can access the patch from: http://lists.mysql.com/commits/51416
[7 Oct 2008 18:23]
Justin Tolmer
Andrei,
In the mysql-5.1.24-rc source, and the copy of the 6.0 sources I have, fake_rotate_event
has this code:
DBUG_ENTER("fake_rotate_event");
char header[LOG_EVENT_HEADER_LEN], buf[ROTATE_HEADER_LEN+100];
...
packet->append(header, sizeof(header));
Thus, the header being written for the event is too large since rotate events are
supposed to be frozen at size LOG_EVENT_MINIMAL_HEADER_LEN.
I continually get errors when trying to view the source on launchpad.net and so don't
know if the head revisions of the files still contain the error or have perhaps
subsequently been fixed.
Description: Much support has been added to sql/log_event.cc so that new fields can be added to the event header without needing to change the file format. Format_description_log_event plays a critical role because it conveys information on the size of the header of events which follow it. This support should allow a server with the current header format containing 19 bytes to replicate from a server with a newer format which has a header with a larger size. The intended functioning of this is outlined in detail in worklog #3610. Unfortunately, there are 2 bugs which prevent this from working correctly. Log_event::Log_event(const char* buf, const Format_description_log_event* description_event) contains this code snippet: if ((buf[EVENT_TYPE_OFFSET] == FORMAT_DESCRIPTION_EVENT) || (buf[EVENT_TYPE_OFFSET] == ROTATE_EVENT)) { /* These events always have a header which stops here (i.e. their header is FROZEN). */ meaning that their headers will forever by 19 bytes. However both Start_log_event_v3::Start_log_event_v3(const char* buf, const Format_description_log_event *description_event) and Rotate_log_event::Rotate_log_event(const char* buf, uint event_len, const Format_description_log_event* description_event) contain code that does: buf+= description_event->common_header_len; which is not correct. How to repeat: Reproducing the error is involved because it requires adding a new field to the Log_event header and then setting up replication using the new header format. Suggested fix: In the 2 contructors mentioned, change the code to buf+= LOG_EVENT_MINIMAL_HEADER_LEN;