MySQL patches by Codership

Possibly dead code in wsrep_recover()

Bug #1335074 reported by Alexey Kopytov
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MySQL patches by Codership
New
Undecided
Teemu Ollakka
Percona XtraDB Cluster moved to https://jira.percona.com/projects/PXC
Status tracked in 5.6
5.5
Won't Fix
Low
Unassigned
Percona XtraDB Cluster moved to https://jira.percona.com/projects/PXC future-5.5
5.6
Fix Released
Low
Kenn Takara
Percona XtraDB Cluster moved to https://jira.percona.com/projects/PXC 5.6.29-25.15

Bug Description

The following code in wsrep_recover() is suspicious:

void wsrep_recover()
{
  if (!memcmp(&local_uuid, &WSREP_UUID_UNDEFINED, sizeof(wsrep_uuid_t)) &&
      local_seqno == -2)
  {
    char uuid_str[40];
    wsrep_uuid_print(&local_uuid, uuid_str, sizeof(uuid_str));
    WSREP_INFO("Position %s:%lld given at startup, skipping position recovery",
               uuid_str, (long long)local_seqno);
    return;
  }
... /* Run the actual GTID recovery procedure */
}

It looks like the intention was to prevent running GTID recovery when both local_uuid and local_seqno got changed from their initial values via wsrep_start_position. That is, ignore wsrep_recover when wsrep_start_position is also specified.

But the actual logic seems be the reverse: that code would be executed when local_uuid == WSREP_UUID_UNDEFINED and local_seqno == -2.

However, local_seqno is initialized to WSREP_SEQNO_UNDEFINED (i.e. -1) and is never assigned the value of -2. Which essentially makes that if() branch dead code, so GTID recovery is always performed, regardless of whether wsrep_start_position is specified or not.

Alex Yurchenko (ayurchen)
Changed in codership-mysql:
assignee: nobody → Teemu Ollakka (teemu-ollakka)
Revision history for this message
Raghavendra D Prabhu (raghavendra-prabhu) wrote :

Providing "--wsrep-start-position='00000000-0000-0000-0000-000000000000:-2' --wsrep-recover" on command line triggers that path:

WSREP: Position 00000000-0000-0000-0000-000000000000:-2 given at startup, skipping position recovery

However, benefits/necessity of it are unclear.

Given, wsrep_recover() is called only when --wsrep-recover is used and
wsrep-start-position given with that doesn't change anything, the code seems
vestigial.

Also, if seqno is -1, whatever is provided by wsrep-start-position is not
considered either, instead values in grastate are used.

Revision history for this message
Kenn Takara (kenn-takara) wrote :

Issue:
Some leftover special case code in the wsrep-recover code path. This
code is generally unreachable. It requires a sequence number of -2, but
sequence numbers start at -1 and are incremented (as well as a all-zero
UUID). So the only way to reach this code is to explicitly set the
wsrep_start_position to this magic value.

Solution:
Removed special case code in wsrep_recover().
Add a test case to check for special case value

also PXC-500 : Possibly dead code in wsrep_recover()

Revision history for this message
Shahriyar Rzayev (rzayev-sehriyar) wrote :

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PXC-500

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.