maria-developers team mailing list archive

[Jeremy Cole <jeremycole@xxxxxxxxxx>]

Sanja,

I could provide some very simple binary logs (from Google MySQL 5.1) which
are internally consistent and have extra headers (they would have 31-byte
headers instead of the usual 19-byte ones). That can at least pretty easily
be used to make sure mysqlbinlog can parse them properly.

Regards,

Jeremy


On Thu, Jul 18, 2013 at 11:07 AM, Oleksandr Byelkin
<sanja@xxxxxxxxxxxxxxxx>wrote:

> 18.07.2013 19:39, Jeremy Cole пишет:
>
>> Sanja,
>>
>> Re: https://mariadb.atlassian.net/**browse/MDEV-4645<https://mariadb.atlassian.net/browse/MDEV-4645>
>>
>> Re: http://lists.askmonty.org/**pipermail/commits/2013-July/**005093.html<http://lists.askmonty.org/pipermail/commits/2013-July/005093.html>
>>
>> I saw your commit on the commits list (sorry, wasn't on the list before
>> the
>> mail went out, so I can't reply directly to that message).
>>
>> A couple of points related to the commit:
>>
>>     - You seem to have an error in the get_header_len, as it's implemented
>>
>>     only in the base class, and you've left functions with the correct
>>     signature for it but the wrong names in the two frozen classes (they
>> were
>>     left named has_fixed_minimal_header). This would mean the change is
>>     completely broken. :)
>>     - The change to get_header_len I think is in general fine, although it
>>
>>     does alter the point of the whole thing -- there is again no way to
>> exactly
>>     identify which events have frozen headers, programmatically (i.e. you
>> can't
>>     differentiate them from events who just "happen" to have header_len =
>>     LOG_EVENT_MINIMAL_HEADER_LEN). I think that doesn't matter for this
>>     patch, but does defeat the point of what I was trying to add.
>>     - I don't really understand why you've made the hex printing code so
>>
>>     complex and completely unreadable (IMHO). This is not
>> performance-sensitive
>>     code and in my opinion should favor readability over performance. All
>> you
>>     seem to have saved in this change is to avoid printing some static
>>     formatting characters ("#", "|", spaces, etc.) inside the loop.
>> However, I
>>     can hardly even figure out what it's supposed to be doing now.
>>     - Additionally, you've changed the output slightly -- normally the
>> ASCII
>>
>>     character section prints its ending "|" immediately after the last
>>     character; it seems now to be fixed position (leaving extra spaces
>> before
>>     "|". This differs from the output of hexdump, and it somewhat
>>     ambiguous/confusing.
>>     - You've mixed tabs and spaces. :(
>>
>
> Thank you a lot for this bug-report, I saw that the test suite do not
> cover the bug so did not pushed the fix hoping for your response.
>
> Test suite still highly appreciated :) (if it is possible to make it at
> all, I doubts)
>
> The cause of changes was:
> 1) avoid double copying string (formatting characters are not really
> matter) and extra memory allocation on the stack.
> 2) avoid 'if' in case when method can return value which we can use
> directly (looks better IMHO)
> 3) avoid extra virtual method call (they are quite expensive)
> Yes 1 and 3 are about performance but I deal with server code mostly.
>
> I'll change the patch, thank you yet another time.
>
>
>