-
Notifications
You must be signed in to change notification settings - Fork 932
Cluster: Assign default human-readable node name when starting server #2777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: unstable
Are you sure you want to change the base?
Conversation
Signed-off-by: Zhijun <[email protected]>
Signed-off-by: Zhijun <[email protected]>
Signed-off-by: Zhijun <[email protected]>
Signed-off-by: Zhijun <[email protected]>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #2777 +/- ##
===========================================
Coverage 72.47% 72.48%
===========================================
Files 128 128
Lines 71624 70156 -1468
===========================================
- Hits 51908 50851 -1057
+ Misses 19716 19305 -411
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I generally use the first/last few characters of node-id while searching and I've never seen it conflicting with other node's id.
@zhijun42 Wouldn't you be able to set this externally on the engine by yourself?
|
On this note, Docker's approach of generating random name for resource has always been interesting, easier to spot/remember. https://github.com/moby/moby/blob/master/pkg/namesgenerator/names-generator.go |
Signed-off-by: Zhijun <[email protected]>
|
@hpatro Thanks for reviewing!
Yeah the first/last few characters of node ID are unique enough. I assume your use case is perhaps like this: You have a node that did something interesting and/or you're interested in it, then you search through the logs using that node ID to see how this node interacted with others. My use case is a bit different though. When developing locally (non Docker/K8s), I have a mini cluster running where each node is running in a terminal tab. I'd like to fully observe all behaviors of the cluster, so inside each terminal tab I'm reading through this node’s log stream and would like to instantly recognize peers it’s communicating with. A 40-hex node ID is cognitively heavy; 127.0.0.1:7002 is instant. Note that I'm not searching for any particular node, I'm simply reading the entire log stream. And having logs like these would be very useful for me:
Yeah we can always manually assign names to nodes, but I don't wanna bother doing this every time I start up a cluster. The default combination of IP + port proposed here is pretty recognizable for me.
TIL! Browsed through the list and just found a lot of great guys in our history. We can probably do the same thing here in Valkey. My approach of using IP + port is functionally the same as using scientists' names - just helping developers associate a server node with some string they can immediately recognize. Since I can easily recognize IP + port, that is what I propose here. The thing is Valkey cluster has most logs print out the node ID and nodename, but the nodename is always empty unless explicitly set, which is not great user experience for me. |
Signed-off-by: Zhijun <[email protected]>
By default, servers in a cluster don't have a human-readable nodename unless explicitly set by user. This can be annoying when reading the server logs during local development, since we would need to manually lookup the unique node ID (40 chars long) to figure out its port number. Most logging statements in the cluster are in the form of
serverLog(LL_NOTICE, "Something.. %.40s (%s) something..", node->name, node->human_nodename);, and thus this would always result in an empty()like the following:Assigning a default human-readable nodename solves the annoyance:
For now I use the node's IP and port combination as the nodename. If users have personal preference on the format, they can always update nodename via
CONFIG SET cluster-announce-human-nodename MYNAMEcommand or edit the config file.