valkey-io / valkey

A flexible distributed key-value datastore that is optimized for caching and other realtime workloads.
https://valkey.io
Other
17.72k stars 670 forks source link

syslog-ident config parameter in Valkey #301

Closed hwware closed 7 months ago

hwware commented 7 months ago

In valkey.conf and sentinel.conf, there are 2 parameters:

syslog-ident redis

syslog-ident sentinel

Do we need update syslog-ident redis to syslog-ident valkey or just keep it?

Thanks

PingXie commented 7 months ago

sentinel looks fine to me.

I think it makes sense to default to "redis" for valkey-server's syslog-ident.

Again, IMO, the ideal experience is that without the compat switch (separate discussion on run-time vs build-time), we default to valkey for all identities; and then there is this single compat switch, when enabled, we get full compat with redis, for both application developers and operators/admins. over time, the community will add more native support for Valkey, and there will be a tipping point when the majority of users can run the image by default just fine without the compat layer.

zuiderkwast commented 7 months ago

I think we shall change it. The question is whether we need the extended compatibility config for it or if we can just change it.

Syslog is disabled by default.

The syslog-ident can be configured. "redis" is just a default value.

Relevant lines:

src/config.c:    createStringConfig("syslog-ident", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.syslog_ident, "redis", NULL, NULL),
valkey.conf:# syslog-ident redis

IMO, we can just change it. A changed config default is a breaking change though, so it should be mentioned in the release notes.

karthyuom commented 7 months ago

@zuiderkwast, I have raised the PR for the above, please review. Thanks.