Merge Users Plugin

codinghorror · April 15, 2019, 5:20am

Either, the goal is to avoid a mindless “…and then you type YES” step.

Dannii · April 27, 2019, 2:28am

I’ve updated the plugin to let you choose whether to message the target user or not.

A better confirmation dialog is still to come.

JagWaugh · June 5, 2019, 4:57pm

Does the rake task which actually makes the change run in the background, or does the arrival of the PM mean that the merge is 100% complete?

I’m asking because we’re having some performance problems today, which persist after ./launcher rebuild. The only change we made recently was implementing the Merge Users Plugin. I merged a couple of users, but that was back on Sunday.

We’re seeing a flurry of unicorn workers, and postgres tasks.

Dannii · June 5, 2019, 10:03pm

The message is sent once the merge task is completed. There should be no performance changes once the task is complete.

JagWaugh · July 13, 2019, 5:47am

I had another try this week.

On our forum, I watched /admin/upgrade#/processes and triggered a merge. Once the merge starts, processes shows a few pg SELECT and DELETE tasks. These tasks can take up to 97% CPU, and they are still there after the PM that the merge has completed arrives - in some cases even a few hours after the PM (merging accounts with +/- 1500 posts).

What I did differently this time was to keep an eye on the pg processes, and didn’t start the next merge until they had died back down to their normal level (below 1%). The first time I did some merging I just started the new merge once the PM arrived.

We had no performance issue this time.

Dannii · July 13, 2019, 12:09pm

Hmm, interesting.

This plugin calls the built in UserMerger class, waits for it to complete, and then sends the PM. You should see the admin log at the same time the PM is sent. I thought that would mean it would actually be fully complete, but I guess it could have some lingering queries. @gerhard does this sound plausible?

github.com

discourse/discourse/blob/888e68a1637ca784a7bf51a6bbb524dcf7413b13/app/services/user_merger.rb#L11-L27


def merge!
  update_username
  move_posts
  update_user_ids
  merge_given_daily_likes
  merge_post_timings
  merge_user_visits
  update_site_settings
  merge_user_attributes


  DiscourseEvent.trigger(:merging_users, @source_user, @target_user)
  update_user_stats


  delete_source_user
  delete_source_user_references
  log_merge
end

JagWaugh · July 13, 2019, 1:50pm

I suspect it must be the user stats (which might be a low priority job).

Or possibly looking for the references to delete them.

gerhard · July 13, 2019, 5:10pm

The user merging doesn’t enqueue any jobs and does most of the merging by directly updating the database. When it’s done, it’s done. It doesn’t trigger background jobs.

I certainly wouldn’t recommend merging more than one user at a time though.

codinghorror · July 13, 2019, 9:35pm

Should there be a warning somewhere about attempting to merge more than one user at the same time?

JagWaugh · July 14, 2019, 7:04am

I waited for the PM to arrive each time before I started the next merge, so I was only running one merge at any given time.

The merge may not trigger background jobs, but there are definitely pg select and delete tasks still running after the merge PM claims the task is complete - there is a correlation between how long those (high CPU) jobs take to finish after the PM and the number of “Posts Created” the source username has. Merging source accounts with less than about 100 posts is fine, but from about 200 posts or more there is a prolonged “spike” in pg activity.

The source username accounts I am merging are all “orphans” from our mbox import - if a user on our old forum changed their email at some point then the import yielded a separate username for each email, the oldest email for that account was assigned the nominal username, then each subsequent email for that account has an integer appended to the username.

I’m not sure if it makes a difference, but none of the source usernames have ever been active since we migrated to Discourse - I have noticed that I see 10

SiteSetting.*something*_url has been deprecated and will be removed in the 2.3 Release.

messages in /logs for each source account that I merge.

In practice it’s not really a problem as long as I wait for those pg jobs to die back down before I proceed to the next merge.

JagWaugh · July 17, 2019, 5:01am

I think that makes sense, if not actually blocking the merge while a previous one is still running.

I’d also like to see a checkbox to enable/disable merging the source email as a secondary on the target.

codinghorror · July 17, 2019, 9:17am

I’ve made a pull request to warn in the copy about only merging one pair of users at a time, cc @Dannii

JagWaugh · July 17, 2019, 9:42am

Would it be possible to block, rather than warn? Imagine a large site like ours with lots of duplicate users leftover from the import and multiple mods cleaning up.

Dannii · July 17, 2019, 10:53am

This mod is really quite “thin”. To truly block it would require hacking the base merge class of Discourse, or adding state tracking to the front end which I wouldn’t know how to do. Blocking other admin users would be tricky. But merging should hopefully not be a very common occurrence, and most sites wouldn’t have such a big admin team that co-coordinating these merges would be too hard. Sorry. Though if anyone could figure out a software solution I would of course accept a PR.

Thanks for the PR @codinghorror!