← Back to team overview

maria-developers team mailing list archive

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

 

The 10.1 patch is good.

But one question,

> Hi Andrei!
>
> Can you review this patch?
...
>> @@ -646,9 +658,9 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
>>                        /* `break' does not work inside DBUG_EXECUTE_IF */
>>                        goto dbug_break; });
>>  
>> -    next= elist->next;
>> +    next= cur->next;
>>  
>> -    table->field[1]->store(elist->sub_id, true);
>> +    table->field[1]->store(cur->sub_id, true);
>>      /* domain_id is already set in table->record[0] from write_row() above. */
>>      key_copy(key_buffer, table->record[0], &table->key_info[0], 0, false);
>>      if (table->file->ha_index_read_map(table->record[1], key_buffer,
>> @@ -662,8 +674,7 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id,
>>        not want to endlessly error on the same element in case of table
>>        corruption or such.
>>      */
>> -    my_free(elist);
>> -    elist= next;
>> +    cur= next;
>>      if (err)
>>        break;
>>    }
>> @@ -686,18 +697,31 @@ IF_DBUG(dbug_break:, )
>>        */
>>        if (elist)
>>        {
>> -        mysql_mutex_lock(&LOCK_slave_state);
>>          put_back_list(gtid->domain_id, elist);
>> -        mysql_mutex_unlock(&LOCK_slave_state);
>> +        elist = 0;
>>        }
>>  
>>        ha_rollback_trans(thd, FALSE);
>>      }
>>      close_thread_tables(thd);
>> -    if (in_transaction)
>> +    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?


Follow ups

References