Uploaded image for project: 'CiviCRM'
  1. CiviCRM
  2. CRM-12014

After renewal, membership status reverted to "Lapsed" or "Expired" by Membership status processor

    Details

    • Type: Patch
    • Status: Done/Fixed
    • Priority: Minor
    • Resolution: Fixed/Completed
    • Affects Version/s: 4.2.8
    • Fix Version/s: 4.3.0
    • Component/s: None
    • Labels:
      None

      Description

      This appears to be caused by a race condition and can therefore be hard to reproduce. Our client's current configuration and use level leads to this problem in around 2% of their online membership renewals.

      Frequency of membership status processor scheduled job runs: Always (equivalent to every 15 minutes)
      Frequency of membership renewals: up to around 70-80 per day

      Activity logs indicate the following patterns for all of these "problem" membership renewals:
      a. The renewal contribution is made while the "Membership status processor" scheduled job is running.
      b. The membership status is updated to "Current" when the contribution is made.
      c. 15 minutes later, the status is reverted to "Lapsed" or "Expired" (whatever it was before the renewal).

      I believe that in rare cases, the contribution is happening during the processing of CRM_Member_BAO_Membership::updateAllMembershipStatus(). This leads to the following race condition:
      1. updateAllMembershipStatus() fetches membership properties from the database. The membership is in Lapsed status with end date sufficiently in the past (e.g., 2012/10/01). It stores these properties in the $memberParams array.
      2. Unknown to updateAllMembershipStatus(), the online donation occurs in a separate process, and the database tables are updated so that the status is "current" and the end date is in the future (e.g., 2013/10/01).
      3. updateAllMembershipStatus() calls api('membership', 'calc') which calculates the status as "Current" based on the end date as stored in the database. Since this is different from the status stored in $memberParams, updateAllMembershipStatus() believes it needs to update the status. So it saves the membership with the new "Current" status, and – and here's the problem – with all the membership properties stored in $memberParams, including the old end date (e.g., 2012/10/01). We now have the membership stored with "current" status and an end date in the past.
      4. On the next scheduled job run, updateAllMembershipStatus() again calculates the membership status and determines that the it should be "Lapsed", based on the saved end date (2012/10/01). It updates the status accordingly. We now have a membership with an end date in the past and Lapsed status.

      I suggest we adjust CRM_Member_BAO_Membership::updateAllMembershipStatus() so that when it updates the membership status (step 3 above), it ONLY writes the status to the database, instead of re-writing all the values it has stored in $memberParams.

      The attached patch covers this by simply unsetting all but the essential elements of the $memberParams array. I'm not sure if there might be a more elegant approach.

        Attachments

          Activity

            People

            • Assignee:
              ravish.nair Ravish Nair
              Reporter:
              allenshaw Allen Shaw
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: