Fixed User Promotions sometimes fail

[guiltar]

I've noticed that user group promotions fail sometimes.
I can't catch the exact cases when it happens since all my tests was ok.
But I found the possible reason in the code.
The promotion transaction is not safe and may be logged as done well when promotion failed to apply.

XenForo_Model_User

    public function addUserGroupChange($userId, $key, $addGroups)
    {
        .........................
        $db->query('
            INSERT INTO xf_user_group_change
                (user_id, change_key, group_ids)
            VALUES
                (?, ?, ?)
            ON DUPLICATE KEY UPDATE
                group_ids = VALUES(group_ids)
        ', array($userId, $key, $addGroups));
 
        $success = $this->_applyUserGroupChanges($userId, $oldGroups, $newGroups);
   
        XenForo_Db::commit($db);
 
        return $success;
    }

Even if $success is false for some reason, table xf_user_group_change contains failed promotion. So, the next time it won't be applied since the failed userGroup will be contained in var $oldGroups. I suggest to replace this place by:

        if ($success = $this->_applyUserGroupChanges($userId, $oldGroups, $newGroups))
        {
              $db->query('
                  INSERT INTO xf_user_group_change
                      (user_id, change_key, group_ids)
                  VALUES
                      (?, ?, ?)
                  ON DUPLICATE KEY UPDATE
                        group_ids = VALUES(group_ids)
                ', array($userId, $key, $addGroups));
        }

Also, I suggest to replace chunk from XenForo_Model_User

    protected function _applyUserGroupChanges($userId, array $oldGroupStrings, array $newGroupStrings)
    {
        ....................
        $dw->setSecondaryGroups($secondaryGroups);
        $dw->save();
 
        return true;
    }

by next line to guarantee, that promotions applied without errors.

    protected function _applyUserGroupChanges($userId, array $oldGroupStrings, array $newGroupStrings)
    {
        ....................
        $dw->setSecondaryGroups($secondaryGroups);
 
        return $dw->save();
    }

 

[Jake Bunce]

Not in direct response to the code, but this will help if you need to force users to go through the promotion again:

http://xenforo.com/community/threads/user-group-promotions-only-run-once.31276/#post-357121

 

[guiltar]

Not in direct response to the code, but this will help if you need to force users to go through the promotion again:

http://xenforo.com/community/threads/user-group-promotions-only-run-once.31276/#post-357121

Yeah, thanks. I've used similar queries to fix the problem. I'm just reporting to improve the engine and make promotions more safe.

 

[Apt Irrelevance]

Not in direct response to the code, but this will help if you need to force users to go through the promotion again:

http://xenforo.com/community/threads/user-group-promotions-only-run-once.31276/#post-357121

My board has ~245,000 members, most of which were imported along with everything else when we migrated to Xenforo. Many active users still have not received any promotions despite being active for weeks. This seems to be happening to hundreds of active members.

Ideally, I'd like to force the promotions to run on all members, regardless of whether they are currently active. How could I do that?

 

[Jake Bunce]

My board has ~245,000 members, most of which were imported along with everything else when we migrated to Xenforo. Many active users still have not received any promotions despite being active for weeks. This seems to be happening to hundreds of active members.

Ideally, I'd like to force the promotions to run on all members, regardless of whether they are currently active. How could I do that?

The cron runs hourly. You can manually run it too:

Admin CP -> Tools -> Cron Entries -> User Group Promotions -> Controls: Run

But even if you manually run the cron it only processes users who have been active in the last 3 days. If you want to process all users regardless of activity then you need to use this updated version of the cron:

http://xenforo.com/community/thread...cron-running-out-of-memory.31541/#post-362657

That will allow you to remove 'last_activity' => array('>', XenForo_Application::$time - 86400 * 3) from the file to process all users without memory problems. But with 245k users you might already be having memory problems requiring you to use that updated version of the cron.

 

[Apt Irrelevance]

Thank you!

We are definitely running into memory issues on the site, mainly during high traffic times of the day. I see a lot of memory errors while doing other things like updating user profiles, adding users to user groups, using the spam cleaner, and sometimes just banning a user. Are there other things we can do to help with memory issues in these situations?

 

[Jake Bunce]

Thank you!

We are definitely running into memory issues on the site, mainly during high traffic times of the day. I see a lot of memory errors while doing other things like updating user profiles, adding users to user groups, using the spam cleaner, and sometimes just banning a user. Are there other things we can do to help with memory issues in these situations?

There is known high memory usage when editing users, groups, and nodes when you have a very large number of groups and nodes. That is actually an optimization (extra processing at save time for maximum runtime performance), but if it is causing memory errors then you need to increase the limit.

http://xenforo.com/community/thread...-nodes-updating-usergroups.28811/#post-333911

Or remove excess groups and nodes.

The spam cleaner can suffer from memory problems when you try to clean users with excessive numbers of posts. There are some manual cleanup options listed here:

http://xenforo.com/community/resources/dealing-with-forum-spam.980/

The trophy cron can also have memory problems with large numbers of users. There is an updated version of that cron in the same thread:

http://xenforo.com/community/thread...cron-running-out-of-memory.31541/#post-362657

Those are all of the common sources of memory problems.

 

[Apt Irrelevance]

I will try all of those, thanks!

We currently have 23 user groups (Is that considered a high number?) and 405 nodes.

We do have a fairly large chunk of forums that are inactive. I think we can probably consolidate a lot of them.

 

[Jake Bunce]

I will try all of those, thanks!

We currently have 23 user groups (Is that considered a high number?) and 405 nodes.

We do have a fairly large chunk of forums that are inactive. I think we can probably consolidate a lot of them.

That's fairly high, yes. You can expect lag time when editing a user with that many groups and nodes.

 

[Apt Irrelevance]

10 of them are actually just user group promotion levels based solely on post count. I could eliminate them altogether and just modify the message_user_info template to check post counts instead of user groups to display the user level. Unless that would cause a significant slowdown, that is.

 

[Jake Bunce]

10 of them are actually just user group promotion levels based solely on post count. I could eliminate them altogether and just modify the message_user_info template to check post counts instead of user groups to display the user level. Unless that would cause a significant slowdown, that is.

Yes indeed:

http://xenforo.com/community/threads/picture-that-changes-with-post-count.27017/#post-321379

 

[Mike]

Fixed now - also applies to rollbacks.