Add 'set-proc-title' config so that this mechanism can be disabled

[clan]

if option `setproctitle' is no, then do nothing for proc title.

The reason has been explained in here, with the copy bellow.

We update redis to 2.8.8, then found there are some side effect when redis always change the process title.

We run several slave instance on one computer, and all these salves listen on unix socket only, then ps will show:
1 S redis 18036 1 0 80 0 - 56130 ep_pol 14:02 ? 00:00:31 /usr/sbin/redis-server *:0
1 S redis 23949 1 0 80 0 - 11074 ep_pol 15:41 ? 00:00:00 /usr/sbin/redis-server *:0
for redis 2.6 the output of ps is like following:
1 S redis 18036 1 0 80 0 - 56130 ep_pol 14:02 ? 00:00:31 /usr/sbin/redis-server /etc/redis/a.conf
1 S redis 23949 1 0 80 0 - 11074 ep_pol 15:41 ? 00:00:00 /usr/sbin/redis-server /etc/redis/b.conf

Later is more informational in our case. The situation is worse when we manage the config and process running state by salt. Salt check the process by running "ps | grep SIG" (for Gentoo System) to check the running state, where SIG is the string to search for when looking for the service process with ps. Previously, we define sig as "/usr/sbin/redis-server /etc/redis/a.conf". Since the ps output is identical for our case, so we have no way to check the state of specified redis instance.

So, for our case, we prefer the old behavior, i.e, do not change the process title for the main redis process. Or add an option such as "setproctitle [yes|no]" to control this behavior.

There are some discussion in issue #694 , #1979, #2081 , but in our case, keep the title untouched is the best option, thanks.

[clan]

nobody care this? thanks.

[GhostZCH]

This is very important when running several unixsock-binded server on the same machine.Hope this can be merged.

[yossigo]

This makes sense to me, @redis/core-team any other thoughts?

[oranagra]

i agree with the idea of a flag to avoid changing the process title at all.
but it also looks like a nice idea to let the config actually explicitly set the title (#694) or possibly by using some macros (#2081).
although i don't like the idea of adding such a mass of code like in the above, and also not sure i like the fact redis keeps updating it again and again at runtime. so maybe the macro language should be more limited (i.e. include port, unix socket, config file name, and other non-mutable configs)

[clan]

we've requiring this option since 2004, and doing manual patch since then. The default value of setproctitle can be changed to yes (to be consistent with redis's current behaviour).

[oranagra]

i'm ok merging it with a default of yes (and a mention in the default redis.conf), but them most users will still suffer from the problem. maybe the right solution is to just make this change:

@@ -5606,8 +5606,9 @@ void redisSetProcTitle(char *title) {
     if (server.cluster_enabled) server_mode = " [cluster]";
     else if (server.sentinel_mode) server_mode = " [sentinel]";
 
-    setproctitle("%s %s:%d%s",
+    setproctitle("%s %s %s:%d%s",
         title,
+        server.configfile ? server.configfile : "-",
         server.bindaddr_count ? server.bindaddr[0] : "*",
         server.port ? server.port : server.tls_port,
         server_mode);

@clan this would solve your problem, right?
@yossigo WDYT?

[clan]

@oranagra ok，I'll change the default value to yes.

About your change on the proc title, it's ok to add configfile in, but sometimes I prefer to keep the command line intact, so add an option is still required in my case.

[madolson]

I would be most inclined to:

1. Add in the unix socket port into the proc title if it's present, so that the proctitle defines a unique Redis server. I am less sure about the config file. Even though it's immutable, it doesn't define a uniqueness.
2. Allow defining a "name" for the Redis server in the config file, that would get added into the proctitle. I think having a generic "name" would be useful elsewhere. It would be nice to provide human readable names for what nodes are your replicas, masters, and peers in the cluster bus. (Nobody likes looking at 40 character hex strings and trying to figure out)

[oranagra]

i'm good with @madolson proposal, when TCP port is not present, show the unix socket name instead of 0.
that is less likely to break anyone's code compared to my suggestion of adding the config file name.

in addition to that we can either add an optional proc-title config that completely replaces the automatic title (hides the port etc)
or an additional instance-name config that will be shown in addition to the reset of the automatic options.
either of these two is an optional (disabled by default) config, so they can come instead of the current setproctitle config in this PR (make is redundant).

@madolson @clan what do you think of that?

[clan]

@oranagra I don't mind to add more content to the proc title, and it's a better step over current implementation.
But in my case, that would add some extra work to keep the config and check consistent, besides of define the name of the config file, so keep the proc title unchanged is still my prefer option. Also, change proc title is a rarely case in most daemon software.

[yossigo]

+1 for reporting Unix socket. I think this along with the option to turn off setproctitle should address this fully. As for an instance name, I'm not basically against this idea but I think it should be considered in a bigger context so I would avoid doing that as part of this localized fix.

[oranagra]

@yossigo so you'd like to avoid instance-name config now. but what about proc-title config?
maybe it can be combined together with / instead of the current setproctitle boolean of this PR.
i.e. a string config that can be set to one of these 3 modes:

• proc-title "auto" - will be the default (current redis code)
• proc-title "disabled" - like this PR suggests, don't try to change it at all.
• proc-title "any other string" - will just set the proc-title to that user provided string.

regarding showing the unix socket, do we show it always (when present), or only when the TCP port is unset? (more backwards compatible)

[clan]

@yossigo so you'd like to avoid instance-name config now. but what about proc-title config?
maybe it can be combined together with / instead of the current setproctitle boolean of this PR.
i.e. a string config that can be set to one of these 3 modes:

• proc-title "auto" - will be the default (current redis code)
• proc-title "disabled" - like this PR suggests, don't try to change it at all.
• proc-title "any other string" - will just set the proc-title to that user provided string.

regarding showing the unix socket, do we show it always (when present), or only when the TCP port is unset? (more backwards compatible)

Basically, I don't against this option, since what this PR requested is included in.
But, my question is, do we have use case to support a more complicated option? Since it's not related to any core feature of redis. Indeed, I don't understand why we change proc title to show just one tcp bind_addr:port either, if you want to distinguish different instances, isn't it better to choose a more meaningful config file name in command line? Anyway, this feature has been existed for over six years, it's ok to keep compatiable w/ the old behaviour.

[yossigo]

@clan This feature could arguably be avoided in the first place, especially given the trouble we had recently with setproctitle() implementation bugs. But I don't think we should drop it now.

Added a few minor changes to your PR.
I'm also going to add a simple way to format the title to address the other concerns (will open a separate PR for that).

Thanks for your patience!

[clan]

@yossigo thanks. and yes, I understand the importance to don't break user's exist code silently.

[madolson]

I am fine with doing doing instance-name later as well.