[WIP] Scheduler to replace chronograph

[benjaoming]

Summary

This is the initial framework for the scheduler, opening up the PR to let the rest of the team know what my thoughts were.

Terminology: Not sure if it should be named a scheduler, it might be more of a producer-consumer pattern, suggestions welcome.

TODO

If not all TODOs are marked, this PR is considered WIP (work in progress)

• Have tests been written for the new code? If you're fixing a bug, write a regression test (or have a really good reason for not writing one... and I mean really good!)
• Has documentation been written/updated?
• New dependencies (if any) added to requirements file
• Add an entry to CHANGELOG.rst

Reviewer guidance

Comment on anything you like

Issues addressed

#4892

[benjaoming]

@rtibbles regarding the discussion on whether it's possible to put a schedule_download boolean field in the items table, here is some more insight to what I was thinking.

[rtibbles]

As the release blocker issues that motivated this PR are now fixed, I would like to suggest that this PR gets targeted to develop rather than 0.16.x.

Let me know if you think otherwise.

[benjaoming]

So yes, we have a working 0.16 for now, and that seems fine, but I have to say that the way this has been fixed seems shaky. But let's test it thoroughly. I'll keep this PR within reach, and I think it should definitely get introduced for 0.17.

I'm not really in favor of the idea of having one huge json file that we add and remove jobs to and from. "It's not a database" would be my main summary of the problem. It doesn't check for uniqueness, you can't really query it to see which jobs are already scheduled, you cannot avoid duplicating download items. I'm not sure if you can cancel an individual download. But I appreciate greatly that thanks to the UI's way of enabling and disabling buttons, the likeliness of these things occurring is low.

It's a bit like we just got rid of all these JSON files and weird in-memory caches, and now they are somehow reappearing? :) I'm not saying that things haven been properly implemented, but just that out of experience this will very likely cause issues.

Some other issues that I think we need to be aware of, in making this choice:

• We only download 1 video at a time.
• Deleting files doesn't happen in a separate background task, and so deleting 3000 files on a flash device (which likely takes time) might not succeed for various reasons.
• The download progress is initiated from user activity, however if the server is restarted or the job otherwise gets interrupted, the queue persists, it's progress is still in the db, but what will happen to resuming? The queue persisting is good, but I'm unsure if we can really say that the chronograph, progress logs, video download queue json file is guaranteed to pickup the download? And if the user has to resume it, they will mark all videos again, now having double-up?
• Rewriting the full JSON file after each video download (video_queue.save()) and marking the video as available in the database seems more expensive than updating the database just once.

[rtibbles]

I am happy to admit that this is not the nicest download mechanism, at all.

I am not sure I see a huge advantage for parallelising downloads, as they are all coming from the same end point, and they are all of a decent size, so the startup, spindown cost of the request is not as big a problem as it might be for a large number of small files.

Unless the way chronograph operates has been radically changed, the job should restart at server restart - has this been altered?

I think this implementation is cleaner, but I am wary of introducing a new feature at this point, whose main advantage seems to be more efficient parallelising of downloads, something that I don't see a strong need for. Happy to hear if there is a strong user demand for this?

[benjaoming]

Nope, nothing has changed in the way that the chronograph works, I just can't from a theoretical perspective grasp if it works.. the only way to do that, is to actually start and stop the server and verify it :)

...I forgot one thing: Thread-safety of the video download queue. There is none, right? So if a write operation is snuck in around the client-side UI intentions, then we might have multiple write operations on the queue file.

@rtibbles we might find a user demand for the parallel downloads, however I think that the other issues are of a technical nature and won't receive user appreciation. I just wanna get them fixed because I know they are there, so I can accept that 0.16 ships if there's some nice manual test verifications, but then I also think we should prioritize getting this stuff cleaned up ASAP. I have a feeling that we are running the risk that this codebase will continue to produce issues that are very hard to resolve.

Best decision would be to give the chronograph the boot entirely and stop trying to do this :)

[benjaoming]

Closing in favor of a PR that targets develop.