Hi! This solution very much solves the problem that I have been facing i.e. large number of acl rules causing latency in requests. Been in discussions separately about it and today I got a chance to test out this patch. I report that it works great! I have been able to route 150k backends with this and the latency added because of the dynamic lookup is in order of microseconds (compared to 24ms earlier).
The usage 'use_backend bk_%[hdr(Host)] if TRUE' works for my use-case but originally I was wondering if one could do a map based lookup for the backend. As posted here : http://stackoverflow.com/questions/22025412/how-to-use-thousands-of-backends-in-haproxy-is-the-new-map-feature-useful-for-t Most of the issues in the above question are now solved, but I tested this with the patch -> use_backend bk_%[hdr(Host), map(host_to_backend_map.file)] if TRUE And it does not work. I am not yet familiar with code to determine why this does not work. Again, the current proposal works well for me but an enhancement should probably consider using maps within dynamic lookup. +1 for the patch. Thanks. Rajat > Hi Bertrand, > > On Sun, Mar 23, 2014 at 04:18:44PM +0100, Bertrand Jacquin wrote: > > Hi, > > > > I did this patch for dev19 some time ago but I am still not sure whether > > it is the best way to do it or not, and did not have the time to discuss > > it since. As the latest changes broke it and forced me to rebase it, and > > it's very useful for us, I'd like to propose it for inclusion before the > > final release if you think it's OK, or to discuss how it should be done. > > Great! > > > Main purpose wanted to achieve is it be able to use many backends > > without the need to declare each routing process from frontend to > > backend and instead use generic and dynamic switching when a sane > > parameter can be used from user request using the logformat logic. For > > example when we have a backend farm dedicated to each 'Host: ' http-header, > > it's pain in the ass to have to declare the backend and the relevant > > use_backend. > > Yes I know there's this request coming from time to time. In fact it > was even planned to work like this before we finally went with ACLs > and use_backend, but we felt it would be a too limited design (eg: no > choice of other routing key). > > > With the proposed solution, you first need to declare a dynamic > > use_backend as the following : > > > > use_backend bk_cust_%[hdr(Host)] if { hdr(Host) -m found } > > > > And then to declare the needed backend. For every new vhost hosted will only > > need to add the backend section to the configuration. > > I'm not opposed to the feature at all, in fact I've even been involved > in a discussion about something more or less in this vein recently. But > I'm having some fears about the use of the %[] form in a use_backend > directive. Indeed, this string format was initially done only for > logformat. Then it was adopted for unique-id. Then for all http-request > directives. And we start to see from time to time people trying to use > it in places which have no relation with it (eg: in ACL declaration). > > I'm seeing several solutions in fact : > - yours above > > - append some argument to use_backend to indicate that it's a logformat > string or a dynamic backend (eg: use_backend -d foo%[bar]), but "-d" > might be a valid backend name, so ... > > - have a different directive name (eg: use_backend_dyn or use_backend_lf), > but that might increase the confusion for some users who will not > necessarily know that they're part of the same ruleset. > > - put use_backend in http-request rules and clearly state that only > http-request can use logformat, but then it means that we can't do > it on TCP, and it can further confuse users who will try to chain > multiple backends using http-request rules in backends. > > So in the end, I tend to think that your solution might still be the best > one, or the least confusing one. But I'd like to read other people's comments > about this, maybe someone has a better idea. > > > More detailed usage and implementation in patch itself. > > > > It was rebased on commit 0e9b1b4d1f0efc5e46a10371d9be21e97581faab. > > OK thanks! > > > A current limitation of that patch can be that if a dynamic use_backend > > is evaluated and the named backend is not found, the default_backend is > > then used. This is not a need we have, but some may complain about it. > > Well, *all* use_backend rules are final today. So if the condition after > use_backend is true, then the rule is executed and the evaluation is > stopped. So I think this is the proper behaviour. Doing it differently > could instead increase confusion, because if we changed the way it works, > some people might then complain for example that when use_backend directs > to a backend whose all servers are dead, we ought to go back to evaluate > all the rules instead. > > Also I could easily see some security issues by not having this behaviour, > because is multiple dynamic rules are chained and it is not at all expected > that someone can reach someone else's backend in a multi-hosted environment, > we could end up still mixing the sets. > > It could be argued that defaulting to default_backend is not logical however > and that instead we should return a 503. But the current design allows both > behaviours if I understand it right, if you don't want to have a > default_backend, by not setting one you'll get the 503. > > One step further would be to make the condition optional on use_backend, > as it is on redirect for example. > > > Future enhancement could be to use the same logic for use-server and > > many other part such as reqadd.. > > OK for use-server, but NAK for req*. These ones have been used for many > years in legacy configs and we would definitely break them by doing so. > The problem is the % character which is used a lot in HTTP and has a lot > of chances of being already present in some configs. That's exactly the > same reason why we support logformat in "http-request redirect" and not > in "redirect". One is compatible with deployed configs and the other one > is new and supports extensions. > > The patch looks fine. I'm tempted to merge it as-is, but I'd like to gather > some opinions first in case we'd have to change some behaviour or syntax. > In the absence of comments, I'll merge it soon. > > Thanks! > Willy >
