If BalancerMembers are configured with a port number or path, then they are not editable in the BalancerManager interface. Clicking on the worker hostname in the interface brings up the 'Edit balancer settings' form instead of the 'Edit worker settings' form. These workers won't be editable via the interface: <Proxy balancer://test1> BalancerMember http://my.server.com:1234 BalancerMember http://otner.server.com:1234/myapp </Proxy> but this is OK: <Proxy balancer://test1> BalancerMember http://my.server.com BalancerMember http://otner.server.com </Proxy> In both cases, the actual proxying of requests seems to work fine; it's just the balancer manager that doesn't work right for the first cluster.
Created attachment 16600 [details] identify workers by worker->name instead of worker->hostname
Upon further investigation, I see that in the balancer_manager interface, the worker->hostname is used to identify each worker. However, the worker should be identified by worker->name (which includes the port number). In my previous example, if I click on the hostname of the first worker in the balancer manager, the URL should contain: 'w=my.server.com:1234' (instead of 'w=my.server.com' as it does now). Also, the value of the 'w' hidden field in the 'Edit worker' form should contain 'my.server.com:1234'. I've attached a patch to mod_proxy_balancer.c that makes that change, and seems to fix the problem.
Thanks for the patch. I have no comment on the functionality change, but this: ( sizeof( wsel->scheme ) + 3 ) looks odd, do you really want strlen() there rather than sizeof()? (i.e. length of string rather than size of pointer)
The same applies to ( sizeof( worker->scheme ) + 3 ). It needs to be strlen(worker->scheme)+3 here. Otherwise it will only work with the scheme http on 32 bit platforms :-). From my personal point of view I do not like the pointer arithmetic in this place and cutting the strings back and forth here. Although it would introduce some overhead here, I would prefer using ap_rprintf here and build things up from worker->hostname and worker->port. But I acknowledge that the pointer arithmetic way is already used in the code, so this is debatable. What worries me more (and this is not the fault or a shortcoming of your patch), that there is a need to discuss, what identifies a worker. Your patch will not fix your first example because ap_proxy_get_worker in proxy_util.c cuts off the path of the uri / url. So you are not able to access http://otner.server.com:1234/myapp from your example. And if it is decided that a worker is identified by its complete URL all parts of the code must follow it and the full url should be displayed in the manager then. I guess I should forward this discussion to the dev list.
The balancer-manager "application" should be used to change specific settings of a balancer or worker, not redefine one (imo). The code needs to be changed to more adequately reflect the "name" which is constant.
(In reply to comment #5) > The balancer-manager "application" should be used to change specific settings of a balancer or worker, > not redefine one (imo). Agreed. > The code needs to be changed to more adequately reflect the "name" which is > constant. And then it should 1. display this "name" in the manager and not only scheme and hostanme 2. enable the user to edit the specific settings of the worker we want to be changeable by the manager
I'll defer to the experts w/r/t the use of sizeof vs. strlen. A little disclosure: I'm not a C programmer, I'm just feeling around in the dark :-) I saw sizeof() used in some nearby lines, and just tried to emulate the style/approach used in the existing balancer_manager code. I agree with the suggestion that the worker name should bbe displayed in the UI as opposed to the hostname.
Fixed in HEAD. Likely to be in 2.1.9 and later
Reclassed to delete a version