maria-developers team mailing list archive

[Nikita Malyavin <nikitamalyavin@xxxxxxxxx>]

Hi Sergei!

Thanks for the review. I have added some changes according to your notes.
Also, few answers to you below

On Fri, 8 Jul 2022 at 01:50, Sergei Golubchik <serg@xxxxxxxxxxx> wrote:

> > @@ -3212,7 +3212,7 @@ int handler::create_lookup_handler()
> >    handler *tmp;
> >    if (lookup_handler != this)
> >      return 0;
> > -  if (!(tmp= clone(table->s->normalized_path.str,
> table->in_use->mem_root)))
> > +  if (!(tmp= clone(table->s->normalized_path.str, &table->mem_root)))
>
> that's wrong, the lifetime of a lookup handler is one statement, it's
> destroyed at the end. You cannot keep allocating new handlers on the
> table->mem_root for every statement.
>
Ouch! I have confused these two. Yes, I needed the statement lifetime. But
the problem is
I was setting a new mem_root for each event, just in case. That was a
mistake. First, I had no case when something was allocated there. Second,
per-event data should be anyway allocated
differently, not on thd->mem_root.

So I removed event_mem_root completely. There was nothing similar in
replication code.

> --- a/sql/log_event_server.cc
> > +++ b/sql/log_event_server.cc
> > @@ -6007,6 +6007,11 @@ int Rows_log_event::do_apply_event(rpl_group_info
> *rgi)
> >       if (m_width == table->s->fields && bitmap_is_set_all(&m_cols))
> >        set_flags(COMPLETE_ROWS_F);
> >
> > +    Rpl_table_data rpl_data{};
> > +    if (rgi) rgi->get_table_data(table, &rpl_data);
> > +
> > +    if (!rpl_data.copy_fields)
> > +    {
> >      /*
> >        Set tables write and read sets.
> >
> > @@ -6027,17 +6027,19 @@ int
> Rows_log_event::do_apply_event(rpl_group_info *rgi)
> >      bitmap_set_all(table->read_set);
> >      if (get_general_type_code() == DELETE_ROWS_EVENT ||
> >          get_general_type_code() == UPDATE_ROWS_EVENT)
> >        bitmap_intersect(table->read_set,&m_cols);
> >
> >      bitmap_set_all(table->write_set);
> >
> >      /* WRITE ROWS EVENTS store the bitmap in m_cols instead of
> m_cols_ai */
> >      MY_BITMAP *after_image= ((get_general_type_code() ==
> UPDATE_ROWS_EVENT) ?
> >                               &m_cols_ai : &m_cols);
> >      bitmap_intersect(table->write_set, after_image);
> >
> >      this->slave_exec_mode= slave_exec_mode_options; // fix the mode
> > +    }
>
> I'm sorry, I don't quite understand. "some fields are excluded based on
> m_cols value" - what columns were excluded? What was the value of m_cols
> here?
>

Yes, it was a replicated columns bitmap, not value here:)

-- 
Yours truly,
Nikita Malyavin