← Back to team overview

maria-developers team mailing list archive

Re: [Commits] 2f4a0c5be2c: Fix accumulation of old rows in mysql.gtid_slave_pos

 

andrei.elkin@xxxxxxxxxx writes:

> The 10.1 patch is good.

Thanks!

> > Maybe moving deletions from record_gtid() into the background thread is too
> > big a change for 10.1/10.2 ? But we could use the current patch for 10.1,
> > and do the more complicated patch only in 10.3 (or 10.4?).
> >
> > What do you think?
> 
> Let's stick your plan.

Ok, great. I'll push this patch (if you have no further questions/comments),
and work on a new patch for 10.3 which moves the delete logic into the
replication background thread.

> But one question,

>>> @@ -662,8 +674,7 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,

>>> +    if (rgi)
>>> +    {
>>>        thd->mdl_context.release_statement_locks();
>>> +      /*
>>> +        Save the list of old gtid entries we deleted. If this transaction
>>> +        fails later for some reason and is rolled back, the deletion of those
>>> +        entries will be rolled back as well, and we will need to put them back
>>> +        on the to-be-deleted list so we can re-do the deletion. Otherwise
>>> +        redundant rows in mysql.gtid_slave_pos may accumulate if transactions
>>> +        are rolled back and retried after record_gtid().
>>> +      */
>>> +      rgi->pending_gtid_deletes_save(gtid->domain_id, elist);
>>> +    }
>>>      else
>>> +    {
>>>        thd->mdl_context.release_transactional_locks();
>
>>> +      rpl_group_info::pending_gtid_deletes_free(elist);
>>> +    }
>
> Why it's not
>           rpl_group_info::pending_gtid_deletes_clear()
> called?

Just because in the else-branch we did not run pending_gtid_deletes_save().
pending_gtid_deletes_clear() frees the list previously stored in rgi by
pending_gtid_deletes_save(), while pending_gtid_deletes_free() just frees an
explicitly given list.

Thanks,

 - Kristian.


Follow ups

References