Re: [PATCH] : Prevent redispatcher from selecting the same server

From: Willy Tarreau <w#1wt.eu>
Date: Mon, 31 Dec 2007 11:08:27 +0100


Hi Krzysztof,

first, thank you for taking the time to discuss such fondamental issues, it really helps finding the most evolutive design.

On Mon, Dec 31, 2007 at 07:13:51AM +0100, Krzysztof Oledzki wrote:
> This example with IP address only shows how I would like to solve
> it. Indeed, using IP as a key really sucks, especially when you need to
> change it - be there, done that. ;) However, server name is not something
> you need to change quite often.

True, but in a configuration written from scratch, it's pretty annoying to repeat the same info on every line. That said, I think I begin to understand the problem. On a Cisco, nobody writes a conf from scratch in a config file, everything is done at the CLI. There, it's very convenient to press the UP arrow, suppress the last word with Ctrl-W then press "?" to get a list of useful keywords, press a letter then <Tab> to see it completed.

But right now we have no CLI, so no completion, no reminder of the keywords or the syntax, and no help to repeat lines. I think this is what makes a difference.

Having used Fortinet's CLI which uses a lot of "config edit"/"end" blocks, I must say that it's not very convenient in CLI mode because in order to enable or disable a single option, you have to enter a level. But it's easy to write plain configurations on the other hand.

That said, I think that we still need the less verbose mode (ie: sublevel) for plain-text configurations, but we will soon want the more verbose one for the CLI. BTW, to make the parallel with the cisco config, we should consider that being in a backend section is equivalent to being in "conf t" as there is basically nothing outside a backend section. And in "conf t", you do not repeat "interface g0/0" or "router bgp 65000" on every line, you find it convenient to enter these contexts. But sometimes you'd like to be able to do :
> conf t

config> no interface g0/0 encaps dot1q

So I think that we can do both. In fact, I would like to consider the following :

For instance :

   server www2

      address 192.168.1.1:80
      inter 1000
   server www3
      backup
      no check

or :

   server www2 address 192.168.1.1:80 inter 1000    server www3 backup no check

or :

   server www2 address 192.168.1.1:80
   server www2 inter 1000
   server www3 backup
   no server www3 check

That would cover all uses, both plain configurations and CLI.

> Also, from programmers' POV, prefixing each sentence means that adding
> support for above syntax is extremely easy. There is no need to keep a
> current context (server) and pointer to a server structure and parse
> everything differently if we are in a different mode.

In fact, I don't really mind about the added complexity for the programmer, because once we find how to do this an we have coded it, everyone benefits from it every day.

> We simply need to
> extend the current parser to allow a server statements without
> ip:port-less *if* this server was defined previously and simply modify a
> current structure instead of creating a new one.
>
> With blocks, we have to rework the parser, so each newline after a
> "server" is treated in a different way. It probably requires to move
> server's code to a different function to prevent code duplication.

I'm not saying that this is a simple small change. Even if we don't have it before 2 or 3 versions, it's not dramatic. But I would like this solution to cover other uses such as the ACLs. Right now it's getting boring to copy-paste "acl xxx" between each line, and I'd like to be able to write them as a block, just like for the servers, except that for the ACLs, I'm planning to support file inclusion too.

> Adding support for inversion in described way should also be trivial, as
> all you need to do is to reuse the global flag. And there is no
> uncertainty if:
>
> server aaa no check backup
>
> means:
>
> no server aaa check
> no server aaa backup
>
> or rather:
>
> no server aaa check
> server aaa backup

The later for me. But while I agree with the little uncertainty, it should not be a long-term problem once people get used to the way the configuration works. Some people might as well ask if "no server aaa check" does just remove the "check" option of "server aaa" or completely removes "server aaa" :-)

> >Also, when we have a CLI, I find it more convenient to do :
> >
> >config> backend bbb
> >backend bbb> server sss2
> >server sss2> no downinter
> >server sss2> no backup
> >server sss2> end
> >backend bbb> end
> >config>
>
> I'm not found of such "ends", it is very easy to overlook it and you newer
> know which block it ends exactly, especially if you keep wrong
> indentation. It works with cli but not with text configs.

I'm not fond of them either, but here I put them for the CLI. You also have it on the cisco, it is Ctrl-Z :-)
On the plain text configurations though, the simple fact that the next keyword does not match the current context indicates an end of context. It works pretty well on cisco even with keyword arguments. For instance, "ip address" is still in the interface context, while "ip route" gets out of the context.

The specific END keyword would be purely optional, to enforce the end of section. On the CLI, it could pretty well be the Ctrl-Z to go back one level when absolutely needed. But generally speaking, configurations would be dumped with possibly ambiguous parameters first.

> Please also note
> that introducing it changes the current syntax as currently "server" does
> not start nesting and there is no need to end it, the same for "backend",
> etc. If we force to end each server/backend we will break compatibility,
> if not we are risking that someone write somethig like:
>
> backend bbb 192.168.0.1:80
> server sss2 1.2.3.4:80
> maxconn 256

Yes I know, but in the following situation, "maxconn 256" would be for the backend because the server line has a parameter and so does not create a subcontext.

The following one is risky :

backend bbb 192.168.0.1:80

	server sss2
		address 1.2.3.4:80
	maxconn 256

Here, maxconn is in the server context. One possibility to definitely solve the issue would be to introduce new argument names when we implement the contexts, so that there's never any uncertainty. The current ones would still be accepted inline for backwards compatibility.

> >... or alternatively :
> >
> >config> backend bbb
> >backend bbb> server sss2
> >server sss2> no downinter
> >server sss2> no backup
> >server sss2> end backend
> >config>
>
> Right, "end backend" is better but it is still harder to implement as you
> need not only to end current backed but often (but not always) also
> current server or maybe also something else. My note about changing the
> syntax is also valid for this example.

yes, but as long as it's optional it's not a problem.

> >... than to do :
> >
> >config> backend bbb
> >backend bbb> no server sss2 downinter
> >backend bbb> no server sss2 backup
> >backend bbb> end
> >config>
> >
>
> Not sure if this "end" is required here. Each proxy ends with a next proxy
> (listen/backend/frontend/..) or "defaults" statement, so:
>
> backend bbb
> server sss2 1.2.3.4:80
> no server sss2 downinter
> no server sss2 backup
>
> backend bbb1
> (...)

Same here. I think that we're saying the same thing in fact :-)

> >I see no reason why the IP:port should be defined first. In the past, the
> >"mode" parameter had to be specified first in the "listen" sections, and
> >it was very annoying because it did not let the user enough freedom to
> >structure their configuration like they wanted. This has changed since the
> >introduction of the default sections I think.
>
> The reason is that a server consists of a name and IP:port. Everything
> else is optionall so if we force IP:port to be defined first there will no
> possibility for a user to left a phantom (server without IP:port defined).
> If we don't, one can define such a invalid server.
>
> ...
>
> OK, after some thinking for a cfg-file we can also post-check all servers
> and reject invalid ones like it is done with proxies. No idea how to solve
> it for cli?

It would result in an incomplete server which would not be used. With a CLI, each newly created server would appear with a "disabled" line and would only be activated once we enter "server xxx enable" or "no server xxx disable", and this entry would be refused if the server is incomplete. Just like you can get errors on a cisco when doing "no shut" on some interfaces, such as VLAN interfaces on old 3500XL switches when there's already one assigned.

> >>transinter? ;)
> >
> >or initinter ? :-)
>
> Hm, it is only "init" when haproxy starts, it is wrong IMHO to call
> it initinter, especially it is used for example after a first unsuccessful
> check.

OK.

> >I have a problem with "trans". It does not make me immediately think
> >about "transition". I think about "transfer", "translate", "transform"
> >but not "transition".
>
> transitinter?

still the problem with abbreviated and combined words. You don't know how many times I've seen people write "conntimeout" instead of "contimeout" because they were also used to see "maxconn".

I'm really wondering if "fastinter" was not better in the end. Most people would use it that way anyway, and the few ones who really want to have it slower than the default know why they would do it that way !

> >This is beginning to remind me the stupid names I had to find for the
> >timeouts until I got enough of them and used a "timeout" prefix to name
> >all others.
> >
> >Maybe the interval will finish like this. Using "inter 10000" to set the
> >default interval, and "inter transition 500" or "inter down 20000" for
> >the other ones. While this could quickly get boring on a single line
> >for each server, it may get more useable with default servers and server
> >subsections.
>
> IMHO it should be called same way for both single- and multi-line servers.

yes, I agree with that. This causes no trouble on a single line, though. It's just painfully long.

Best regards,
Willy Received on 2007/12/31 11:08

This archive was generated by hypermail 2.2.0 : 2007/12/31 11:15 CET