[mythtv] [mythtv-commits] Ticket #3196: Autoexpire and backend fail to heed maxnewest = 0, deletes old programs that shouldn't and records new ones

[Chris Pinkham]

* On Thu Mar 15, 2007 at 01:21:08AM -0500, Subbyz wrote:
> On Wed, 14 Mar 2007, Chris Pinkham wrote:
> 
> >The expirer shouldn't have to care.  If the scheduler is doing its
> >job then the expirer knows that it just has to delete the maxepisodes+1
> >show in starttime descending order.
> 
> I'm not sure why you are insisting that the maxnewest flag is only for use 
> by the scheduler.  The maxepisodes and maxnewest settings are logically 
> closely related in how shows are dealt with after being recorded.

I'm insisting because that's the way it is.  The scheduler is in charge
of whether to record new episodes or not and the expirer is in charge
of keeping X maxepisodes around.  It's as simple as that.  The scheduler
won't record any new episodes if we are at the max _unless_ maxnewest
is turned ON because then it knows that the expirer will handle deleting
the oldest one.  The expirer's _only_ job is to keep us from exceeding
maxepisodes.

I added the maxepisodes and maxnewest feature over 3 years ago and you're
the first person that I know of that hasn't been able to understand
that max means max.  If you set the max to 5, you can expect only 5
episodes at most to be on the Watch Recordings screen next time you
look at that program, unless you have preserved episodes via the popup
menu on that same screen.

	http://svn.mythtv.org/trac/changeset/1811/trunk/mythtv/programs/mythbackend/autoexpire.cpp
	http://svn.mythtv.org/trac/changeset/1845/trunk/mythtv/programs/mythbackend/scheduler.cpp

> With that being the case, it seems to be a little inconsistent if you are 
> saying that the maxnewest flag is (and should be) solely a scheduler 
> setting, while it's ok for the maxepisodes setting to be used by both the 
> scheduler and the expirer.  If the maxepisodes setting is used by both, why 
> can't they both use the maxnewest too?

I wrote the code, I can be inconsistent when it makes sense.  The answer
to why can't they both use maxnewest is because the expirer doesn't need
to.  maxnewest just lets the scheduler record a new episode when we hit
the max.  That is its only reason for existence.  The expirer doesn't need
to look at maxnewest because it doesn't care because its only job is to
keep maxepisodes recordings around.

If you go putzing with the scheduled recording, you can expect shows to
be deleted if you reduce the max.

Read the help text for the maxnewest setting:

	"Delete oldest if this would exceed the max episodes"  and
	"Don't record if this would exceed the max episodes"

So, if maxnewest is ON, we delete the _oldest_ episode if we go over
the max.  If maxnewest is OFF, we don't record any new episodes.  Sounds
like it describes exactly the way the code is written.

> And that is my point, and seems to be root of the problem.  I wouldn't 

The root of the problem is that you're manufacturing invalid cases because
you don't know enough about Myth to understand how to use it.  The preserve
field was added because someone like you came along and said "I want to
keep an episode and I don't want it to count against my max."

> expect it to delete shows that I've already recorded, when they are set to 
> not autoexpire AND I don't have maxnewest set.  I agree the max limit 

AutoExpire has nothing to do with maxepisodes other than the fact that
the maxepisodes code is in autoexpire.cpp.  AutoExpire is done for
disk space reasons and programs are deleted for maxepisodes because
they exceeded the maximum number of episodes that you told Myth to keep
around.  The autoexpire flag has nothing to do with whether a show can
be deleted because it exceeds the maxepisodes count.

> should keep more shows from being recorded, but it should NOT affect 
> deletion of episodes that I may have specifically increased the limit in 
> order to get. Once they are on disk, and I have autoexpire on them set to 
> 0, then I do not expect them to disappear from under me for any reason 
> unless absolutely necessary (like running out of disk space).

Myth isn't Kreskin, it doesn't know that when you set maxepisodes to 10
3 weeks ago and then to 5 now, that you want to keep those extra 5 episodes
that recorded 2 weeks ago.  You have to tell it that by setting the
preserve flag.  If you have maxepisodes set to 5 now, that's what Myth
honors.

> Ok then, how does one go about getting the backend to automatically set the 
> preserve flag on all episodes recorded under a certain rule?  I may have 
> missed it, but I don't see a setting that appears to do that.

You don't.  If you wanted it set automatically then why not bump up the
maxepisodes count?  You're not making sense.  Automatically setting the
preserve flag would negate the effect of turning on maxepisodes because
nothing would be allowed to delete if it was over the max.  You may as well
turn off maxepisodes and just manually delete things when you've watched
them.

> Come to think of it, I haven't even seen a way to set the preserve flag 
> manually for an individual recording.  Is that a new flag that isn't fully 
> supported yet?  Even if there is, I certainly hope that you're not 

You just have to look for it.  Under the INFO popup menu on the Watch
Recordings screen, there is a "Storage Options" submenu.  This setting is
under Storage Options because that's where maxepisodes/maxnewest are on
the scheduled recording editor.  On the Storage Options submenu, you'll
see a "Preserve this episode" button (or "Do not preserve this episode"
if preserve is already on).

> suggesting that having to go and manually set a flag on a bunch of 
> recordings is an acceptable way to get the desired behavior of having it 
> not delete existing recordings.

You already told it to delete existing recordings when you bumped
maxepisodes down.  The help text for maxepisodes says this:

	"Keep at most X episodes"

That's not that hard to comprehend.  If you tell it 5, then it keeps
5 unless some are marked as preserve, in which case it will keep more
than 5.  If you had it set to 10 then change it to 5, then common sense
says that it will delete 5 to keep you at the new limit of 5.

> I realize that maxepisodes overrides the autoexpire flag, and that it is 
> behaving as it was designed.  But I think it should be changed so that it 
> only does so when the maxnewest flag is true.  Otherwise, the maxepisodes 
> should only affect the scheduler, and not the expirer.

I may think the world is flat, but that doesn't make it so.  The scheduler
records, the expirer deletes.  They both share the responsibility of
keeping us at a max of maxepisodes, but only the scheduler can record
new shows, so it is the only one that needs to check maxnewest which by
definition of the help text allows us to record new episodes and let
the oldest get automatically deleted.

> Another big reason it is a problem (in addition to being more work for the 
> user) is because it exposes a race condition to the user.

If the user changes maxepisodes down when it specifically says "Keep at most
X episodes" and expects more than X to still stick around then they need
a hit with the clue stick.

> Suppose I had done as my previous example and bumped up the recordings 
> temporarily for a long weekend or something like that.  After the backend 
> has gotten a few extra shows, I want to bump it back down, but wait, I 
> can't do that yet because if I do the expirer will delete programs that I 
> want and even may be in the middle of watching.

Yes you can, just preserve 5 of those episodes, otherwise Myth will do
exactly what you tell it and will "Keep at most X episodes".

> In my specific case, the recording rule is getting all programs with a 
> genre of "Documentary".  Now, with Discovery channel and several other 
> educational channels, one program or another of type "Documentary" is on 
> pretty much at all hours of the day.  So, even though it is a hassle, since 
> I don't want programs to be deleted out from under me, I try to follow your 
> suggestion of not reducing the limit until I've cleared out some programs 
> to get it below the limit.

Use the preserve flag, it's not that hard.  It would only take you a minute
to go through and mark enough episodes to allow you to reduce maxepisodes
to its previous value again without worry of losing programs.

> So, after I've watched and deleted a program, now I immediately have to go 
> and change the recording rule to decrease it's maxlimit by one, or the 
> backend will start recording another show of "Documentary" at the next hour 
> or half hour.  If I'm not quick enough, the backend may already have 
> started a recording, and when I do go get the setting changed, now the 
> expirer will delete an older program I haven't yet finished watching, which 
> is exactly what I was trying to avoid.  So I've done a lot more work, and 
> ended up being hit or miss on wether I worked around the problem that is 
> currently inherent in the design.

Use the preserve flag, this is exactly what it is for, when you want to keep
more than maxepisodes around for whatever reason.  Maybe you saw an episode
you liked and don't want it to disappear because of the max, you can preserve
it.  Maybe you want to lower your maxepisodes count.  Preserve a few shows
and you don't have to worry about them being automatically deleted when
you bump maxepisodes down.

> Making the user do more work just to get around inconsistencies and race 
> conditions in the code just seems like a bad idea to me.  Especially when 
> the fix is so simple and logical.

Dude, you're beating a dead horse.  This feature works as intended and over
the 3.5 years that it has existed, no one has wasted so much time on an
argument such as this.  Myth does exactly what you tell it, that is simple
and logical.  Having the expirer do something fuzzy in the background and
try to base it's decision on what you were thinking 3 weeks ago when you
bumped the max up or 2 weeks ago when you bumped it down or a week ago
when you bumped it up again or 3 days ago when you bumped it down again
for the second time is not logical or simple.

--
Chris (who doesn't plan on commenting on this anymore and doesn't plan on
       changing existing functionality other than fixing the scheduler bug)