Andrei Elkin wrote:
> He Zhenxing, good morning!
>
> > Andrei Elkin wrote:
> >> He Zhenxing,
> >>
> >> Here is what I left on #rep today.
> >>
> >> <andrei> jasonh, regarding to skipping,
> >> [15:20]<andrei> jansonh, don't you think that if an event is in a log
> it
> >> should be skippable?
> >
> > Events with LOG_EVENT_IGNORE_F flag set is automatically skipped.
> >
>
> To explain better, I meant do_shall_skip() might apply
> `rli->slave_skip_counter > 0' rule to produce EVENT_SKIP_COUNT to the
> caller.
> I.e the user could find that there is a Rotate event and count it for
> skipping with set @@global.sql_slave_skip_counter.
>
> Although it makes sense in my opinion, still there is
> the `server_id == ::server_id && !rli->replicate_same_server_id'
> original rule producing EVENT_SKIP_IGNORE verdict.
> This means that we have had a case where the user may be in trouble in
> counting how to skip manually.
> I think we shall document that `sql_slave_skip_counter' does not include
> events flagged with the new flag as well as events originated by the
> slave server itself whenever replicate_same_server_id is set to FALSE.
>
> Would you agree with that?
>
Thanks for the explaination, and I agree with you on that this behaviour
should be documented.
> >> [15:21]<andrei> jasonh, i think of LOG_EVENT_IGNORE_F that does not
> let to
> >> execute an event.
> >> [15:25]<andrei> jasonh, otherwise it's confusing: consider a flagged
> event,
> >> first we try to skip and succeed ( Log_event::do_shall_skip),
> >> but then why should we care for execution
> >> (Rotate_log_event::do_update_pos)?
> >>
> >
> > Sometimes events that are skipped still need to update the master log
> > position, so Log_event::do_shall_skip does not mean we can skip
> > execution of do_update_pos.
>
> you are right.
>
>
> >
> >> I am still in opinion we should not make the LOG_EVENT_IGNORE_F-flagged
> event
> >> automatically skippable.
>
> I meant that set sql_slave_skip_counter counter. Now I think we can
> have yet another case where an event in the relay log may not be
> counted for the manual skipping.
>
yes, since the relay log rotate event is not skippable in normal case,
it's better to stick to that to not adding more confusion.
> >
> > My intention is to invent a new flag that could be set to tell the SQL
> > thread to totoally ignore it, and use it for Rotate_log_event generated
> > by slave for relay log is just one instance of this.
> >
>
> i see and that's ok.
>
> regards,
>
> Andrei
>
> >>
> >> cheers,
> >>
> >> Andrei
> >>
> >>
> >> <hezx@stripped> writes:
> >>
> >> > Hi Andrei
> >> >
> >> > Sorry for the delay, missed your mail.
> >> >
> >> > Andrei Elkin wrote:
> >> >> He Zhenxing, hello!
> >> >>
> >> >> Thanks for making this work!
> >> >>
> >> >> I have only one concern which is about Log_event::do_shall_skip().
> >> >> Some minor comments are presented as well.
> >> >>
> >> >> > #At file:///media/sda3/work/mysql/bzrwork/b38734/5.1-rpl/
> >> >> >
> >> >> > 2661 He Zhenxing 2008-08-22
> >> >> > BUG#38734 rpl_server_id2 sync_with_master failed
> >> >> >
> >> >> > Rotate event is automatically generated and written when
> rotating binary
> >> >> > log or relay log. Rotate events for relay logs are
> usually ignored by slave
> >> >> > SQL thread becuase they have the same server id as that
> of the slave.
> >> >> > However, if --replicate-same-server-id is enabled,
> rotate event
> >> >> > for relay log would be treated as if it's a rotate event
> from master, and
> >> >> > would be executed by slave to update the
> rli->group_master_log_name and
> >> >> > rli->group_master_log_pos to a wrong value and cause
> the MASTER_POS_WAIT
> >> >> > function to fail and return NULL.
> >> >> >
> >> >> > This patch fixed this problem by setting a flag bit in
> the event to tell the
> >> >> > SQL thread to ignore these Rotate events generated for
> relay logs.
> >> >> > modified:
> >> >> > sql/log.cc
> >> >> > sql/log.h
> >> >> > sql/log_event.cc
> >> >> > sql/log_event.h
> >> >> > sql/slave.cc
> >> >> >
> >> >> > per-file messages
> >> >> > sql/log.cc
> >> >> > Add a parameter to MYSQL_BIN_LOG::new_file to distinguish
> create new file for binary log or relay log.
> >> >> > sql/log.h
> >> >> > Add a parameter to MYSQL_BIN_LOG::new_file to distinguish
> create
> >> >> > new file for binary log or relay log.
> >> >>
> >> >> Should not it be put "to distinguish create new file for binary log
> from relay log"?
> >> >>
> >> >
> >> > you're right
> >> >
> >> >> > sql/log_event.cc
> >> >> > If LOG_EVENT_IGNORE_F is set in the event flags, ignore
> it
> >> >> > sql/log_event.h
> >> >> > Add LOG_EVENT_IGNORE_F flag to Log_event flags, events has
> this flags set will be ignored by slave SQL thread
> >> >> > Add RELAY_LOG flag to Rotate_log_event flags
> >> >> > sql/slave.cc
> >> >> > Set parameter relay_log to 1 when creating new relay log
> files
> >> >> > === modified file 'sql/log.cc'
> >> >> > --- a/sql/log.cc 2008-07-22 10:41:55 +0000
> >> >> > +++ b/sql/log.cc 2008-08-22 05:06:23 +0000
> >> >> > @@ -3388,15 +3388,15 @@ bool MYSQL_BIN_LOG::is_active(const
> char
> >> >> > method).
> >> >> > */
> >> >> >
> >> >> > -void MYSQL_BIN_LOG::new_file()
> >> >> > +void MYSQL_BIN_LOG::new_file(bool relay_log)
> >> >> > {
> >> >> > - new_file_impl(1);
> >> >> > + new_file_impl(1, relay_log);
> >> >> > }
> >> >> >
> >> >> >
> >> >> > -void MYSQL_BIN_LOG::new_file_without_locking()
> >> >> > +void MYSQL_BIN_LOG::new_file_without_locking(bool relay_log)
> >> >> > {
> >> >> > - new_file_impl(0);
> >> >> > + new_file_impl(0, relay_log);
> >> >> > }
> >> >> >
> >> >> >
> >> >> > @@ -3409,7 +3409,7 @@ void
> MYSQL_BIN_LOG::new_file_without_loc
> >> >> > The new file name is stored last in the index file
> >> >> > */
> >> >> >
> >> >> > -void MYSQL_BIN_LOG::new_file_impl(bool need_lock)
> >> >> > +void MYSQL_BIN_LOG::new_file_impl(bool need_lock, bool
> relay_log)
> >> >> > {
> >> >> > char new_name[FN_REFLEN], *new_name_ptr, *old_name;
> >> >> >
> >> >> > @@ -3468,7 +3468,7 @@ void MYSQL_BIN_LOG::new_file_impl(bool
> n
> >> >> > to change base names at some point.
> >> >> > */
> >> >> > Rotate_log_event r(new_name+dirname_length(new_name),
> >> >> > - 0, LOG_EVENT_OFFSET, 0);
> >> >> > + 0, LOG_EVENT_OFFSET, relay_log ?
> Rotate_log_event::RELAY_LOG : 0);
> >> >> > r.write(&log_file);
> >> >> > bytes_written += r.data_written;
> >> >> > }
> >> >> >
> >> >> > === modified file 'sql/log.h'
> >> >> > --- a/sql/log.h 2007-10-30 08:03:34 +0000
> >> >> > +++ b/sql/log.h 2008-08-22 05:06:23 +0000
> >> >> > @@ -268,8 +268,8 @@ class MYSQL_BIN_LOG: public TC_LOG, priv
> >> >> > new_file() is locking. new_file_without_locking() does
> not acquire
> >> >> > LOCK_log.
> >> >> > */
> >> >> > - void new_file_without_locking();
> >> >> > - void new_file_impl(bool need_lock);
> >> >> > + void new_file_without_locking(bool relay_log=0);
> >> >> > + void new_file_impl(bool need_lock, bool relay_log=0);
> >> >> >
> >> >> > public:
> >> >> > MYSQL_LOG::generate_name;
> >> >> > @@ -341,7 +341,7 @@ public:
> >> >> > bool open_index_file(const char *index_file_name_arg,
> >> >> > const char *log_name);
> >> >> > /* Use this to start writing a new log file */
> >> >> > - void new_file();
> >> >> > + void new_file(bool relay_log=0);
> >> >> >
> >> >> > bool write(Log_event* event_info); // binary log write
> >> >> > bool write(THD *thd, IO_CACHE *cache, Log_event
> *commit_event);
> >> >> >
> >> >>
> >> >> I am not sure we need to refine skipping.
> >> >> Certainly it introdudes incompatibility. But does it really fixes
> anything?
> >> >>
> >> >
> >> > for Rotate_events, this is not necessary, but since this flag might be
> >> > used later for other events, I think it's better to add this for
> >> > completeness.
> >> >
> >> >> > === modified file 'sql/log_event.cc'
> >> >> > --- a/sql/log_event.cc 2008-08-06 10:41:27 +0000
> >> >> > +++ b/sql/log_event.cc 2008-08-22 05:06:23 +0000
> >> >> > @@ -725,6 +725,9 @@ Log_event::do_shall_skip(Relay_log_info
> >> >> > (ulong) server_id, (ulong)
> ::server_id,
> >> >> > rli->replicate_same_server_id,
> >> >> > rli->slave_skip_counter));
> >> >> > + DBUG_PRINT("info", ("ev->type=%d, ev->flags=%d",
> get_type_code(), flags));
> >> >> > + if (flags & LOG_EVENT_IGNORE_F)
> >> >> > + return EVENT_SKIP_IGNORE;
> >> >> > if (server_id == ::server_id &&
> !rli->replicate_same_server_id ||
> >> >> > rli->slave_skip_counter == 1 &&
> rli->is_in_group())
> >> >> > return EVENT_SKIP_IGNORE;
> >> >>
> >> >>
> >> >>
> >> >> > @@ -4054,6 +4057,8 @@
> Rotate_log_event::Rotate_log_event(const
> >> >> > #endif
> >> >> > if (flags & DUP_NAME)
> >> >> > new_log_ident= my_strndup(new_log_ident_arg, ident_len,
> MYF(MY_WME));
> >> >> > + if (flags & RELAY_LOG)
> >> >> > + Log_event::flags |= LOG_EVENT_IGNORE_F;
> >> >> > DBUG_VOID_RETURN;
> >> >> > }
> >> >> > #endif
> >> >> > @@ -4144,6 +4149,7 @@ int
> Rotate_log_event::do_update_pos(Rela
> >> >> > relay log, which shall not change the group positions.
> >> >> > */
> >> >> > if ((server_id != ::server_id ||
> rli->replicate_same_server_id) &&
> >> >> > + !(Log_event::flags & LOG_EVENT_IGNORE_F)
> &&
> >> >> > !rli->is_in_group())
> >> >> > {
> >> >> > DBUG_PRINT("info", ("old group_master_log_name: '%s' "
> >> >> >
> >> >> > === modified file 'sql/log_event.h'
> >> >> > --- a/sql/log_event.h 2008-07-31 06:24:27 +0000
> >> >> > +++ b/sql/log_event.h 2008-08-22 05:06:23 +0000
> >> >> > @@ -449,6 +449,9 @@ struct sql_ex_info
> >> >> > */
> >> >> > #define LOG_EVENT_UPDATE_TABLE_MAP_VERSION_F 0x10
> >> >> >
> >> >> > +/* Event with this flag set will be ignored by SQL thread */
> >> >> > +#define LOG_EVENT_IGNORE_F 0x20
> >> >> > +
> >> >> > /**
> >> >> > @def OPTIONS_WRITTEN_TO_BIN_LOG
> >> >> >
> >> >> > @@ -2518,7 +2521,8 @@ class Rotate_log_event: public
> Log_event
> >> >> > {
> >> >> > public:
> >> >> > enum {
> >> >> > - DUP_NAME= 2 // if constructor should dup the string
> argument
> >> >> > + DUP_NAME= 2, // if constructor should dup the string
> argument
> >> >> > + RELAY_LOG= 4 // generated by slave for relay log
> >> >> > };
> >> >> > const char* new_log_ident;
> >> >> > ulonglong pos;
> >> >> >
> >> >> > === modified file 'sql/slave.cc'
> >> >> > --- a/sql/slave.cc 2008-06-30 20:11:18 +0000
> >> >> > +++ b/sql/slave.cc 2008-08-22 05:06:23 +0000
> >> >> > @@ -4029,7 +4029,7 @@ void rotate_relay_log(Master_info* mi)
> >> >> > }
> >> >> >
> >> >> > /* If the relay log is closed, new_file() will do nothing.
> */
> >> >> > - rli->relay_log.new_file();
> >> >> > + rli->relay_log.new_file(1);
> >> >> >
> >> >> > /*
> >> >> > We harvest now, because otherwise BIN_LOG_HEADER_SIZE
> will not immediately
> >> >> >
> >> >> >
> >> >> > --
> >> >> > MySQL Code Commits Mailing List
> >> >> > For list archives: http://lists.mysql.com/commits
> >> >> > To unsubscribe: http://lists.mysql.com/commits?unsub=1
> >> >> >
> >> >>
> >> >>
> >> >> regards,
> >> >>
> >> >> Andrei
