Andrei Elkin wrote:
> 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.
>
To check if a query is a DDL, yes
> 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.
>
I don't think so, and actually I think after fixing the problem of NDB,
we can also make DDLs through S-cache.
> 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.
>
No, Incident event will not be flagged with this, I think Incident event
is very special, it's not a problem to handle this case specially in
MYSQL_BIN_LOG::write().
> 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'd like to do this within MYSQL_BIN_LOG::write() instead.
>
> I hope you will find these arguments sensisble.
>
I see your point :), whether remove the 'direct' argument or not is more
a matter of personal coding preference, I'm OK with or without it.
> cheers,
>
> Andrei
>
