|
On Mon, Nov 07, 2005 at 09:24:07AM +0200, Julian Anastasov wrote:
>
> Hello,
>
> On Mon, 7 Nov 2005, Horms wrote:
>
> > I'm a bit worried about that too. Although the template has to have
> > actually timed out for this to occur. In any case, perhaps a patch that
> > doesn't change the timeout would be better. Something like the
>
> The problem is that we invalidate templates breaking sessions,
> in such case we don't care much when template expires.
>
> > following, though I'm not sure why cp->timeout is set at the top
> > of ip_vs_conn_expire(), it would seem cleaner if it was set below
> > expire_later. Actually, I'm not really sure why the timeout is altered
> > at all.
>
> Agreed, ip_vs_conn_expire_now() already does not set cp->timeout
> to 0, so there is no good reason to modify it in ip_vs_conn_expire.
> May be the code should look in this way:
>
> if (cp->flags & IP_VS_CONN_F_TEMPLATE)
> cp->timeout = 10*HZ;
>
> Then the timeout for normal connections will not be modified
> while templates will change from 360 to 10 if waiting all its normal
> connections to expire.
That seems like a good work around to me, though perhaps HZ would
be better than 10*HZ.
Here is a patch to formalise it slightly. This is for 2.6, i'll send a
version for 2.5 shortly.
diff --git a/net/ipv4/ipvs/ip_vs_conn.c b/net/ipv4/ipvs/ip_vs_conn.c
index f828fa2..f531d59 100644
--- a/net/ipv4/ipvs/ip_vs_conn.c
+++ b/net/ipv4/ipvs/ip_vs_conn.c
@@ -525,7 +525,8 @@ static void ip_vs_conn_expire(unsigned l
{
struct ip_vs_conn *cp = (struct ip_vs_conn *)data;
- cp->timeout = 60*HZ;
+ if (cp->flags & IP_VS_CONN_F_TEMPLATE)
+ cp->timeout = 10*HZ;
/*
* hey, I'm using it
> > > So, there can be a sysctl var "extra_persistence",
> > > "persistence_extent" or another more suitable name. Its default
> > > value can be "60" seconds, as now. The dangerous value of 0 will
> > > invalidate the template as you propose (but still use some timeout
> > > to check for removal). Any other value will be used as before in
> > > ip_vs_conn_expire, just for templates.
> >
> > I'm pretty reluctant to add yet another sysctl value, people
> > are already very confused about how persistance works.
>
> It is true that such global value can not be applied to
> all persistent services in same box, if it is implemented it
> should be per-service. But your patch is too dramatic for web
> sessions without such information. May be changing timeout from 60
> to 10 is a compromise for templates?
I didn't realise my patch would break existing sessions, obviously
that is bad. I think your approach is a good one.
--
Horms
|
