8:11 p.m.
Hello!
Jussi and our nightly testing had issues with not being able to do
anything but slow syncs. I was able to reproduce it for a while:
* The log showed that "Saved Last Remote Server Anchor=''" and
"Last Local Client Anchor=''".
* With both anchors being empty, a slow sync was done.
* According to "SaveAdminData: saving remote anchor =
'1259088941028'", the new anchor should have been saved, but
didn't end up in the binfile in the .synthesis directory.
* => another slow sync the next time
I then switched back to SyncEvolution 0.9.1, which managed to do two-way
syncs with the same config and binfiles. After that, the current
development version also worked again.
I know that this isn't particular helpful to diagnose the root cause of
the issue. I'm just writing to let you know that if you have problems
with a certain configuration, try removing the entire .synthesis
directory in it.
Before you do that, make a copy of it and report back so that we can
have a closer look.
I'll also check my other configs; perhaps one of them still has this
problem.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
8:22 p.m.
On Tue, 2009-11-24 at 20:11 +0100, Patrick Ohly wrote:
I know that this isn't particular helpful to diagnose the root
cause of
the issue. I'm just writing to let you know that if you have problems
with a certain configuration, try removing the entire .synthesis
directory in it.
It's the other way around. *Removing* the binfiles and creating them
from scratch with the current development code leads to the problem.
Writing them *once* with SyncEvolution 0.9.1 *fixes* the problem.
I'm not sure whether I'll have time to look into this some more today.
If I find anything, I'll let you know.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
10:52 p.m.
On Tue, 2009-11-24 at 20:22 +0100, Patrick Ohly wrote:
On Tue, 2009-11-24 at 20:11 +0100, Patrick Ohly wrote:
> I know that this isn't particular helpful to diagnose the root cause of
> the issue. I'm just writing to let you know that if you have problems
> with a certain configuration, try removing the entire .synthesis
> directory in it.
It's the other way around. *Removing* the binfiles and creating them
from scratch with the current development code leads to the problem.
Writing them *once* with SyncEvolution 0.9.1 *fixes* the problem.
I don't quite understand why running SyncEvolution 0.9.1 had an effect.
Anyway, the root cause of the problem was a recent commit in
SyncEvolution. Fix is in "master":
commit a0a7a659c606eb1c77062a0881c617ff8bcf9ac9
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Tue Nov 24 22:01:23 2009 +0100
SyncContext: only use one engine instance, fixes slow sync issue
This removes code which was added as part of the some of the SAN
handling patches. The code added a second Synthesis engine while the
first one was still running, using this statement:
// reinitializes the engine, only at this time can we decide whether
// this is a server session or client session.
SwapEngine swapengine(*this);
I recognize the comment. It must have been copied from an older
SyncEvolution revision. However, SyncEvolution no longer uses two
instances of the engine, so this code can and has to be removed again.
Instead the first instance is reinitialized once the full config is
known.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
3:34 a.m.
commit a0a7a659c606eb1c77062a0881c617ff8bcf9ac9
Author: Patrick Ohly <patrick.ohly(a)intel.com>
Date: Tue Nov 24 22:01:23 2009 +0100
SyncContext: only use one engine instance, fixes slow sync issue
This removes code which was added as part of the some of the SAN
handling patches. The code added a second Synthesis engine while the
first one was still running, using this statement:
// reinitializes the engine, only at this time can we decide whether
// this is a server session or client session.
SwapEngine swapengine(*this);
I recognize the comment. It must have been copied from an older
SyncEvolution revision. However, SyncEvolution no longer uses two
instances of the engine, so this code can and has to be removed again.
Instead the first instance is reinitialized once the full config is
known.
Well, this will break server alerted sync case. The problem is:
1) we init the engine (for logging purpose, with the assumption that this is a client
session)
2) after we found this is a server alerted sync session and the underlying SynthesisEngine
must be reinitialized to a server session.
The solution I think of is:
1) Don't init the engine until we reliably know this is a server session or client
session (lost some logging capabilities)
Or
2) Move these routines (those need to be done before we finally detect this is a server or
client session) to another function, so that leaving this function will automatically
destruct the underlying SharedEngine and reinitialize the real engine afterwards.
--
Regards, Congwu
4:21 a.m.
2) Move these routines (those need to be done before we finally detect
this is
a server or client session) to another function, so that leaving this function
will automatically destruct the underlying SharedEngine and reinitialize the
real engine afterwards.
See commit b4dc47edd1cd9b413bc3c7953d57e3a402d1ca42 at phone branch.
--
Regards, Congwu
8:20 a.m.
On Wed, 2009-11-25 at 02:34 +0000, Chen, Congwu wrote:
>commit a0a7a659c606eb1c77062a0881c617ff8bcf9ac9
>Author: Patrick Ohly <patrick.ohly(a)intel.com>
>Date: Tue Nov 24 22:01:23 2009 +0100
>
> SyncContext: only use one engine instance, fixes slow sync issue
>
> This removes code which was added as part of the some of the SAN
> handling patches. The code added a second Synthesis engine while the
> first one was still running, using this statement:
>
> // reinitializes the engine, only at this time can we decide whether
> // this is a server session or client session.
> SwapEngine swapengine(*this);
>
> I recognize the comment. It must have been copied from an older
> SyncEvolution revision. However, SyncEvolution no longer uses two
> instances of the engine, so this code can and has to be removed again.
> Instead the first instance is reinitialized once the full config is
> known.
>
Well, this will break server alerted sync case. The problem is:
1) we init the engine (for logging purpose, with the assumption that
this is a client session)
2) after we found this is a server alerted sync session and the
underlying SynthesisEngine must be reinitialized to a server session.
Okay, I see.
The solution I think of is:
1) Don't init the engine until we reliably know this is a server
session or client session (lost some logging capabilities)
Or
2) Move these routines (those need to be done before we finally detect
this is a server or client session) to another function, so that
leaving this function will automatically destruct the underlying
SharedEngine and reinitialize the real engine afterwards.
I agree with solution 2 and merged this patch into master.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
10:34 a.m.
On Wed, 2009-11-25 at 02:34 +0000, Chen, Congwu wrote:
The solution I think of is:
1) Don't init the engine until we reliably know this is a server session or client
session (lost some logging capabilities)
Or
2) Move these routines (those need to be done before we finally detect this is a server
or client session) to another function, so that leaving this function will automatically
destruct the underlying SharedEngine and reinitialize the real engine afterwards.
I've had another look at this, because instantiating the engine twice
still has drawbacks (not so nice HTML log, overhead).
Wouldn't it work to simply move this part before the first instantiation
of the engine:
if ( getPeerIsClient()) {
m_serverMode = true;
}
Then we can revert the "presync" patch and avoid instantiating the
engine anew, because it will be already the right one.
There's also a related TODO remark in DBusServer::startSession():
// TODO: how do we decide whether this is a client or server session?
I think this has become obsolete, because the Session instance supports
both the same way and the SyncContext can decide for itself what it has
to do when asked to sync.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
10:54 a.m.
Patrick wrote:
I've had another look at this, because instantiating the engine
twice
still has drawbacks (not so nice HTML log, overhead).
Wouldn't it work to simply move this part before the first instantiation
of the engine:
if ( getPeerIsClient()) {
m_serverMode = true;
}
Then we can revert the "presync" patch and avoid instantiating the
engine anew, because it will be already the right one.
That's really nice. We only need to check serverMode again to decide whether it
needs
to call initSAN.
Totally agree with you.
There's also a related TODO remark in
DBusServer::startSession():
// TODO: how do we decide whether this is a client or server session?
I think this has become obsolete, because the Session instance supports
both the same way and the SyncContext can decide for itself what it has
to do when asked to sync.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
--
Regards,
Congwu
3:49 a.m.
Chen, Congwu wrote:
Patrick wrote:
>Then we can revert the "presync" patch and avoid instantiating the
>engine anew, because it will be already the right one.
That's really nice. We only need to check serverMode again to decide whether
it needs
to call initSAN.
Totally agree with you.
I have just push the commit to phone branch.
I think we still need to think about this sometime later. It should be reasonable to
carry out the SAN process out of the real sync session (Especially for automatic sync),
while we should still need the logging facility.
-
Regards,
Congwu
8:40 a.m.
On Fri, 2009-11-27 at 02:49 +0000, Chen, Congwu wrote:
I think we still need to think about this sometime later. It should
be reasonable to
carry out the SAN process out of the real sync session (Especially for automatic sync),
while we should still need the logging facility.
Correct. We need some kind of "global log" for things like failed
connection requests etc. Currently this all goes to stdout of
syncevo-dbus-server.
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
10:05 p.m.
Patrick Ohly wrote:
Hello!
Jussi and our nightly testing had issues with not being able to do
anything but slow syncs.
While trying to debug this (duplicating my addressbook impressively), I
also noticed perl processing taking a lot of time even when using the
DBus api. Wasn't this tweaked at some point so the diffs weren't being
generated in syncevo-dbus-server?
I didn't do any real bisecting but looking at my branches, I believe
this is a point where things still worked:
Nov 11 10:55:15 fee9c3917b81859a23a37886e39098517695023e
Jussi
8:10 a.m.
New subject: [SyncEvolution] D-Bus + printChanges (was: Re: binfile corruption -> slow sync?)
On Tue, 2009-11-24 at 21:05 +0000, Jussi Kukkonen wrote:
While trying to debug this (duplicating my addressbook impressively),
I
also noticed perl processing taking a lot of time even when using the
DBus api. Wasn't this tweaked at some point so the diffs weren't being
generated in syncevo-dbus-server?
Yes, but never in the new implementation - at least as far as I
remember.
The synccompare invocation can be controlled via the "printChanges" sync
property. If a D-Bus client doesn't want this, it can temporarily set
the property to 0.
Should we change the semantic of the D-Bus API so that this is done
automatically? It would force users of the API to set it explicitly to 1
when wanted (command line?) and disable the user's configuration option
(unless the D-Bus client checks the config first itself).
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
9:33 a.m.
New subject: [SyncEvolution] D-Bus + printChanges
Patrick Ohly wrote:
On Tue, 2009-11-24 at 21:05 +0000, Jussi Kukkonen wrote:
> While trying to debug this (duplicating my addressbook impressively), I
> also noticed perl processing taking a lot of time even when using the
> DBus api. Wasn't this tweaked at some point so the diffs weren't being
> generated in syncevo-dbus-server?
Yes, but never in the new implementation - at least as far as I
remember.
The synccompare invocation can be controlled via the "printChanges" sync
property. If a D-Bus client doesn't want this, it can temporarily set
the property to 0.
Should we change the semantic of the D-Bus API so that this is done
automatically? It would force users of the API to set it explicitly to 1
when wanted (command line?) and disable the user's configuration option
(unless the D-Bus client checks the config first itself).
Setting printChanges=0 in the client is not a big deal, but defaulting
to 0 (however that technically happens) in the D-Bus case makes sense to
me... I wouldn't expect a service to spend a lot of time outputting to
stdout.
- Jussi
9:48 a.m.
New subject: [SyncEvolution] D-Bus + printChanges
On Wed, 2009-11-25 at 08:33 +0000, Jussi Kukkonen wrote:
Patrick Ohly wrote:
> Should we change the semantic of the D-Bus API so that this is done
> automatically? It would force users of the API to set it explicitly to 1
> when wanted (command line?) and disable the user's configuration option
> (unless the D-Bus client checks the config first itself).
Setting printChanges=0 in the client is not a big deal, but defaulting
to 0 (however that technically happens) in the D-Bus case makes sense to
me... I wouldn't expect a service to spend a lot of time outputting to
stdout.
Except when the client is the command line and we implement #5042: D-Bus
API: send log output to clients... That would include the output of
synccompare.
For the sake of a clean design, would you mind setting printChanges=0
yourself?
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
9:58 a.m.
New subject: [SyncEvolution] D-Bus + printChanges
Patrick Ohly wrote:
On Wed, 2009-11-25 at 08:33 +0000, Jussi Kukkonen wrote:
> Patrick Ohly wrote:
>> Should we change the semantic of the D-Bus API so that this is done
>> automatically? It would force users of the API to set it explicitly to 1
>> when wanted (command line?) and disable the user's configuration option
>> (unless the D-Bus client checks the config first itself).
> Setting printChanges=0 in the client is not a big deal, but defaulting
> to 0 (however that technically happens) in the D-Bus case makes sense to
> me... I wouldn't expect a service to spend a lot of time outputting to
> stdout.
Except when the client is the command line and we implement #5042: D-Bus
API: send log output to clients... That would include the output of
synccompare.
Yes, the point was a generic one: I didn't expect that to happen by
default.
For the sake of a clean design, would you mind setting
printChanges=0
yourself?
Sure. I'll add a note to Session.Sync() docs, I'm pretty sure other
clients (if any) will want to do the same thing.
- Jussi
4068
days inactive
4071
days old
syncevolution@synevolution.org
14 comments
3 participants
participants (3)
-
Chen, Congwu -
Jussi Kukkonen -
Patrick Ohly