[mythtv] Convoluted program expiry and deletion

[Ian Dall]

I have been trying to debug the program deletion code (Ticket #10704).
However, this post is really about how convoluted the program deletion
actually has become and some suggestions for small improvement.

As I was debugging I took some notes about how the process works (or
doesn't). It looks to me like the "Change delete behaviour so that we
always use the deleted recording group" commit
f78f9992d754390fa42f109e5139b8eaf224d076, has left some artefacts which
may have made sense once, but now seem pointless. The whole process
seems excessively complicated.

Here are the notes I made. Sorry that they are long. That is the point
really. The following is just the main path, not all the branches to
deal with corner cases etc. Also this analysis stops at the point where
the actual DeleteThread is started (and so doesn't address the "slow
truncate" code).

The processing bounces between threads, processes (and hosts).

On Master
Autoexpire thread runs continuously sleeping periodically:
1. Autoexpire::RunExpirer(): loops every 60 seconds. Call
   UpdateDontExpireSet(). If time to run autoexpire has expired, check
   programs in the deleted group and deletes the files and database
   entries (if they have aged enough, depending on settings) and
   eventually calls ExpireRecordings().

2. Autoexpire::ExpireRecordings(): constructs expireList (an ordered
   list of expirable programs)

3. Autoexpire::ExpireRecordings(): loops through expireList, pushing
   programs onto deleteList until enough free space is predicted to
   occur when the programs are eventually deleted.

4. Autoexpire::ExpireRecordings(): calls SendDeleteMessages() 

5. AutoExpire::SendDeleteMessages(): raise an AUTO_EXPIRE event for
   each program and record a key for the program in deleted_set.

Main event handler
6. MainServer::customEvent(): AUTO_EXPIRE events cause call to DoHandleDeleteRecording()

7. MainServer::DoHandleDeleteRecording(): justexpire is set because
   programs are not yet in "Deleted" group.

8. MainServer::DoHandleDeleteRecording(): If on a master backend
   signal the slave to delete the program with DELETE_RECORDINGS commands.

On slave

9. MainServer::HandleDeleteRecording(): calls DoHandleDeleteRecording()

10.  MainServer::DoHandleDeleteRecording(): justexpire is still true

11. MainServer::DoHandleDeleteRecording(): put program in "Deleted"
   group and set autoexpire to 9999 reply to master and return. These
   programs should be (really) deleted the next time the autoexpire thread runs.

Actual deletion of the files and database entries for the deleted
programs occurs as follows:

On Master

12. AutoExpire::ExpireQuickDeleted() or ExpireOldDeleted(): Calls
   FillDBOrdered to create an expireList.

13. AutoExpire::FillDBOrdered(): queries the database to get all the
   programs in the "Deleted" group, but then calls IsInDontExpireSet()
   to determine whether to skip the program.

14. AutoExpire::IsInDontExpireSet(): checks if key is in dont_expire_set

15. AutoExpire::ExpireQuickDeleted() or ExpireOldDeleted(): Calls SendDeleteMessages() 

16. AutoExpire::SendDeleteMessages(): raise an AUTO_EXPIRE event for
   each program and record a key for the program in deleted_set.

Main event handler
17. MainServer::customEvent(): AUTO_EXPIRE events cause call to DoHandleDeleteRecording()

18. MainServer::DoHandleDeleteRecording(): justexpire is false this
   time through because the program is in the "Deleted" group.

19. MainServer::DoHandleDeleteRecording(): If on a master backend
   signal the slave to delete the program with DELETE_RECORDINGS commands.

On slave

20. MainServer::HandleDeleteRecording(): calls DoHandleDeleteRecording()

21. MainServer::DoHandleDeleteRecording(): justexpire is still false and
    we are not a master so fall through and start the actual delete thread.


Notes:

a) This two phase process of sticking the programs in the "Deleted"
   group, then actually deleting the file and removing the database
   entry some time later is broken when the master and slave are the
   same on the same hostname. In this case MainServer::DoHandleDeleteRecording()
   after testing if the recording hostname is the same as the master
   hostname execution falls through to start the delete thread and
   programs are deleted without ever being stuck in the "Deleted"
   recording group as is apparently intended.

b) The deletion of programs in the "Deleted" group in step 1 is broken.
   This is because:

   The dont_expire_set referred to in step 14 is initialized from
   deleted_set when UpdateDontExpireSet() is run in step 1 above.

   The deleted_set is is added to in step 5, above, but entries are
   never removed. In fact as far as I can understand, deleted_set is
   never initialised either. So programs are never actually deleted.

   This is Ticket #10704

c) There is no point in going to the slave for steps 9-11. This is just
   a database update and all the information needed is on the master.

   The code to go to the slave the second time (steps 20,21) seems a bit
   strange. Go to the slave if it is up, otherwise try and delete it from
   the master. Why not "if the recording is available on the master,
   delete it, otherwise go to the slave to delete it"?

d) It would be nice to have some consistent terminology/naming.
   eg DoHandleDeleteRecording, the first time through is actually not
   deleting anything it is expiring the recording. Even the second time
   through it starts a new DeleteThread to do the deletion, so
   DoHandleDeleteRecording is pretty much a misnomer.

I hope this is some use to someone. Is there some place where internals
are documented and stuff like this could be placed?

Regards,
Ian


-- 
Ian Dall <ian at beware.dropbear.id.au>