> -----Original Message-----
> From: Worley, Dale (BL60:9D30)
> Sent: Monday, April 06, 2009 12:45 PM
> To: Joly, Robert (CAR:9D30)
> Cc: Lawrence, Scott (BL60:9D30); sipX-dev
> Subject: RE: [sipX-dev] Loading the subscription DB when
> sipxrls restarts
>
> On Mon, 2009-04-06 at 08:25 -0400, Joly, Robert (CAR:9D30) wrote:
> > > On Fri, 2009-04-03 at 17:02 -0400, Scott Lawrence wrote:
> > > > It should be calling the SubscriptionDB::getInstance(), which
> > > > would load it only if the database were not already
> loaded (which
> > > it should
> > > > be in every case, since the sipXsupervisor should have
> preloaded
> > > > it before your process was started).
> > >
> > > The code in question is a combination of rev. 14244 and 14584.
> > > Essentially, it sets up SubscriptionDB::mTableLoaded. When the
> > > SubscriptionDB object is created, the code will load the
> IMDB table
> > > from the persistent file if the observed number of users
> is 1 (that
> > > is, only the current process) or if mTableLoaded is 'false':
> > >
> > > 9999 xmlscott SubscriptionDB::SubscriptionDB( const
> > > UtlString& name )
> > > 9999 xmlscott : mDatabaseName( name )
> > > 14584 dworley , mTableLoaded ( false )
> > > 9999 xmlscott {
> > > 9999 xmlscott // Access the shared table databse
> > > 9999 xmlscott SIPDBManager* pSIPDBManager =
> > > SIPDBManager::getInstance();
> > > 9999 xmlscott m_pFastDB =
> pSIPDBManager->getDatabase(name);
> > > 9999 xmlscott
> > > 9999 xmlscott // If we are the first process to attach
> > > 9999 xmlscott // then we need to load the DB
> > > 9999 xmlscott int users =
> > > pSIPDBManager->getNumDatabaseProcesses(name);
> > > 14244 rjoly if ( users == 1 || ( users > 1 &&
> > > mTableLoaded == false ) )
> > > 9999 xmlscott {
> > > 14244 rjoly mTableLoaded = false;
> > > 9999 xmlscott // Load the file implicitly
> > > 9999 xmlscott this->load();
> > > 14244 rjoly // the SubscriptionDB is not
> > > replicated from sipXconfig, as
> > > 14244 rjoly // a result, make this table appear
> > > as being loaded regardless
> > > 14244 rjoly // of the load() result.
> > > 14244 rjoly mTableLoaded = true;
> > > 9999 xmlscott }
> > > 9999 xmlscott }
> > >
> > > Currently, mTableLoaded is initialized to false, which makes the
> > > constructor always load the table. Initializing mTableLoaded to
> > > true gives the behavior we want, but it also makes mTableLoaded
> > > useless. I suspect that mTableLoaded was intended to be set by a
> > > call to some IMDB routine, but I don't think we have that code.
> >
> > I'm not clear on what is the 'behavior that we want' - what
> is broken
> > with the current behavior? The way it stands, the code will
> > initialize the subscription DB when the first user of the
> system tries
> > to gets its instance. That first user will always be
> sipXsupervisor.
> > Having said that, the way we use 'mTableLoaded' is strange for
> > subscriptionDB. With sipXconfig-replicated DBs, the state of
> > mTableLoaded set in the c'tor based on the result of the
> > 'this->load()' operation but for the subcriptionDB (and
> > registrationDB), given that they are process-initiated as
> opposed to
> > sipXconfig-replicated, the result of the load operation is not be
> > taken into account to set the value of mTableLoaded because
> for such
> > databases, absence of the corresponding xml file does not
> indicate a
> > real failure or that initial replication hasn't yet happened.
> > However, the subscription c'tor code could be cleaned up and also,
> > default values should be provided for mTableLoaded for all
> DB classes
> > but that would not affect the behavior. I'll open a minor
> tracker for that later today.
>
> The behavior we are now seeing is that the subscription IMDB
> table is loaded from subscription.xml by (more or less) every
> process that accesses it.
>
> (This can be seen by adding this patch:
>
> Index: sipXcommserverLib/src/sipdb/SubscriptionDB.cpp
> ===================================================================
> --- sipXcommserverLib/src/sipdb/SubscriptionDB.cpp (revision 15078)
> +++ sipXcommserverLib/src/sipdb/SubscriptionDB.cpp (working copy)
> @@ -84,6 +84,9 @@
> // If we are the first process to attach
> // then we need to load the DB
> int users = pSIPDBManager->getNumDatabaseProcesses(name);
> + OsSysLog::add(FAC_DB, PRI_DEBUG,
> + "SubscriptionDB::_ users = %d, mTableLoaded = %d",
> + users, mTableLoaded);
> if ( users == 1 || ( users > 1 && mTableLoaded == false ) )
> {
> mTableLoaded = false;
>
> and then looking at the resulting messages:
>
> $ grep -E 'SubscriptionDB::load loading|"SubscriptionDB::_
> users' sip*.log
> sipregistrar.log:"2009-04-03T21:23:28.196212Z":322:SIPDB:DEBUG
> :victoria-pingtel-com.us.nortel.com:SipRegistrar:B7556B90:SipR
egistrar:"SubscriptionDB::_ users = 3, mTableLoaded = 0"
> sipregistrar.log:"2009-04-03T21:23:28.207557Z":325:SIPDB:DEBUG
> :victoria-pingtel-com.us.nortel.com:SipRegistrar:B7556B90:SipR
egistrar:"SubscriptionDB::load loading >
\"/home/dworley/sandbox-147/sipX/../dir-local/var/sipxdata/sip
db/subscription.xml\""
> sipstatus.log:"2009-04-03T21:23:26.764098Z":91:SIPDB:DEBUG:vic
> toria-pingtel-com.us.nortel.com:pid-13797:00000000:SipStatus:"
SubscriptionDB::_ users = 1, mTableLoaded = 0"
> sipstatus.log:"2009-04-03T21:23:26.764177Z":93:SIPDB:DEBUG:vic
> toria-pingtel-com.us.nortel.com:pid-13797:00000000:SipStatus:"
SubscriptionDB::load loading >
\"/home/dworley/sandbox-147/sipX/../dir-local/var/sipxdata/sip
db/subscription.xml\""
> sipxpark.log:"2009-04-03T21:23:26.924860Z":131:SIPDB:DEBUG:vic
> toria-pingtel-com.us.nortel.com:pid-13809:00000000:sipxpark:"S
ubscriptionDB::_ users = 2, mTableLoaded = 0"
> sipxpark.log:"2009-04-03T21:23:26.928617Z":133:SIPDB:DEBUG:vic
> toria-pingtel-com.us.nortel.com:pid-13809:00000000:sipxpark:"S
ubscriptionDB::load loading >
\"/home/dworley/sandbox-147/sipX/../dir-local/var/sipxdata/sip
db/subscription.xml\""
> sipxrls.log:"2009-04-03T21:25:25.699141Z":125:SIPDB:DEBUG:vict
> oria-pingtel-com.us.nortel.com:pid-13305:00000000:sipxrls:"Sub
scriptionDB::_ users = 4, mTableLoaded = 0"
> sipxrls.log:"2009-04-03T21:25:25.701222Z":127:SIPDB:DEBUG:vict
> oria-pingtel-com.us.nortel.com:pid-13305:00000000:sipxrls:"Sub
scriptionDB::load loading >
\"/home/dworley/sandbox-147/sipX/../dir-local/var/sipxdata/sip
db/subscription.xml\""
>
> )
>
> The problem is that once the IMDB table is loaded from the
> XML file, it should not be loaded again -- the XML file is up
> to 20 seconds behind the information in the IMDB table, and
> reloading it risks losing updates. (It's also a waste of effort.)
>
> I suspect the same thing is happening with the other IMDB
> tables, as the code is the same for all of them.
>
> The code in question was added in rev. 14244. The difficulty
> with that code was that mTableLoaded was not initialized upon
> object instantiation, leaving it usually with a non-zero
> value. Unfortunately, the constructor references
> mTableLoaded's value before it is set: "if ( users == 1 || (
> users > 1 && mTableLoaded == false ) )"
>
> I added code to initialize mTableLoaded to false as part of
> revs. 14582, 14584, and 14585. This seemed to be the right
> thing to do, given the name of the member variable, but I now
> believe that it is incorrect.
>
> At this point, I'm unsure how the code is supposed to
> operate. The value 'users' is the number of processes that
> access the table, and so will be 1 if this process is the
> first, and >1 if another process is first. That would
> suggest that the 'if' in question should be 'if (users ==
> 1)". The additional logic seems to be intended to handle the
> case if the first process fails to read the XML file,
> apparently acting during later creations of the object within
> a process. But it is not clear to me how to initialize
> mTableLoaded to obtain this effect.
I think you have made a very clear demonstration that the current
behavior is broken. I'm trying to recall why we introduced that
business of also testing for ( users > 1 && mTableLoaded == false ). If
my memory serves me well, that was done as a (failed) attempt to make
the code more robust and be capable of coping with possible future
changes that would cause sipXsupervisor not to be the first one to get
an instance to a DB. Without the problematic test, only the first
process to get a DB's instance would actually attempt to load it. If
the load() failed, the load would never be re-attempted by subsequent
processes trying to get the DB's instance nor would they be made aware
that the load() failed. The sipXsupervisor is a good example of a
process that needs to know whether or not a load() operation passed or
failed as it uses that information to enforce IMDB dependencies.
Here are two solutions that I an think of:
#1- If we comfortable with postulating that the sipXsupervisor will be
the first process to get any DB's instance forevermore, then the
problematic test can be taken out and do a simple if( user == 1 ) test;
#2- otherwise we need to redesign the logic such that mTableLoaded gets
initialized to true if *any* process has succeeded in loading the DB,
otherwise it is set to false. There are no current means that I'm
aware of that would allow someone to query whether or not *any* process
has succeeded in loading the DB, so this would need to be invented in
order to fix the problem.
Raymond, you did most of the work on that issue, I would be interested
in your views on the subject to make sure we have covered all the bases.
_______________________________________________
sipx-dev mailing list
[email protected]
List Archive: http://list.sipfoundry.org/archive/sipx-dev
Unsubscribe: http://list.sipfoundry.org/mailman/listinfo/sipx-dev
