|
On Tue, Dec 06, 2005 at 02:40:37AM +0200, Julian Anastasov wrote:
>
> Hello,
>
> On Mon, 5 Dec 2005, Horms wrote:
>
> > I think that you are confused about what I am trying to do,
> > or perhaps I am confused about what I am trying to do.
> >
> > The idea of my patch is not to change the persistance sementics.
> > It is mearly to fix the case where the template expires and
> > still has controlled connections. The current code will change
> > the persistance timeout if this occurs. And that persistance template,
> > whith the new timeout, can continued to be used for new connections
> > and existing. Even though its timout is not the user-specified value.
> > Which is just plain weird.
>
> Yes, that is why your patch that does not change cp->timeout
> at all is more valid for such example:
>
> i'm browsing https site, reading some docs before deciding to
> fill 3 forms. Persistence is set to 15 mins (5 mins for each form).
> So, the timing is:
>
> +00min: i'm starting to read docs, reading 10mins
> +10min: starting to fill form 1 for 4 mins (i'm fast)
> +14min: posting form 1, starting to fill form 2 for 5 mins (normal)
> +15min: invalidate broke my persistence because 15min persistence
> expired while there are no connections, even if my last posting
> was 1min ago and this conn expired
> +19min: posting form 2, starting to fill form 3. What happens is that
> this web form goes to another real server because first template
> is invalidated or marked as expired
>
> What i want to show you is that the semantic "persistence
> timeout covers period from first connection" is "just plain weird" :)
> What i would want is persistence timeout to be large enough to
> cover the idle periods between connections part from session. In
> this example, i think 15mins is enough for my web site to allow
> users to fill the web forms. The current timeout of 60 seconds
> can be short too, that is why I say your patch that will extend
> persistence with another 15mins is more correct. What is the
> benefit to invalidate the persistence exactly at its defined period?
> It helps the load balancing? Or just saves memory? I would care
> for proper web form posting to single real server. How can i
> select this behavior?
Hi Julian,
Yes, I agree with eveyrhing you say above. However, the
case that we are looking at is that the persistance timeout has
actually expired. But for some reason there are still controlled
connections. I'm happy to just leave the timeout as is, because this
case only really occurs if the persistance timeout is really really
short, shorter than the FIN_WAIT timout.
In any case, I agree with Ratz that FIN_WAIT should probably be
reduced. Which would aleviate the problem.
I believe that the idea of reducing the timeout is primarily
to reduce memory usage. This makes sense for non-templates.
Although 60*HZ is probably a bit high for them.
However, I think that reinstating the timeout, as below, might meet the
goals of reducing memory preasure, by giving expired entries a low
timeout, and honouring the user-defined timeout of templates, even if
they expired between use. I'd like to work with that idea a little more.
> > > 3 seconds is very low value, it can create load.
> >
> > I don't really mind what value it is, perhaps 10 seconds would
> > be more appropriate. The current 60 value seems a little high to me.
> > What do you recommend?
>
> Original user-defined value or user-defined extent
>
> > > Better one to call ip_vs_conn_expire_now for ct
> > > when its n_control reaches 0, eg. in ip_vs_control_del.
> >
> > I don't really think that is desirable at all. Won't that
> > cause the persistance template to expire regardless of if
> > its timeout has expired?
>
> Yes, in this form it is bogus
>
> > You have stated on several occasions that invalidating
> > the hash template using that method will break connections.
> > So surely its a bad idea.
> >
> > The idea of IP_VS_CONN_F_EXPIRED is just to make sure that the
> > template will not be used for any new connections. Existing
>
> That is what i don't want for my example, my session is
> redirected to different server.
Ok, I was just thinking that if the template had timed
out it sholdn't be used. But that will probably cause
other problems like...
>
> > connections will continue on as normal. Although they will
> > almost certainly be in the FIN_WAIT state. Although I guess
> > this semantic is broken if they aren't in that state.
> >
> > Perhaps a better idea, which I was thinking about this morning,
> > is to change the timeout value, to say 60*HZ in ip_vs_conn_expire(),
> > as per the existing code. But always reset it to svc->timeout
> > when it is used to create a new connection, so if it had timeout
> > out, but ends up being used again, its timeout will be the
> > user-defined value when it is re-used.
> >
> > I think this patch has racy access to ct->timeout,
> > but it gives you an idea of what I am thinking about.
>
> This is a good idea, in this case setting 10 seconds in
> ip_vs_conn_expire for templates is not so bad. Also, we should
> take into account that the ip_vs_control_* mechanism is used from
> FTP, may be just to extend the TCP EST period for the control conn
> while there are DATA transfers.
Yes, I was wondering what to do about that. I took a quick look into
that, and it seems to me that, if the control connection needs
to handle a new data connection, then it must be TCP EST,
else it wouldn't recieve the PASV command. And in the case were it
is just handling existing data connections, this is really
the same as a persistance template who has unexpired children.
So I don't think any extra handling of ip_vs_control_* is required.
Am I mistaken?
Also, do you think the access to ct->timeout below is racy?
Yesterdays' patch + changing timeout to 10*HZ instead of 60*HZ,
which I am in favour of.
diff --git a/net/ipv4/ipvs/ip_vs_conn.c b/net/ipv4/ipvs/ip_vs_conn.c
index f828fa2..19bd6a9 100644
--- a/net/ipv4/ipvs/ip_vs_conn.c
+++ b/net/ipv4/ipvs/ip_vs_conn.c
@@ -525,7 +525,7 @@ static void ip_vs_conn_expire(unsigned l
{
struct ip_vs_conn *cp = (struct ip_vs_conn *)data;
- cp->timeout = 60*HZ;
+ cp->timeout = 10*HZ;
/*
* hey, I'm using it
diff --git a/net/ipv4/ipvs/ip_vs_core.c b/net/ipv4/ipvs/ip_vs_core.c
index 1a0843c..7e011e3 100644
--- a/net/ipv4/ipvs/ip_vs_core.c
+++ b/net/ipv4/ipvs/ip_vs_core.c
@@ -283,8 +283,6 @@ ip_vs_sched_persist(struct ip_vs_service
dest);
if (ct == NULL)
return NULL;
-
- ct->timeout = svc->timeout;
} else {
/* set destination with the found template */
dest = ct->dest;
@@ -337,8 +335,6 @@ ip_vs_sched_persist(struct ip_vs_service
dest);
if (ct == NULL)
return NULL;
-
- ct->timeout = svc->timeout;
} else {
/* set destination with the found template */
dest = ct->dest;
@@ -346,6 +342,11 @@ ip_vs_sched_persist(struct ip_vs_service
dport = ports[1];
}
+ /*
+ * Make sure template has user-defined timeout
+ */
+ ct->timeout = svc->timeout;
+
/*
* Create a new connection according to the template
*/
|
