Page MenuHomePhabricator

Batch queries and job queue for onEchoGetDefaultNotifiedUsers function
Closed, ResolvedPublic

Description

As @Bawolff pointed out,

  1. Queries to user table should be done in batches, as opposed to 1 by 1. Especially if there are a lot of subscribers.
  2. How does this function scale. Will there ever be more than say 20,000 subscribers to a newsletter? More than 100,000 subscribers? Might have to put things like this in the job queue if the number of subscribers is large.

Event Timeline

Tinaj1234 claimed this task.
Tinaj1234 raised the priority of this task from to Normal.
Tinaj1234 updated the task description. (Show Details)
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 27 2015, 3:19 PM

btw, to clarify, when I mean batch user queries, I mean use something like UserArray::newFromIds

For 2, whether or not that's important depends on the max number of people you expect to ever subscribe to a newsletter. Currently the signpost has 872 subscribers which I imagine serves as a good upper bound [for now], and that is still in the range where the function performs ok. So its probably ok to leave the function as it is for initial deployment, with a code comment saying it might have to be split up in the future if the extension becomes wildly popular.

Qgil lowered the priority of this task from Normal to Lowest.Aug 28 2015, 11:30 AM
Qgil added a subscriber: Qgil.

If 1000 subscribers is an OK limit, I think we could start with that assumption. We will have time to react if a newsletter if reaching that limit (unless this extension is so successful that we get newsletters with 5000 subscribers overnight, but in that case we will have other problems). :)

Suggesting Lowest, as in patches welcome but we will focus on something else for now.

I'm guessing this is talking about the implementation of the onEchoGetDefaultNotifiedUsers hook? :)

I'm guessing this is talking about the implementation of the onEchoGetDefaultNotifiedUsers hook? :)

yeah.

If 1000 subscribers is an OK limit, I think we could start with that assumption. We will have time to react if a newsletter if reaching that limit (unless this extension is so successful that we get newsletters with 5000 subscribers overnight, but in that case we will have other problems). :)

The function would probably be ok even up to 5000 subscribers. I'd start to be concerned if there was something with 10,000 subscribers (Maybe. These numbers are all pulled out of my hat, I don't really know)

Tinaj1234 removed Tinaj1234 as the assignee of this task.Oct 1 2015, 11:05 AM
Tinaj1234 set Security to None.

First part of this was done in https://gerrit.wikimedia.org/r/241070. Regarding the second part, I believe Echo already uses the job queue for sending notifications (unless we explicitly set it to not use the job queue). Hence closing this as resolved.

Glaisher closed this task as Resolved.Oct 1 2015, 3:43 PM
Glaisher claimed this task.

However, on Wikimedia wikis, the job queue is used on group0 wikis only currently. So we might need to change that in the future (if it's not done on Echo by then) but it should work for initial deployment. See T101050 and T99573.