Rename redis in aof logs and proc title redis-aof-rewrite to valkey-aof-rewrite. #393
Conversation
Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
|
TSC had a discussion on this topic (#361) The rule of thumb is that command error messages should be retained for backward compatibility. Log lines are in general OK to change. We haven't discussed the proc title; though I consider it much closer to logs (hence safe to change). @zuiderkwast thoughts? |
src/aof.c
Outdated
| @@ -996,7 +996,7 @@ int startAppendOnly(void) { | |||
|
|
|||
| if (rewriteAppendOnlyFileBackground() == C_ERR) { | |||
| server.aof_state = AOF_OFF; | |||
| serverLog(LL_WARNING,"Redis needs to enable the AOF but can't trigger a background AOF rewrite operation. Check the above logs for more info about the error."); | |||
| serverLog(LL_WARNING,"Server needs to enable the AOF but can't trigger a background AOF rewrite operation. Check the above logs for more info about the error."); | |||
There was a problem hiding this comment.
This is part of #207 so I will tag @0del because he is assigned to it. @0del are you still working on the remaining logging?
In #207 we discussed this style:
serverLog(LL_WARNING,"%s needs to enable the AOF but can't trigger a background AOF rewrite operation. Check the above logs for more info about the error.",
SERVER_TITLE);
... because then it would be easy patch the SERVER_TITLE macro to be "Redis" to get the old logging. But then we discussed it a bit more and decided that most of the logging isn't a problem about redis compatibility, so I think we don't need this complexity.
I'm OK with "Server", but probably better grammar is "The server" in this case.
There was a problem hiding this comment.
Thank you for the review, Changed the string "Server" to "The server" and updated the diff.
src/aof.c
Outdated
| @@ -2471,7 +2471,7 @@ int rewriteAppendOnlyFileBackground(void) { | |||
| char tmpfile[256]; | |||
|
|
|||
| /* Child */ | |||
| serverSetProcTitle("redis-aof-rewrite"); | |||
| serverSetProcTitle("valkey-aof-rewrite"); | |||
There was a problem hiding this comment.
@PingXie You're right that proc title is not logging.
For the most reasonable result we could check if Valkey was started using a redis-server symlink, similar to how we check if we was started as valkey-check-aof and other names, here:
Line 7019 in a5a1377
Maybe it's too complex and not important. I'm not sure.
There was a problem hiding this comment.
check if Valkey was started using a redis-server symlink
+1
There was a problem hiding this comment.
I have tried to add a flag similar to server.sentinel_mode, I am still not sure whether it is appropriate and best way.
Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #393 +/- ##
============================================
+ Coverage 68.35% 68.42% +0.06%
============================================
Files 108 109 +1
Lines 61563 61671 +108
============================================
+ Hits 42084 42198 +114
+ Misses 19479 19473 -6
|
Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
zuiderkwast
added
release-notes
…of-rewrite (valkey-io#393) Renamed redis to valkey/server in aof.c serverlogs. The AOF rewrite child process title is set to "redis-aof-rewrite" if Valkey was started from a redis-server symlink, otherwise to "valkey-aof-rewrite". This is a breaking changes since logs are changed. Part of valkey-io#207. --------- Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com>
Renamed redis to valkey/server in aof.c serverlogs.
The AOF rewrite child process title is set to "redis-aof-rewrite" if Valkey was started from a redis-server symlink, otherwise to "valkey-aof-rewrite".
This is a breaking changes since logs are changed.
Part of #207.