On 05.08.2008, at 12:57, Vincent CHAVANIS wrote:
Hi andreas,
Andreas Fink a écrit :
On 05.08.2008, at 12:17, Vincent CHAVANIS wrote:
\
- (time(NULL) > (PRIVDATA(conn)->last_activity_time +
PRIVDATA(conn)->keepalive)))
+ (time(NULL) >= (PRIVDATA(conn)->last_activity_time +
PRIVDATA(conn)->keepalive)))
so this is keepalive always being +1... sounds good to me.
So you aggree to that fact that keepalive will be keepalive + 1 ?
yes because of >= versus >.
The impact is that each UCP31 will be done with 1 second inlate.
This is not what users want.
If I specify keepalive each 5 sec, i don't want it each 6 sec.
} else if (emimsg->ot == 31) {
+ PRIVDATA(conn)->last_activity_time = time
(NULL); /* XXX Process error codes here
if (octstr_get_char(emimsg->fields[0], 0) ==
'N') {
long errorcode;
not sure why this is needed. Would sound like it would have never
worked otherwise...
This is absolutly needed!
Take this exemple
00:00:00 you sent a UCP31
00:00:30 you received an ack of your UCP31
(the SMSC is overloaded in this case)
Then if your keepalive is set to 31 you will send just after 1 sec
an other UCP31. And will result to block the sender thread
(can_write = 0)
This should never happen if you have window properly set. And while we
have traffic, last activity is counting up so keepalive will not be
sent out anyway as the activity keeps the link alive. Keepalive is
ONLY sent if there is NO activity. But counting keepalive as activity
itself doesnt sound wrong neither.
@@ -1269,7 +1317,7 @@
*/
static double emi2_get_timeouttime (SMSCConn *conn, Connection
*server)
{
- double ka_timeouttime = PRIVDATA(conn)->keepalive ?
PRIVDATA(conn)->keepalive + 1 : DBL_MAX;
+ double ka_timeouttime = PRIVDATA(conn)->keepalive ?
PRIVDATA(conn)->keepalive : DBL_MAX;
shouldn't timeout be longer than keepalive? could above change not
create a race condition to the fact that the keepalive was "just
about to be sent" but timeout hit first?
double idle_timeouttime = (PRIVDATA(conn)->idle_timeout &&
server) ? PRIVDATA(conn)->idle_timeout : DBL_MAX;
double result = ka_timeouttime < idle_timeouttime ?
ka_timeouttime : idle_timeouttime;
@@ -1626,7 +1674,7 @@
privdata->listening_socket = -1;
privdata->can_write = 1;
privdata->priv_nexttrn = 0;
- privdata->last_activity_time = 0;
+ privdata->last_activity_time = time (NULL); /* to *NOT* force
keepalive after login */
as I said I would leave keepalive at login.
This is not so trivial.
Vincent
