Hello Willy, So I'm fighting with dba97077 made by Frédéric Lécaille - it broke many things. E.g. you can't use "source", because that patch broke it. I'm curious how many other stuff got broken with those patches around default-server.
We need some CI (even if they will only build haproxy) and IMHO people with @haproxy.com mails should test their code before posting and merging :( In attachment there is patch fixed after your (Willy) review. Sorry for loop, I was focused on fixing all those problems with Frédérics patch I just didn't think how to replace do..while (which obviously works) with this cleaner version - thanks for this. The patch got even simpler. Thanks again, Michał 2017-03-27 15:34 GMT+02:00 Willy Tarreau <[email protected]>: > Hi Michal, > > On Mon, Mar 27, 2017 at 03:18:10PM +0200, Michal wrote: > > Hello, > > Thank you for your response. I agree with part about backward > compatibility > > and of course I don't want to break working things > > > > I prepared patch with described functionality and with two notes in doc > to > > let users know about this behaviour. > > Thanks. Just a few points : > > 1) cosmetic cleanups below : > > > diff --git a/doc/configuration.txt b/doc/configuration.txt > > index fb3e691..7f22782 100644 > > --- a/doc/configuration.txt > > +++ b/doc/configuration.txt > > @@ -11370,6 +11370,10 @@ track [<proxy>/]<server> > > enabled. If <proxy> is omitted the current one is used. If > disable-on-404 is > > used, it has to be enabled on both proxies. > > > > + Note: > > + Relative weight changes are propagated to all tracking servers. Each > > + tracking server will have it's weight recalculated separately. > > s/it's/its/ > > > + > > Supported in default-server: No > > > > verify [none|required] > > diff --git a/doc/management.txt b/doc/management.txt > > index 1d34f84..3f3730a 100644 > > --- a/doc/management.txt > > +++ b/doc/management.txt > > @@ -1694,6 +1694,10 @@ set weight <backend>/<server> <weight>[%] > > "admin". Both the backend and the server may be specified either by > their > > name or by their numeric ID, prefixed with a sharp ('#'). > > > > + Note: > > + Relative weight changes are propagated to all tracking servers. Each > > + tracking server will have it's weight recalculated separately. > > s/it's/its/ > > > + > > show cli sockets > > List CLI sockets. The output format is composed of 3 fields separated > by > > spaces. The first field is the socket address, it can be a unix > socket, a > > diff --git a/src/server.c b/src/server.c > > index 5589723..9462a16 100644 > > --- a/src/server.c > > +++ b/src/server.c > > @@ -45,6 +45,9 @@ > > static void srv_update_state(struct server *srv, int version, char > **params); > > static int srv_apply_lastaddr(struct server *srv, int *err_code); > > > > +const char *server_propagate_weight_change_request(struct server *sv, > > + const char *weight_str); > > + > > /* List head of all known server keywords */ > > static struct srv_kw_list srv_keywords = { > > .list = LIST_HEAD_INIT(srv_keywords.list) > > @@ -912,6 +915,8 @@ const char *server_parse_weight_change_request(struct > server *sv, > > struct proxy *px; > > long int w; > > char *end; > > + const char *msg; > > + int relative = 0; > > > > px = sv->proxy; > > > > @@ -933,6 +938,8 @@ const char *server_parse_weight_change_request(struct > server *sv, > > w = sv->iweight * w / 100; > > if (w > 256) > > w = 256; > > + > > + relative = 1; > > } > > else if (w < 0 || w > 256) > > return "Absolute weight can only be between 0 and 256 > inclusive.\n"; > > @@ -945,6 +952,34 @@ const char *server_parse_weight_change_request(struct > server *sv, > > sv->uweight = w; > > server_recalc_eweight(sv); > > > > + if (relative) { > > + msg = server_propagate_weight_change_request(sv, > weight_str); > > + if (msg != NULL) { > > + return msg; > > + } > > + } > > + > > + return NULL; > > +} > > + > > +const char *server_propagate_weight_change_request(struct server *sv, > > + const char *weight_str) > > +{ > > + if (sv->trackers == NULL) > > + return NULL; > > + > > + struct server *_propagated_server = sv->trackers; > > + const char *msg; > > Please don't mix code and declarations, building this emits a warning. > Also please avoid starting your variable name with an underscore, it > makes it look "special". > > > + > > + do { > > + msg = server_parse_weight_change_req > uest(_propagated_server, > > + weight_str); > > + > > + if (msg != NULL) { > > + return msg; > > + } > > + } while ((_propagated_server = _propagated_server->tracknext) != > NULL); > > The test at the beginning of the function and the loop combined into > this quite simplified form : > > struct server *tracker; > const char *msg; > > for (tracker = sv->trackers; tracker; tracker = > tracker->tracknext) { > msg = server_parse_weight_change_request(_propagated_server, > weight_str); > if (msg) > return msg; > } > > And 2) please send the resulting patch to the mailing list so that others > can participate and share their opinion. Some may say "wow cool" and others > might say "hey please no" (though I suspect that nobody will possibly > complain > about the change on relative weights only). And you'll get more testers. > Just for your information you don't need to be subscribed to the mailing > list, it's open and you can freely post. > I'm sorry. I tend to click "Reply" instead of "Reply to all". > > Thanks, > Willy >
0001-MINOR-server-Inherit-CLI-weight-changes-and-agent-ch.patch
Description: Binary data

