V2

[ccrvlh]

@selwin I've been thinking about some changes, but I always run into backwards compatibility issues. Have you given some though on a possible V2? Are you on DIscord so we could sync on this?

[selwin]

I'm ok with starting to plan for a v2 release, there's a lot of things that I'd like to deprecate.

I'm not on Discord because I find it too distracting. If you don't mind, I'd rather we do the planning here, we can also create an issue to list all of the things that we want to deprecate or change and work towards the 2.0 release.

Some things that I want to rework:

• exc_handler and exception_handlers argument on Worker.__init__: https://github.com/rq/rq/blob/master/rq/worker.py#L172
• Requiring Redis server >= 5.0
• prepare_for_work and log_job_description args on Worker.__init__

[ccrvlh]

Sure, no worries. Some comments:

• Add a new RQ object
  This should centralize everything

• Remove static management methods from Queue/Worker/Job
  A lot of management tasks are currently being made by the classes themselves, so what happen is we have a Queue class that handles all queues, same for Job and Worker. This is perfect use case for the RQ object, and will make the classes cleaner.

• Make methods private
  There are a lot of methods that are public now, and make it harder to make non breaking changes. We should expose a small amount of methods in the public API and make the rest of them private.

• Remove all static class attributes
  We currently have a lot of configuration being done statically (namespaces are a great example). By having a centralized class we could make config a lot more dynamic and pass the config around (rpkak has done some great work on this).

• Change the default Worker behavior
  The current behavior is for the worker to fork(), this is an opinionated way of handling jobs, with pros and cos. Forking is fine and can be ideal for some use cases, but also not for others. I think it makes more sense to have the current SimpleWorker as the default Worker and then have a ForkWorker

• Remove SimpleWorker
  Related to the above

• Create a new ForkWorker
  Related to the above

• Remove HerokuWorker
  “Heroku” is infrastructure provider, just as we won’t implement an AWSWorker or GCPWorker or XProviderWorker I would remove this completely. We could have a separate package to rq that adds support to Heroku or have the worker on the docs.

• Remove the RoundRobinWorker and RandomWorker
  RoundRobin and Random are strategies to handle the dequeue process, it doesn’t say much about “working”. This can be seen in the worker code itself: both have a single line of code that changes a very small behavior of the worker. I would have a strategy argument on the Worker and work method, add a strategy option to the CLI, and remove those two workers completely. A lot more flexibility and less overhead (can use this strategy with multiple workers, don’t need to handle a two separate workers).

• Concurrency Support
  Add support for concurrency: Worker, ForkWorker ProcessWorker ThreadWorker AsyncWorker and PrefokWorker

[ccrvlh]

Doing some work in parallel here

[selwin]

Lots of great ideas, in general, we can introduce new features over time so we can keep the releases small-ish and stable. For me, 2.X is more about the release where we'll drop backwards compatibility for deprecated features and not where we release X number of huge features.

Add a new RQ object
We can add this even before 2.X

Remove static management methods from Queue/Worker/Job
A lot of management tasks are currently being made by the classes themselves, so what happen is we have a Queue class that handles all queues, same for Job and Worker. This is perfect use case for the RQ object, and will make the classes cleaner.

I'll have to review a more concrete proposal on which methods will be moved to which class. But I think this discussion is for after we introduce the RQ class.

Make methods private
There are a lot of methods that are public now, and make it harder to make non breaking changes. We should expose a small amount of methods in the public API and make the rest of them private.

In general, I haven't had much trouble with this so far, this is okay to do but not a high priority.

Remove all static class attributes
We currently have a lot of configuration being done statically (namespaces are a great example). By having a centralized class we could make config a lot more dynamic and pass the config around (rpkak has done some great work on this).

Yes.

Change the default Worker behavior
The current behavior is for the worker to fork(), this is an opinionated way of handling jobs, with pros and cos. Forking is fine and can be ideal for some use cases, but also not for others. I think it makes more sense to have the current SimpleWorker as the default Worker and then have a ForkWorker

I don't think we'll gain much from switching the default. I'd still argue that the forking mechanism is a good default. We can keep the Worker class so we don't introduce any backward compatibility, but still introduce ForkingWorker and update the docs to use SimpleWorker and ForkingWorker. I think we can also think of a better name, because I'd like to change os.fork() to os.spawn(), which is the recommended method to fork a new process on most modern systems.

Remove the RoundRobinWorker and RandomWorker
RoundRobin and Random are strategies to handle the dequeue process, it doesn’t say much about “working”. This can be seen in the worker code itself: both have a single line of code that changes a very small behavior of the worker. I would have a strategy argument on the Worker and work method, add a strategy option to the CLI, and remove those two workers completely. A lot more flexibility and less overhead (can use this strategy with multiple workers, don’t need to handle a two separate workers).

Agree, maybe we can introduce a DequeueStrategy.

Concurrency Support
Yes, this is something I'd like to work on too. Actually I started working on this here master...watcher (but this is a big feature and there's no ETA). The idea is to be able to do rq worker --num-workers=3.

[ccrvlh]

About the release:

I think there are trade-offs. What I’ve found was that quite a few adjustments would be needed to fix typing for example. Sometimes those are trivial, but sometimes they would demand changing the approach. We could do that slowly, but than we would be breaking a small thing every release - even things there were not supposed to be public (Local for example). I have the impression that a lot of users use custom classes, so it’s hard to tell what a small adjustment will break or not. This is the main reason why I felt that going the V2 route with a lot more flexibility would be worth it. That does not mean changing 100% of the code (at all), but it does mean less restrictions while working things out.

About the static management methods
You’re right. I feel that it wouldn’t even be needed to actually remove those tasks, but “recommended” approach should be to do things through the main RQclass, and there are quite a few benefits: no need to pass the connection around multiple times (as we do when using a job static method, or multiple queues for example), single import when using multiple queues, no need to have a pre-build empty job with a connection to get a job status among others. I’ll have a prototype ready and we can discuss on top of it.

Private methods
Related to the first point, I guess the most relevant thing here is more freedom to work on the codebase. Since most of the methods are public, we always need to assume that they are being used, even if they weren’t built with that in mind. So I don’t think there’s an issue per se, but it would definitely make it easier to handle the codebase in the future.

Default Worker Behavior
I totally understand this is not trivial, so let me try to elaborate my argument, since I do believe this is very relevant.

Whenever we run something (anywhere), we are most of the time running (1) synchronously (2) in the same process thread (3) in the main thread. So the default behavior of most people is to think of tasks as just regular code.

When the default is a fork, this breaks 2 of 3 internal assumptions folks have when doing things. While working on the issues this is quite clear: quite a few doubts on memory sharing, libraries with conflicts, database pooling and other topics. And automatically the “horse” concept is introduced (which doesn’t exist in a single process single thread architecture). So the fork brings a strong opinion on how the worker should be run: on a child process, that should be killed afterwards.

With that in mind, it’s natural to have “non-forking-worker”, and because the Worker carries an opinion on how to run tasks, we then need to call it SimpleWorker which doesn’t actually mean much, since simplicity not necessarily translates in single process / single thread - we could actually argue that it’s simpler to run a fork, since we can just kill it afterwards so it’s “simpler” to clean. So we get into a point the naming is fairly arbitrary, and kind of confusing.

When looking at the worker’s code, it’s pretty clear the division between whats “fork-related” and what’s just worker’s code (there was even a suggestion to split the class into Worker and WorkHorse) and now we are going into a direction where our codebase is a lot more complex, because a Worker may or may not use a WorkHorse. This is specially true if we consider concurrency on future workers (Async, Gevent, Process etc) - do they use the work horse or not? If they do, then why is it not in the main class (should be something every worker uses), if they don’t, then why the main worker class needs a WorkHorse?

Looking at workers would be interesting to implement, none of them uses it. They have they own way of handling things, managing the pool, handling signals etc. And when we use the SimpleWorker we are actually just ignoring about 5-6 methods in the worker class (that were specifically created for the fork(). In reasonable to think that the worker should be the minimum common denominator between all workers (minimizes the unused code in the main worker class when implementing a new worker, and minimizes the number of methods that need to be reimplemented by a new worker).

Also, there’s quite a bit of code that is specifically designed for the work horse (commands for example). So forking - which is highly opinionated is being considered all around the codebase. So as of now, new implementations need to consider a lot of side effects.

So a default non forking worker would solve (1) windows problem, (2) db pooling problem, (3) code loading problem, (4) “unknown” strategy problem (5) unused code problem (6) obvious execution strategy problem (7) naming problem (8) agnostic execution implementation problem.

On the other hand, it would introduce (1) the task reloading problem and (2) name change problem. But considering this was written 12 years ago, I think that’s a small price to pay for a more extensible architecture.

I'll keep working on the V2, and come back with a more solid proposition on all those topics.

[ccrvlh]

@selwin a draft doc on the main RQ class: https://github.com/lowercase00/rq/blob/feature/app/rq/main.md