RFC: Extend the JobQueue QueueInterface with countFailed

(Daniel Lienert) #1

Hey Flowpackorz,

We use the ContentRepositoryQueueIndexer to index a huge amount of nodes to Elasticearch. The package works that way, that it creates indexer-jobs that index batches of nodes and then one index-switch-job to switch the index to the new one.

If we have an error in the indexing configuration somewhere, the batches fail and in the worst case, the switching job switches to an empty index.

Thats why I want to implement a safeguard, so that the switch job checks for failed jobs in the queue and decides on that if it should witch or not:

To make this work, I need a method to countFailed to count failed jobs, like we have a count()for ready jobs. This method need to go to the QueueInterface and needs to be implemented by the queue implementations.

When on it, I would also add a countReserved().

What do you think about adding the methods?

(Bastian Waidelich) #2

From reading your description this makes sense to me, thought it’s a breaking change probably (unless we introduce a new interface).
Did you already have a look at what that would mean for our provided implementations, i.e. what it means to extend BeanstalkdQueue, RedisQueue and DoctrineQueue accordingly?

(Daniel Lienert) #3

Changing the interface is a breaking change and will require a major release 3.0 of the common package.

Had a look at all three implementations. Its pretty easy for all backends to provide the data.

(Daniel Lienert) #4

And here are some PRs to make it happen:

Prepare the beanstalk package for a common major release - needs a bugfix release:

Adjust the interface in the common package:

Implement the feature in the backend implementations:

(Soren Malling) #5

I don’t know how if this should/could go into this interface changes. but what about a way to fetch jobs that are reserved, failed and ready? Returning a iterable object of Messages. Use full for any display such as GUI and CLI stuff. Or maybe this goes into something seperate ? I’m seeing a need for a uniformed way of fetching and there states

(Bastian Waidelich) #6

You’re the man!
What about the beanstalkd implementation? Did you already find a way to make that work with it or is that impossible with the beanstalkd backend?
In the latter case I’d propose to add the methods to an additional interface that the packages can implement optionally (this would also allow for a minor version bump)

That would be possible with some (basic) implementations but certainly not with all. It’s probably easier to implement such extended integrations with the actual backend directly (i.E. I built a small package retrieving metadata from the Beanstalkd backend that the other backends wouldn’t provide)

(Daniel Lienert) #7

Need to setup a beanstalkd for testing. But it seems possible and it will follow.

I really would like to avoid that. The “count” method of the existing interface should be deprecated, as it not clearly states what it counts. Some of the packages are not updated for years now. Having a new major version with a cleared up interface does no harm. And implementations we are not aware of could stick to the 2.0 interface.

As Bastian said, that would be possible with DB (doctrine) and Cache (Redis) based implementations but beanstalkd seems not to have methods to do that.

(Soren Malling) #8

Thanks both of you - I will create some own logic for fetching the jobs :slight_smile:

(Daniel Lienert) #9

That would be a pitty though :frowning: In this case , as @bwaidelich mentioned add another interface to JobQueue Common with these methods, and lets implement this interface in the backend packages which support it.
I think its way better in this case to have that on the base package than implementing own logic.

(Bastian Waidelich) #10

I think my response was a bit misleading.
I’m all for adding the new methods. If we change the interface we don’t have to deprecate count() though as implementations need to be adjusted anyways. We can just replace it.

Regarding an additional interface, imagine this:

interface CountableQueueInterface {
  public function countReady(): int;
  public function countReserved(): int;
  public function countFailed(): int;

And then

class DoctrineQueue implements QueueInterface, CountableQueueInterface ...

and so on.
This would just be a way to introduce this without breaking and – more importantly – allowing implementing packages to keep up with new versions even if they don’t support counting.

edit: I slightly adjusted the code example to allow for polymorphism

(Soren Malling) #11

I think we can add a new interface in a 3.1 or something and if a backend implementation implements that, it can be used :slight_smile: Nothing wasted, will work on it as something that can be used further down the road

(Bastian Waidelich) #12

In general it’s always a slippery slope with those “interface packages”: Do we want to add features that not all implementations support or keep the purpose of this package slim and focused. Both have advantages and disadvantages…

(Soren Malling) #13

Just replying my self with the idea of how this interface could look, and be implemented by each backend implementation

interface ListableQueueInterface extends QueueInterface {
  public function getReadyMessages();
  // and so on ...

(Soren Malling) #14

I personally see it as important to ask, how can we support/extend the functionality, for example fetching a list of jobs to be used in a dashboard, cli tool etc. without requiring touching the slim jobqueue task. This just being a example for the JobQueue package, I’m totally in favor of keeping it slim and simple and have the possibilites of extending the functionality in a non-hacky-way

(Bastian Waidelich) #15

FYI: I added some tests and adjusted the Beanstalkd Implementation with https://github.com/Flowpack/jobqueue-beanstalkd/pull/11