2:56 a.m.
http://bugzilla.moblin.org/show_bug.cgi?id=7710
Summary: server session handling
Classification: Moblin Projects
Product: SyncEvolution
Version: upstream
Platform: Netbook
OS/Version: Moblin Linux
Status: ASSIGNED
Severity: normal
Priority: P1
Component: *Feature Request
AssignedTo: patrick.ohly(a)intel.com
ReportedBy: patrick.ohly(a)intel.com
CC: syncevolution(a)lists.intel.com
Depends on: 7707
In server mode, we have the following problems:
- sessions that the client no longer replies to don't time out,
blocking new sync sessions
- a client aborting one session and starting a new one is not
able to do so until the previous session has timed out; it
would be nicer if the server would detect this and close the
older session
- we should delay picking a peer configuration until we really
know the peer's device ID; currently we only check that the
pre-chosen peer configuration matches
- database dumping should be delayed until we know a) that the
client is authorized and b) which sources are really active
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
7:58 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
pohly <patrick.ohly(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Blocks| |7214
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
10:08 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #1 from pohly <patrick.ohly(a)intel.com> 2009-11-06 03:08:14 ---
(In reply to comment #0)
- we should delay picking a peer configuration until we really
know the peer's device ID; currently we only check that the
pre-chosen peer configuration matches
Implemented.
Further changes are needed (tenative, see mailing list discussion):
- deviceID becomes a "per source set" configuration. It always stands
for the ID used by the local side, regardless whether it is acting
as server or client. As server the ID doesn't have much use, though,
except in special SAN messages to SyncEvolution (not implemented yet).
- remoteDeviceID must become a public property, because users and/or
frontends have to set it. Currently it is set automatically upon
first connect for the pre-chosen configuration. We want to get rid of
the need to choose the configuration in advance and replace it
with matching against existing remoteDeviceID properties, so these
values must get into the configurations somehow.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
7:55 a.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #2 from pohly <patrick.ohly(a)intel.com> 2009-11-09 12:55:43 PST ---
(In reply to comment #0)
- a client aborting one session and starting a new one is not
able to do so until the previous session has timed out; it
would be nicer if the server would detect this and close the
older session
Implemented. Congwu, please have a look at the "server-sessions" branch and
review the following commit:
syncevo-dbus-server: kill old session(s) when the same client connects
again (MB#7710)
The server now checks whether it has other sessions active or in the
queue with the same device ID and kills the older sessions when adding
a new one. Because it does this on each Process(), it is unlikely
(impossible?!) to to have more than one older session, but because the
session might be running or still inactive in the queue, both are
checked.
As part of testing with test-dbus.py it was observed that a connection
that was to be aborted was also closed normally first. That was because
the sync engine did not set an error status in this case. Better check
for abort requests explicitly before calling agent->shutdown(). However,
this should be fixed as part of the server progress handling (MB#7709).
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
8:48 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
Chen Congwu <congwu.chen(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |congwu.chen(a)intel.com
--- Comment #3 from Chen Congwu <congwu.chen(a)intel.com> 2009-11-10 01:48:02 PST
---
(In reply to comment #2)
(In reply to comment #0)
> - a client aborting one session and starting a new one is not
> able to do so until the previous session has timed out; it
> would be nicer if the server would detect this and close the
> older session
Implemented. Congwu, please have a look at the "server-sessions" branch and
review the following commit:
Looks good.
Why do you use double check here:
it->lock(), it->lock()
I thinks we are in only one thread, so this is not necessary.
+ while (it != m_workQueue.end()) {
+ boost::shared_ptr<Session> session = it->lock();
+ if (session && session->getPeerDeviceID() == peerDeviceID) {
+ SE_LOG_DEBUG(NULL, NULL, "removing pending session %s because it matches
deviceID %s",
+ session->getSessionID().c_str(),
+ peerDeviceID.c_str());
+ // remove session and its corresponding connection
+ boost::shared_ptr<Session> s = it->lock();
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
9:58 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #4 from pohly <patrick.ohly(a)intel.com> 2009-11-10 02:58:49 PST ---
(In reply to comment #3)
Why do you use double check here:
it->lock(), it->lock()
I thinks we are in only one thread, so this is not necessary.
+ while (it != m_workQueue.end()) {
+ boost::shared_ptr<Session> session = it->lock();
+ if (session && session->getPeerDeviceID() == peerDeviceID) {
+ SE_LOG_DEBUG(NULL, NULL, "removing pending session %s because it matches
deviceID %s",
+ session->getSessionID().c_str(),
+ peerDeviceID.c_str());
+ // remove session and its corresponding connection
+ boost::shared_ptr<Session> s = it->lock();
Well spotted. The second lock is redundant. I'll remove it and merge the
result.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
9:22 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #5 from pohly <patrick.ohly(a)intel.com> 2009-11-11 02:22:05 PST ---
(In reply to comment #0)
In server mode, we have the following problems:
- sessions that the client no longer replies to don't time out,
blocking new sync sessions
This is what Lukas wrote about this:
| I just have added two new read-only values in the session key:
| "lastused" and "timedout".
|
| "timedout" will return true when the session has timed out (was
| inactive during the configured timeout period) and should be
| terminated (either by just closing it, which will end it with
| 408=timeout status, or by writing "abortstatus" with a different
| status code first if 408 is not appropriate).
|
| "lastused" returns the time when the session thinks it was last used
| (i.e. when it has last encountered a SyncML message).
To use these values we:
- need a new configuration option for this timeout value
- set it in the XML config
- implement regular callbacks in the DBusTransportAgent
(there's a TODO in the code)
- check this session timeout when running as server, instead of the
resend interval in client mode
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
3:33 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #6 from Chen Congwu <congwu.chen(a)intel.com> 2009-11-16 20:33:50 PST
---
(In reply to comment #1)
There is minor problem:
Peeking into the first message to get the deviceID to pick the right peer
configuration, at this time peer configuration is not available. You are using
an empty string to identify this.
My test shows this maybe not worked as you expected, a hidden file
~/.config/syncevolution/.internal.ini is created for this purpose and this
cause checkDevice to report an error each time the remote device is changed.
In this case, the error may not be catched.
+ SyncContext sync("", false);
(In reply to comment #0)
> - we should delay picking a peer configuration until we really
> know the peer's device ID; currently we only check that the
> pre-chosen peer configuration matches
Implemented.
Further changes are needed (tenative, see mailing list discussion):
- deviceID becomes a "per source set" configuration. It always stands
for the ID used by the local side, regardless whether it is acting
as server or client. As server the ID doesn't have much use, though,
except in special SAN messages to SyncEvolution (not implemented yet).
- remoteDeviceID must become a public property, because users and/or
frontends have to set it. Currently it is set automatically upon
first connect for the pre-chosen configuration. We want to get rid of
the need to choose the configuration in advance and replace it
with matching against existing remoteDeviceID properties, so these
values must get into the configurations somehow.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
5:28 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #7 from pohly <patrick.ohly(a)intel.com> 2009-11-16 22:28:24 PST ---
(In reply to comment #6)
(In reply to comment #1)
There is minor problem:
Peeking into the first message to get the deviceID to pick the right peer
configuration, at this time peer configuration is not available. You are using
an empty string to identify this.
My test shows this maybe not worked as you expected, a hidden file
~/.config/syncevolution/.internal.ini is created
I have already changed this as part of the "server-config" branch: the
SyncContext is now instantiated with a VolatileConfigTree as storage, which
puts all properties into memory without any chance (or risk, in this case) of
flushing them to disk.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
6:22 a.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #8 from pohly <patrick.ohly(a)intel.com> 2009-11-27 11:22:18 PST ---
(In reply to comment #1)
- remoteDeviceID must become a public property, because users
and/or
frontends have to set it.
Done.
On this occasion, I also checked all other sync properties to verify whether
they make sense for a SyncML server.
Congu, can you review this? I committed directly into "master". Any typos and
errors will be recorded for eternity...
(In reply to comment #0)
In server mode, we have the following problems:
- sessions that the client no longer replies to don't time out,
blocking new sync sessions
In the documentation update I ear-marked RetryDuration as the configuration
option which could be used to specify the timeout duration. Not implemented
yet, though.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
11:54 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
pohly <patrick.ohly(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED
--- Comment #9 from pohly <patrick.ohly(a)intel.com> 2010-01-14 04:54:19 PST ---
(In reply to comment #8)
(In reply to comment #1)
> - remoteDeviceID must become a public property, because users and/or
> frontends have to set it.
Done.
On this occasion, I also checked all other sync properties to verify whether
they make sense for a SyncML server.
Congu, can you review this? I committed directly into "master". Any typos and
errors will be recorded for eternity...
I don't remember - did you look at those commits?
(In reply to comment #0)
> In server mode, we have the following problems:
> - sessions that the client no longer replies to don't time out,
> blocking new sync sessions
In the documentation update I ear-marked RetryDuration as the configuration
option which could be used to specify the timeout duration. Not implemented
yet, though.
Implemented, see mb7110 branch. Congwu, I changed some of your code in SAN
sending and SoupTransportAgent event handling. Can you review that branch?
Did I fix everything mentioned in this issue so that we can consider it
"resolved" now?
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
1:39 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #10 from Chen Congwu <congwu.chen(a)intel.com> 2010-01-14 18:39:17 PST
---
(In reply to comment #9)
(In reply to comment #8)
> (In reply to comment #1)
I don't remember - did you look at those commits?
Yes, nice for me.
> (In reply to comment #0)
> > In server mode, we have the following problems:
> > - sessions that the client no longer replies to don't time out,
> > blocking new sync sessions
>
> In the documentation update I ear-marked RetryDuration as the configuration
> option which could be used to specify the timeout duration. Not implemented
> yet, though.
Implemented, see mb7110 branch. Congwu, I changed some of your code in SAN
sending and SoupTransportAgent event handling. Can you review that branch?
My OK.
+ T release() { T res = m_pointer; m_pointer = NULL; return res; }
I think this is a typo, m_pointer is not a pointer now, should not assign NULL
to it.
Did I fix everything mentioned in this issue so that we can consider
it
"resolved" now?
Did you forget this as you mentioned in MB7838
"
Implementation will be a bit tricky. We have to start a Synthesis session with
the "server" config in read-only mode, then at the point when the client is
authenticated, create a new config and switch to that while proceeding with the
same SyncML session. This depends on deferring some of the operations currently
done at the start of a sync until that point when the client is authorized (bug
#7710). I can have a look, but might not get around to it this week.
"
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
3:13 a.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
pohly <patrick.ohly(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|FIXED |
--- Comment #11 from pohly <patrick.ohly(a)intel.com> 2010-01-15 08:13:45 PST ---
(In reply to comment #10)
+ T release() { T res = m_pointer; m_pointer = NULL; return res; }
I think this is a typo, m_pointer is not a pointer now, should not assign NULL
to it.
Correct. In C++ NULL = 0, so this only a naming problem. Will change it.
> Did I fix everything mentioned in this issue so that we can
consider it
> "resolved" now?
Did you forget this as you mentioned in MB7838
Tip: if you use "bug #7838", Bugzilla will create a link to it.
"
Implementation will be a bit tricky. We have to start a Synthesis session with
the "server" config in read-only mode, then at the point when the client is
authenticated, create a new config and switch to that while proceeding with the
same SyncML session. This depends on deferring some of the operations currently
done at the start of a sync until that point when the client is authorized (bug
#7710). I can have a look, but might not get around to it this week.
"
Right. So the remaining open item is:
- database dumping should be delayed until we know a) that the
client is authorized and b) which sources are really active
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
6:58 p.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
--- Comment #12 from pohly <patrick.ohly(a)intel.com> 2010-02-16 23:58:10 PST ---
(In reply to comment #11)
Right. So the remaining open item is:
- database dumping should be delayed until we know a) that the
client is authorized and b) which sources are really active
Implemented in "pohly" for both server and client. It makes sense also for
clients because it avoids database dumping in cases where the server cannot be
reached.
Still needs to be reviewed and merged.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
1:55 a.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
pohly <patrick.ohly(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution| |FIXED
--- Comment #13 from pohly <patrick.ohly(a)intel.com> 2010-02-19 06:55:00 PST ---
(In reply to comment #12)
Still needs to be reviewed and merged.
Reviewed by Jussi, merged into master.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
2:25 a.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
Bug 7710 depends on bug 7707, which changed state.
Bug 7707 Summary: revised configuration layout: multiple peers per config
http://bugzilla.moblin.org/show_bug.cgi?id=7707
What |Old Value |New Value
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution| |FIXED
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
3:25 a.m.
New subject: [Bug 7710] server session handling
http://bugzilla.moblin.org/show_bug.cgi?id=7710
pohly <patrick.ohly(a)intel.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Resolution|FIXED |INTEGRATED
--- Comment #14 from pohly <patrick.ohly(a)intel.com> 2010-02-24 08:25:19 PST ---
Integrated in 1.0 beta 2 = 0.99.11.
--
Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching someone on the CC list of the bug.
4589
days inactive
4703
days old
syncevolution-issues@syncevolution.org
16 comments
1 participants
participants (1)
-
bugzilla＠moblin.org