He Zhenxing <Zhenxing.He@stripped> writes:
> Andrei Elkin wrote:
>> Alfranio, Zhen Xing, hello.
>>
>> > Alfranio Correia wrote:
>> >
>> > Hi Jasonh,
>> >
>> > I like your idea:
>> >
>> > "I'm thinking about maybe we can use the command flags setupped in
>> > init_update_queries(), such as CF_AUTO_COMMIT_TRANS, or maybe we can
>> > define a new flag."
>> >
>> >
>> > Maybe we could create a new flag and remove the "direct" flag from
>> > the MYSQL_BIN_LOG::write().
>>
>> To repeat (and clarify on), we have had the bool is_trans which I find
>> is better to be an exact qualifier where the being logged stuff should
>> go.
>> And we have three choices with the patch: the direct, the T-cache, and
>> the N-cache.
>>
>
> I agree that the direct argument should be removed.
>
> I only see three cases where direct is set to true (please point out if
> there are other cases):
> 1) for DDL, in this case it can be checked within the function
> 2) for Incident events, this can also be checked within the function
> 3) in close_temporary_tables(THD *thd) for the statement to drop
> temporary tables, I think in this case, it does not matter to write it
> directly or at the end of the statement through the S-cache.
>
> So in short, I think the direct argument can be removed.
>
>> I still think there is nothing to remove in the base code, not to add
>> the new direct bool as well, but rather to convert the existing `bool
>> is_trans' into an enum having the three values.
>>
>
> I think we do not need to make is_trans into an enum. See above comment.
You mean CF_AUTO_COMMIT_TRANS don't you.
Before Alfranio's patch the `is_trans' bool served to send an event
into the requested location which were either T-cache of the direct
log.
With the patch there are 3 destination and therefore the 3 values
for/instead of `is_trans' is natural unless necessary.
In some distant future we may gain yet another 4th and so on.
As you mention yourself, 2) Incident event and some more request and some more
in future can request the direct binlogging. That means we would have
to tag with CF_AUTO_COMMIT_TRANS some more stuff than DDL:s.
Yes, callers of MYSQL_BIN_LOG::write() can check CF_AUTO_COMMIT_TRANS
and basing on that set the suggested s/bool is_trans/enum
destination/ enum appopriately.
I hope you will find these arguments sensisble.
cheers,
Andrei
