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 <s...@mariadb.org> 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
_______________________________________________
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
