[mythtv-commits] Ticket #7762: Autoshutdown slave feature enhanced
MythTV
mythtv at cvs.mythtv.org
Wed Feb 10 15:40:19 UTC 2010
#7762: Autoshutdown slave feature enhanced
-------------------------------+--------------------------------------------
Reporter: gmembre@… | Owner: cpinkham
Type: enhancement | Status: assigned
Priority: minor | Milestone:
Component: MythTV - General | Version: unknown
Severity: medium | Mlocked: 0
-------------------------------+--------------------------------------------
Comment(by cpinkham):
Thanks for the feature enhancement, but there are a few things that I
think need to be corrected before this patch can be accepted. Here are
the issues I noticed:
1) You can't instantiate a Scheduler anywhere but the master. Just
calling DisableScheduling() still lets the main parts of the scheduler run
but it doesn't fire off any recordings. The
Scheduler code is not guaranteed to be 'multi-backend-safe', meaning that
it potentially could
do things to the database that should only be done from the master
Scheduler. I don't think
there is any code like this currently, but it's not something we want to
deal with in the
future. Even the "mythbackend --printsched" functionality opens a
connection to the master
scheduler and downloads the list of scheduled recordings. If
Scheduler::CheckShutdownServer() is the only thing you need from the
scheduler, then you can make this method static and call it as
"bool status = Scheduler::CheckShutdownServer(0, idleSince,
blockShutdown);" in your code.
2) There are a couple reasons that EncoderLink::GoToSleep() can return
false other than when the slave returns a non "OK" status, so you can't
rip out the while loop that I have that sets the slave's status to
undefined. It sounds like EncoderLink::GoToSleep() and
PlaybackSock::GoToSleep() both need to return an int status. Something
like -1 == error, set status to undefined, 0 == OK slave is going to
sleep, and 1 == slave was busy and can't go to sleep.
3) The changes to MainServer::HandleGoToSleep() seem wrong in a couple
ways. a) you are calling SendResponse() twice in the case where 'status'
is true, but the SleepCommand being empty. b) you don't need to get the
value of blockSDWUwithoutClient prior to calling CheckShutdownServer. The
'blockshutdown' parameter to CheckShutdownServer is passed by reference,
and updated inside the method so that the caller can get the value. You
can just declare "bool blockShutdown;" without the GetNumSetting since you
don't need the value of blockShutdown.
4) please observe myth coding style and put brackets { and } on their own
lines.
5) the Feb 9 version of the patch still uses the AllowSlaveToSleep setting
in one place.
6) since Scheduler::WakeUpSlaves is now checking if a slave IsAwake(), it
probably should also check IsWaking() in case we just told it to wake up a
minute ago and the slave's mythbackend isn't up and running yet.
I think that's about all I noticed from my first pass through the patch.
--
Ticket URL: <http://svn.mythtv.org/trac/ticket/7762#comment:6>
MythTV <http://www.mythtv.org/>
MythTV
More information about the mythtv-commits
mailing list