QueueClassic/queue_classic

Wishlist for the next major revision

bronson opened this issue · 2 comments

bronson commented

For discussion, this is a list of potential improvements to Queue Classic. Most of them are breaking changes that require a major revision bump. Would be nice to do them all at once.

  • Discourage inheriting from QC classes (using inheritance makes it harder to make backward-compatible changes in the future). Try to satisfy everyone with callbacks and configuration instead.
  • If it's unreasonable to remove inheritance entirely, reduce the exposed API by marking parts of classes private and declaring explicit base classes. Right now, with everything public, most modifications are technically breaking changes.
  • Minimum of Ruby 2 / Rails 4
  • Rename exposed methods and rake tasks to be more consistent. (example: #256)
  • Change schema to make monitoring easier: #252
  • Simplify creating QC::Queue objects: #249
  • Ensure migrations don't break if I remove QC from my gemfile (will update when I file the PR)
  • Maybe remove forking? Honest question: #207 (comment))
  • Maybe just use ActiveRecord's connection? Since AR allows multiple connections, does QC need its own implementation?

very unlikely:

Anything else? Anyone have opinions?

I'd like to expand any of these that make sense to PRs.

bronson commented

I played with the second-to-last item in the list: removing the QC::ConnAdapter class and just using ActiveRecord's connection (or a plain PGconn when AR isn't available).

 lib/queue_classic.rb              |  27 ++++++++--
 lib/queue_classic/conn_adapter.rb | 111 --------------------------------------
 lib/queue_classic/worker.rb       |  21 +++++---
 test/helper.rb                    |   1 +
 4 files changed, 39 insertions(+), 121 deletions(-)

So far it's a nice simplification. I still need to make tests pass again but it's getting there. Some assumptions:

  • mutex synchronization isn't needed. This seems OK to me because Rails and AR are concurrent now. Parallelization should produce a nice speedup I'd have to do a torture test to see if this is still correct.
  • #249 is applied. Breaking changes everywhere.

Result

I'm not sure if it's worth pursuing further...

This change removes a bunch of code and makes things nicer for people using AR/ActiveJob (especially in the migrations!). However, it also removes some functionality from the standalone case (mostly stuff that AR already does like logging and exception handling).

Personally, I only use QC in Rails apps sharing the AR connection, so this cleanup appeals to me a lot. But if people care about preserving all QC's functionality when used standalone then, I gotta say, this doesn't make much sense.

bronson commented

Closing because our project is now using Que. Still, queue_classic is a great project. Hope it all goes well.