On Do, 2010-02-11 at 12:44 +0000, Patrick Ohly wrote:
> Hello!
> 
> Thanks for the explanation. I'm working on the client side support.
> 
> On Do, 2010-02-11 at 11:19 +0000, Lukas Zeller wrote:
> > What would be needed is
> > 
> > a) a user settings flag that indicates for some datastores that these
> > must NOT be run on their own, but within the superdatastore.
> 
> How about setting a special URI, like <none>? Because of the angle
> brackets it cannot conflict with valid URIs.
> 
> > b) An enhanced SelectProfile() in binfileimplclient.cpp, which would
> > check that flag and figure out which superdatastore to instantiate in
> > addition to the normal datastores.
> 
> Is that really necessary? If we require that all relevant targets are
> enabled, both for the <superdatastore> and its underlying <datastore>s,
> then SelectProfile() can simply instantiate them all.
> 
> For that to work, TSuperDataStore must be derived from TBinfileImplDS to
> avoid segfaults similar to the one seen earlier.

That's going down the wrong way. Changing the base class changes too
much of TSuperDataStore's implementation because of all the virtual
methods that get inherited from TBinfileImplDS. What I did instead was
to let binfileimplclient.cpp work with both TBinfileImplDS and
TSuperDataStore instances, as quoted below.

Now I have another question: where does the binfile client decide about
slow and first time syncs? Does it perhaps depend on TSuperDataStore
being a TBinfileImplDS after all, for example to save anchors?

In other words, where should anchors be saved in this case?

What happens right now on the client side is that I see 'MakeAdminReady'
for the first sub-datastore, but not for the second or the
superdatastore. Then it decides that it must do a "slow first time Sync"
for the superdatastore, even though this should be a normal sync.

> Do you agree with
> Congwu's approach of changing the base class?

For the config this might be okay, but I am not sure.

> > c) Some tweaking of the for-each-datastore-do-something loops that
> > control the flow of a client session (especially syncagent.cpp around
> > 1090, where engPrepareClientSyncAlert() is called)
> 
> I see that
> engPrepareClientSyncAlert(TSuperDataStore *aAsSubDatastoreOf)
> is already prepared to take a TSuperDataStore pointer. But that pointer
> is never set anywhere, is it? I only find one call which passes NULL.
> 
> After changing engPrepareClientSyncAlert() to check for a <none> URI
> instead of the always-NULL *aAsSubDatastoreOf, I get the first message
> out to the server as expected.
> 
> After processing the reply, the client becomes unhappy because it cannot
> identify the matching DevInfo entry:
>         Warning: Remote datastore name '<none>' not found in received DevInf.
>         Warning: No DevInf for remote datastore, running blind sync attempt
> 
> This is in TLocalEngineDS::engInitForSyncOps(). How should that be
> handled? The code has some comments about possibly being called earlier.
> Would that be the case if the superdatastore had been checked first?

My approach right now is to skip some of these operations when uri ==
"<none>" marks the datastore as special.



diff --git a/src/sysync/binfileimplclient.cpp b/src/sysync/binfileimplclient.cpp
index 457b105..cdb7d0e 100755
--- a/src/sysync/binfileimplclient.cpp
+++ b/src/sysync/binfileimplclient.cpp
@@ -2777,15 +2777,19 @@ localstatus TBinfileImplClient::SelectProfile(uInt32 
aProfileSelector, bool aAut
             #endif
             if (syncit)
             {
-              TBinfileImplDS *binfiledsP=NULL;
+              TLocalEngineDS *localenginedsP=NULL;
               if (binfiledscfgP) {
-                // create datastore
-                binfiledsP = static_cast<TBinfileImplDS 
*>(binfiledscfgP->newLocalDataStore(this));
+                localenginedsP=binfiledscfgP->newLocalDataStore(this);
               }
-              if (binfiledsP) {
+              if (localenginedsP) {
+                // superdatastores are not derived from TBinfileImplDS
+                TBinfileImplDS 
*binfiledsP=localenginedsP->castTBinfileImplDS();
+
                 // copy target info to datastore for later access during sync
-                binfiledsP->fTargetIndex=recidx;
-                binfiledsP->fTarget=target;
+                if (binfiledsP) {
+                  binfiledsP->fTargetIndex=recidx;
+                  binfiledsP->fTarget=target;
+                }
                 // determine sync mode / flags to use
                 TSyncModes myMode;
                 bool mySlow;
@@ -2816,7 +2820,7 @@ localstatus TBinfileImplClient::SelectProfile(uInt32 
aProfileSelector, bool aAut
                 }
                 // set non-BinFile specific parameters (note that this call 
might
                 // be to a derivate which uses additional info from fTarget to 
set sync params)
-                binfiledsP->dsSetClientSyncParams(
+                localenginedsP->dsSetClientSyncParams(
                   myMode,
                   mySlow,
                   target.remoteDBpath,
@@ -2828,13 +2832,13 @@ localstatus TBinfileImplClient::SelectProfile(uInt32 
aProfileSelector, bool aAut
                   false // filter inclusive
                 );
                 // prepare local datastore (basic init can be done here) and 
check availability
-                if (binfiledsP->localDatastorePrep()) {
+                if (!binfiledsP || binfiledsP->localDatastorePrep()) {
                   // add to datastores for this sync
-                  fLocalDataStores.push_back(binfiledsP);
+                  fLocalDataStores.push_back(localenginedsP);
                 }
                 else {
                   // silently discard (do not sync it)
-                  PDEBUGPRINTFX(DBG_ERROR,("Local Database for datastore '%s' 
prepares not ok -> not synced",binfiledsP->getName()));
+                  PDEBUGPRINTFX(DBG_ERROR,("Local Database for datastore '%s' 
prepares not ok -> not synced",localenginedsP->getName()));
                   // show event (alerted for no database)
                   OBJ_PROGRESS_EVENT(
                     getSyncAppBase(),
@@ -2842,7 +2846,7 @@ localstatus TBinfileImplClient::SelectProfile(uInt32 
aProfileSelector, bool aAut
                     binfiledscfgP,
                     LOCERR_LOCDBNOTRDY,0,0
                   );
-                  delete binfiledsP;
+                  delete localenginedsP;
                 }
               }
             } // if target DB enabled for sync
diff --git a/src/sysync/localengineds.h b/src/sysync/localengineds.h
index 799b2cb..be149e1 100755
--- a/src/sysync/localengineds.h
+++ b/src/sysync/localengineds.h
@@ -285,6 +285,8 @@ class TLDSfuncs;
   #define TSuperDataStore TLocalEngineDS // to allow parameter for 
engProcessSyncAlert()
 #endif
 
+class TBinfileImplDS;
+
 /// @brief Local datastore engine and abstraction of actual implementation
 /// - session and commands only call non-virtual engXXXX members of this class 
to
 ///   perform any operations on the datastore
@@ -529,6 +531,9 @@ public:
   /// destructor
   virtual ~TLocalEngineDS();
 
+  /// dynamic cast, returns NULL if not of that type
+  virtual TBinfileImplDS *castTBinfileImplDS() { return NULL; }
+
   /// @name dsProperty property and state querying methods
   /// @{
   #ifndef MINIMAL_CODE



_______________________________________________
os-libsynthesis mailing list
os-libsynthesis@synthesis.ch
http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis

Reply via email to