Bug #54605 Xid_log_event::do_apply_event() should update future_group_master_log_pos
Submitted: 18 Jun 2010 6:22 Modified: 2 Nov 2010 21:16
Reporter: Yoshinori Matsunobu (OCA) Email Updates:
Status: Won't fix Impact on me:
None 
Category:MySQL Server: Replication Severity:S3 (Non-critical)
Version:5.0, 5.1, 5.5 OS:Any
Assigned to: Zhenxing He CPU Architecture:Any
Tags: Contribution, replication

[18 Jun 2010 6:22] Yoshinori Matsunobu
Description:
Currently Xid_log_event::do_apply_event() (::exec_event() in 5.0) 
does not update rli->future_group_master_log_pos, but it should.
Without updating future_group_master_log_pos here, 
InnoDB gets a position of *before* Xid event (generated by implicit/explicit commit), not *after* xid event. 

How to repeat:
Read Xid_log_event::do_apply_event() in log_event.cc 

Suggested fix:
int Xid_log_event::do_apply_event(Relay_log_info const *rli)
{
  bool res;
  /* For a slave Xid_log_event is COMMIT */
  general_log_print(thd, COM_QUERY,
                    "COMMIT /* implicit, from Xid_log_event */");
+  const_cast<Relay_log_info*>(rli)->future_group_master_log_pos= log_pos;
  if (!(res= trans_commit(thd)))
  {
    close_thread_tables(thd);
    thd->mdl_context.release_transactional_locks();
  }
  return res;
}
[18 Jun 2010 8:06] Sveta Smirnova
Thank you for the report.

Which exact version of MySQL 5.1 do you use?
[18 Jun 2010 8:11] Yoshinori Matsunobu
@Sveta,
The latest (5.5.4, 5.1.47, and 5.0.91).
[22 Jun 2010 7:17] Sveta Smirnova
Thank you for the feedback.

Verified as described. Diff is for 5.5 series.
[28 Jun 2010 12:55] Kristian Nielsen
I am wondering, what exactly is the bug here?

It seems that future_group_master_log_pos is not actually used:

$ find . -type f -print0|xargs -0 fgrep future_group_master_log_pos
./sql/log_event.cc:  const_cast<Relay_log_info*>(rli)->future_group_master_log_pos= log_pos;
./sql/log_event.cc:    const_cast<Relay_log_info*>(rli)->future_group_master_log_pos= log_pos;
./sql/log_event.cc:  const_cast<Relay_log_info*>(rli)->future_group_master_log_pos= log_pos;
./sql/rpl_rli.h:  ulonglong future_group_master_log_pos;

It is assigned in a couple places, but never read (as far as I can determine).

Maybe the real fix is to just remove this (old/unused/bogus?) code?
[2 Nov 2010 21:16] Konstantin Osipov
Won't fix since this is 5.0 only.
[26 Nov 2012 15:20] Florent Guiliani
As of 5.5.28 the 'problem' is still there.

No one seems to make use of future_group_master_log_pos, even though the comment was mentioning InnoDB.

I wonder what InnoDB does instead then? I am writing a storage engine and I would need it during the execution of a XID log event.