Allow modules to check whether the current worker is configured to run background tasks.

[reivilibre]

Base: develop

Original commit schedule, with full messages:

1. Add module API function to ask whether the current worker is configured for background tasks

[clokep]

This seems like a fairly low-level thing to expose -- can you expand a bit for why we need this? Is this to be paired with #15993 inside a module? Would it make more sense to have the same run_on_all_instances that looping_background_call has?

[reivilibre]

It's to be paired with looping_background_call for registering an event handler that may be useful in conjunction with the background call.
I get it's a bit weird but the alternative is having every module need to accept its own configuration for which worker to use as the background worker (often we do this and default to the master process).

(Maybe I should just push the configuration and logic up to the module and use run_on_all_instances=True but only register the background call on one worker?)

[clokep]

(Maybe I should just push the configuration and logic up to the module and use run_on_all_instances=True but only register the background call on one worker?)

I'm confused -- why can't you just let it do run_on_all_instances=False? It sounds like the goal is to run it on a single worker?

[reivilibre]

I need to register an event handler for new events (on_new_event) on the same worker as the one running the looping_background_call jobs, so either need to let the background worker be in charge, or alternatively ignore the logic for run_on_all_instances=False and control it entirely within the module.

[clokep]

I feel like this requires the module writer to know a lot about how synapse workers work, which isn't ideal. I don't see any other short-term solution to this though.

[reivilibre]

I feel like this requires the module writer to know a lot about how synapse workers work

Whilst the current situation is not really good, I'm not sure this particularly adds any extra points — module writers already have to know about workers if they don't want to duplicate work.

All this does is to expose an 'is this a good place to run background jobs?' flag rather than expecting every module, that needs to do slightly more than just run basic tasks on an interval, to write their own by accepting a worker name in the config file and comparing it to the one they find at runtime (usually defaulting to the master process since there is no other sensible default available!).

I'd say it's about as wrong as run_on_all_workers=False existing — which to be fair I'm not as pleased about either :-)

At the end of the day, if you don't like it (which I can see why one wouldn't), I can trivially re-roll the worker name configuration solution into what I'm writing right now, like at least 3 other modules do. I am not too fussed either way, so the choice is yours :)

[clokep]

I think without a better way handling the logic in one spot (instead of having each module hand-roll / copy & paste) sounds like a concrete improvement. 👍 Ship it.