Page MenuHomePhabricator

ReassignMentees does not enforce batch size limit for mentees that are dropped from the mentor/mentee relationship
Closed, ResolvedPublicBUG REPORT

Description

ReassignMentees reassigns the user to another mentor if possible. If not possible, the mentor/mentee relationship is dropped. To support mentors with a lot of mentees, the workload can be split into multiple jobs, with one job handling up to GEMentorshipReassignMenteesBatchSize mentees. This is implemented by having a counter incremented for each mentor; if the batch size is reached, the job stops and a new one is scheduled.

Currently, ReassignMentees does this:

  1. First, we try to assign if a new mentor is available.
  2. If not, we drop the relationship and continue with the next mentee.
  3. Otherwise, we increment the counter.
  4. The mentee is reassigned to someone else and we continue with the next mentee.

This means the counter is never incremented for mentee whose relationship is dropped. Note the testWithMenteesOverLimit test added in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/1243136 should be adjusted once we fix this (see the FIXME there).

Event Timeline

Urbanecm_WMF changed the subtype of this task from "Task" to "Bug Report".

Change #1244567 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] fix(ReassignMentees): Count all processed users into the limit

https://gerrit.wikimedia.org/r/1244567

Change #1244567 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] fix(ReassignMentees): Count all processed users into the limit

https://gerrit.wikimedia.org/r/1244567

I can verify that I see a lot of tests added on the patch. Is that enough to close out this task?