> If it's about the stats page, I think we'd rather emit the FQDN there,
> what do you think ?
It is about the stats page, but also about our automation's ability to
take information from a service registry (which has host+port pairs +
health information) and identify which server in a HAProxy backend
maps to that service instance.
For example "enable server <b>/<s>" or "disable server <b>/<s>" has to
take the name, so if we can't rely on the server name being some
combination of host+port then for every up/down we'd have to "show
stat" and map from host+port to name for each update (as opposed to
issuing one socket "disable server". We also have lots of automation
(e.g. synapse, service monitoring scripts) which parse out the svname
to find the host+port, which while I can fix, is rather difficult.
Also there are a few stat commands (like "show servers state") which
don't show the host+port and only show the name.
> I'm having a few problems with changing the server's name :
> - it's a unique identifier, so changing it after unicity has been
> verified can be a problem and can become confusing in logs, stats
> or anything else.
I could verify that the name is unique perhaps? I don't feel like
logs/stats would be an issue because the only time you'd update the
name would be if the server is in MAINT (basically a free slot to
dynamically update to a new server)? Also logs usually have the actual
address information in addition to the name?
>
> - the name is used for lookups at various places (eg: use_server)
> and we really don't want to change it after the config is processed;
Is the concern that we might end up with a duplicate name and then the
lookup functions would return one or the other? Or is the concern that
changing it might cause bugs in those lookups?
>
> - you may even accidently set a name that you'll have a hard time
> entering again just to change it, forcing you to restart after a
> small mistake or an error in a script.
I don't really understand this, is the concern that you will enter a
name that conflicts so you then can't change it (because find_server
always returns the first server and not the one you set) or is it
something else?
> Given your intent is to provide human-readable information and that the
> server name is a config primary key (not always the most readable one
> by the way), better add a "desc <name>" config option that you can change
> at will on the CLI (set server <b>/<s> desc <desc>), and add it to the
> stats page (only when stats-show-desc is set), and let's add a sample
> fetch function to return it (logs, headers, anything). What do you think ?
I'll try it out, but that is a larger change that has backwards
compatibility concerns, and I'm still concerned about how does
automation go from address to name going forward. If I verified that
the name was unique would that be an ok solution?
>> +const char *update_server_name(struct server *server, const char *name,
>> const char *updater)
>> +{
>> + struct chunk *msg;
>> +
>> + msg = get_trash_chunk();
>> + chunk_reset(msg);
>> +
>> + if (!strcmp(name, server->id)) {
>> + chunk_appendf(msg, "no need to change the name");
>> + goto out;
>> + }
>> +
>> + char* old_name = server->id;
>
> This will not build, please avoid inline declarations.
I thought I declared it in /include/proto/server.h, oops I will double check!
>> + server->id = strdup(name);
>> + if (!server->id) {
>> + server->id = old_name;
>> + chunk_reset(msg);
>> + chunk_appendf(msg, "could not update %s/%s name",
>> + server->proxy->id, server->id);
>> + } else {
>> + chunk_appendf(msg, "%s/%s changed its name from %s to %s",
>> + server->proxy->id, old_name, old_name, name);
>> + free(old_name);
>> + }
>
> I think you'll have less operations to perform if you first allocate the
> new name then free the old one and switch the pointers in case of success.
> It may also require less locking later for multithreading.
Ok I will work on it!
Thanks for the reply!
-Joey
