Hi, Andrei,
I still don't understand this.
1. Why rgi->options_to_bin_log is only set for GTID_EVENT?
Is there a guarantee that there always be a GTID_EVENT before a
QUERY_EVENT? What if gtids aren't enabled?
2. how do you guarantee that all query events for a previous value of
options_written_to_bin_log have been applied before updating it
in the rgi?
3. and if you do guarantee it, why is it atomic? No query events =
nobody reads options_written_to_bin_log concurrently.
And why not to do a simple patch, like the one I've attached?
(note, it's an intentionally minimal patch to show the fix, practically
I'd accompany it with a small cleanup patch)
Regards,
Sergei
VP of MariaDB Server Engineering
and secur...@mariadb.org
On Sep 02, Andrei Elkin wrote:
>
> > Query_log_event cannot just randomly use some
> > options_written_to_bin_log, it must use options_written_to_bin_log
> > from the Format_description_log_event that came from the binlog file
> > where the Query_log_event was read from.
>
> [ Not to engage you into a separate discussion,
> still why 'randomly'? In the quoted description p.1 says
> the Query_log_event uses `options_written_to_bin_log` from the
> immediately preceding-in-binlog FD. ]
>
> > If rli->relay_log.description_event_for_exec is not that
> > Format_description_log_event, and assuming that the correct
> > Format_description_log_event is not stored anywhere, the Query_log_event
> > has to remember options_written_to_bin_log itself,
>
> It's an option, but the chosen one avoids such remembering per each
> Query event. Instead ...
>
> > there's not much we can do about it.
> >
> > Or it can apply the mask in the constructor and store not flags2 but
> > the new value of thd->variables.option_bits.
> >
> > Or it can extend flags2 to cover all OPTIONS_WRITTEN_TO_BINLOG.
>
> ... of taking care of individual events the patch basically goes along
> the base to use a placeholder that
> A. lasts (unlike FD instance) the whole slave session time, and
> B. the stored `options_written_to_binlog` is guaranteed to be a copy
> of the most recent FD before the Query
>
> To help you to understand the method, here is a sequence diagram,
> where on the top line
> the slave SQL thread, its Relay-log-info object, a similar to that Worker's
> instance,
> and a Worker thread.
>
>
> SQL thread RLI/RGI Worker RGI Worker
> | . . .
> --->+ read FD . . i
> | . . .
> +-------------->
> stores options
> .
> --->+ read Query
> | .
> +-------------------------->+
> finds Worker RGI
> to store in theree a copy FD.options
> | .
> +-------------------------------------------->+
> schedules Query Worker |
> . |
> .<----------------+
> reads options |
> | applies
> ... Query
>
> It must be clear the A holds.
> To prove B let after Query a second FD^2 is read by the SQL thread and
> that FD^2 is from a different version server.
> Then the SQL thread always have a wait-for-workers-to-become-idle
> condition, so execution would go like this:
>
> --->+ read Query . ...
> | . |
> +-------------------------->+ |
> finds Worker RGI |
> to store in theree a copy FD.options |
> | |
> --->+ read FD^2 | applying of
> | | Query
> ... |
> ... wait for Worker to complete Query |
> ... |
> +<============================================|
> |
> +-------------->.
> | stores FD^2
> | options
> ...
>
> So FD^2 can't affect queries from earlier binlogs.
>
> I am all ears to hear about your feedback.
>
> Cheers,
>
> Andrei
diff --git a/sql/log_event.cc b/sql/log_event.cc
index 0aeec20b2ea..7cf27438700 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -1509,7 +1509,7 @@ Query_log_event::Query_log_event(const uchar *buf, uint
event_len,
switch (*pos++) {
case Q_FLAGS2_CODE:
CHECK_SPACE(pos, end, 4);
- flags2_inited= 1;
+ flags2_inited= description_event->options_written_to_bin_log;
flags2= uint4korr(pos);
DBUG_PRINT("info",("In Query_log_event, read flags2: %lu", (ulong)
flags2));
pos+= 4;
diff --git a/sql/log_event.h b/sql/log_event.h
index 82cfe0abfa6..b031fec665d 100644
--- a/sql/log_event.h
+++ b/sql/log_event.h
@@ -2099,7 +2099,7 @@ class Query_log_event: public Log_event
flags2==0 (5.0 master, we know this has a meaning of flags all down which
must influence the query).
*/
- bool flags2_inited;
+ uint32 flags2_inited;
bool sql_mode_inited;
bool charset_inited;
diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc
index 2cae9f60e4e..02fb8d9dd80 100644
--- a/sql/log_event_server.cc
+++ b/sql/log_event_server.cc
@@ -1728,7 +1728,7 @@ int Query_log_event::do_apply_event(rpl_group_info *rgi,
OPTIONS_WRITTEN_TO_BIN_LOG must take their value from
flags2.
*/
- ulonglong mask=
rli->relay_log.description_event_for_exec->options_written_to_bin_log;
+ ulonglong mask= flags2_inited;
thd->variables.option_bits= (flags2 & mask) |
(thd->variables.option_bits & ~mask);
}
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
