Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync job results in heavy database activity #41

Open
wmortada opened this issue Mar 2, 2022 · 8 comments
Open

Sync job results in heavy database activity #41

wmortada opened this issue Mar 2, 2022 · 8 comments

Comments

@wmortada
Copy link

wmortada commented Mar 2, 2022

We noticed some unusual database activity on a website that has this plugin installed. We believe that this was due to the civi_wp_member_sync_refresh job which syncs the status for every member. This code appears to loop through every user account and perform the sync. For this particular site there are more than 90,000 users so this is quite a resource intensive operation.

In this particular case we decided to simply turn this job off and rely on the sync occurring when the user logs in or an admin updates the CiviCRM contact record. However, we think this code could be optimised to work for sites with a large number of users. Potentially the job could be set to run in batches.

public function sync_all_wp_user_memberships() {
// Kick out if no CiviCRM.
if ( ! civi_wp()->initialize() ) {
return;
}
// Make sure CiviCRM file is included.
require_once 'CRM/Core/BAO/UFMatch.php';
// Get all WordPress Users.
$users = get_users( [ 'all_with_meta' => true ] );
// Loop through all Users.
foreach ( $users as $user ) {
// Skip if we don't have a valid User.
if ( ! ( $user instanceof WP_User ) ) {
continue;
}
if ( ! $user->exists() ) {
continue;
}
// Call login method.
$this->sync_to_user( $user->user_login, $user );
}
}

The above function also appears to overlap with the functionality of sync_all_civicrm_memberships. I'm wondering if they could be combined? Particularly as the latter has the ability to process contacts in batches.

public function sync_all_civicrm_memberships() {

@christianwach
Copy link
Owner

@wmortada Thanks for the report - yes sync_all_wp_user_memberships() is really old code predating my fork of the original plugin. I'd recommend running the plugin without using the WordPress pseudo-cron whether or not there are lots of WordPress Users. It's generally not that useful since the changes are largely on the CiviCRM side and synced to WordPress.

I'll leave this issue open as a cookie trail for others - along with the warning that this functionality is likely to be (a) removed or (b) rewritten.

@wmortada
Copy link
Author

wmortada commented Mar 2, 2022

Thanks @christianwach

@wmortada
Copy link
Author

wmortada commented Mar 2, 2022

Just to check, I presume that when a membership expires in CiviCRM, this won't be synced to the WordPress user until they next log in?

@christianwach
Copy link
Owner

It should sync the moment the CiviCRM Membership status changes as a result of the cronjob. So when they log in, they will do so with their "expired" role or capabilities.

@wmortada
Copy link
Author

wmortada commented Mar 2, 2022

Thanks for confirming. I wasn't sure if the cron job would trigger it.

@christianwach
Copy link
Owner

I've begun to address this in a79b232 and will remove in future versions since it's not actually that useful.

@wmortada
Copy link
Author

wmortada commented Apr 6, 2022

The website in questions is running with significantly less load on the database since we switched this job off.

@christianwach
Copy link
Owner

Good to hear @wmortada thanks for reporting back. The setting will be off by default for new installs. I have yet to decide how to handle existing ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants