Possibly dead code in wsrep_recover()

Bug #1335074 reported by Alexey Kopytov on 2014-06-27
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MySQL patches by Codership
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
5.6
Fix Released
Low
Kenn Takara

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.

Changed in codership-mysql:
assignee: nobody → Teemu Ollakka (teemu-ollakka)

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.

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()

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  Edit
Everyone can see this information.

Other bug subscribers