-
Notifications
You must be signed in to change notification settings - Fork 695
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
Update lateny report output message #426
Conversation
Signed-off-by: hwware <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #426 +/- ##
============================================
- Coverage 68.44% 68.43% -0.01%
============================================
Files 109 109
Lines 61671 61671
============================================
- Hits 42212 42206 -6
- Misses 19459 19465 +6
|
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.
yean, it look like a minor changes
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'm a little sad to remove the Dave dialog. It's a reference to the film 2001 A Space Odyssey. Have you watched it? It's a brilliant film from early time, 1969 IIRC. The computer talks to the crewman Dave on a space ship.
The film is producted in 1969 https://en.wikipedia.org/wiki/2001:_A_Space_Odyssey
} | ||
|
||
if (advise_slowlog_tuning) { | ||
report = sdscatprintf(report,"- Your current Slow Log configuration only logs events that are slower than your configured latency monitor threshold. Please use 'CONFIG SET slowlog-log-slower-than %llu'.\n", (unsigned long long)server.latency_monitor_threshold*1000); | ||
} | ||
|
||
if (advise_slowlog_inspect) { | ||
report = sdscat(report,"- Check your Slow Log to understand what are the commands you are running which are too slow to execute. Please check https://redis.io/commands/slowlog for more information.\n"); | ||
report = sdscat(report,"- Check your Slow Log to understand what are the commands you are running which are too slow to execute. Please check https://valkey.io/commands/slowlog/ for more information.\n"); |
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.
The command pages are not dirs so I think a trailing slash makes it a broken link.
report = sdscat(report,"- Check your Slow Log to understand what are the commands you are running which are too slow to execute. Please check https://valkey.io/commands/slowlog/ for more information.\n"); | |
report = sdscat(report,"- Check your Slow Log to understand what are the commands you are running which are too slow to execute. Please check https://valkey.io/commands/slowlog for more information.\n"); |
wow, i haven't watched it, don't know the history. I did a search and it looks like a very exciting film. Can't imagine a movie this old, i am going to add it to my list and then try to find time to take a look. If that's the case, I think maybe we can keep it? It seems that the modification is not particularly important, but it seems to be very memorable. |
@madolson Let's keep the Dave dialog? |
Sorry, I never see this movie. Now i know where Dave comes from, I am just curious why Dave here. |
I have had more than one conversation with someone asking who "dave" was, and then I had to explain to them how it's a reference to 2001 a space odyssey, and then they told me they have never heard of the movie. Some of them are also weird, like "I honestly think you ought to sit down calmly, take a stress pill, and think things over". I think someone might take that the wrong way. "I'm sorry, Dave, I can't do that." Is the iconic line, it's also on an error path since latency is disabled. So I guess my vote is let's just keep that one and delete all the other references. |
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.
Minor grammar fixes, and some of these events no longer seem relevant.
} else if (eventnum > 0 && advices == 0) { | ||
report = sdscat(report,"\nWhile there are latency events logged, I'm not able to suggest any easy fix. Please use the Redis community to get some help, providing this report in your help request.\n"); | ||
report = sdscat(report,"\nThere are latency events logged, they are not easy fix. Please get some help from Valkey community, providing this report in your help request.\n"); |
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.
report = sdscat(report,"\nThere are latency events logged, they are not easy fix. Please get some help from Valkey community, providing this report in your help request.\n"); | |
report = sdscat(report,"\nThere are latency events logged, they are not easy to fix. Please get some help from Valkey community, providing this report in your help request.\n"); |
} else { | ||
/* Add all the suggestions accumulated so far. */ | ||
|
||
/* Better VM. */ | ||
report = sdscat(report,"\nI have a few advices for you:\n\n"); | ||
report = sdscat(report,"\nHere are a few advices for you:\n\n"); |
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.
report = sdscat(report,"\nHere are a few advices for you:\n\n"); | |
report = sdscat(report,"\nHere is some advice for you:\n\n"); |
@@ -437,19 +437,19 @@ sds createLatencyReport(void) { | |||
} | |||
|
|||
if (advise_hz && server.hz < 100) { | |||
report = sdscat(report,"- In order to make the Redis keys expiring process more incremental, try to set the 'hz' configuration parameter to 100 using 'CONFIG SET hz 100'.\n"); | |||
report = sdscat(report,"- In order to make the Valkey keys expiring process more incremental, try to set the 'hz' configuration parameter to 100 using 'CONFIG SET hz 100'.\n"); | |||
} | |||
|
|||
if (advise_large_objects) { | |||
report = sdscat(report,"- Deleting, expiring or evicting (because of maxmemory policy) large objects is a blocking operation. If you have very large objects that are often deleted, expired, or evicted, try to fragment those objects into multiple smaller objects.\n"); |
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.
Maybe pin for a second issue, we should consider updating this to reference to include lazy deletion.
} | ||
|
||
if (advise_large_objects) { | ||
report = sdscat(report,"- Deleting, expiring or evicting (because of maxmemory policy) large objects is a blocking operation. If you have very large objects that are often deleted, expired, or evicted, try to fragment those objects into multiple smaller objects.\n"); | ||
} | ||
|
||
if (advise_mass_eviction) { | ||
report = sdscat(report,"- Sudden changes to the 'maxmemory' setting via 'CONFIG SET', or allocation of large objects via sets or sorted sets intersections, STORE option of SORT, Redis Cluster large keys migrations (RESTORE command), may create sudden memory pressure forcing the server to block trying to evict keys. \n"); | ||
report = sdscat(report,"- Sudden changes to the 'maxmemory' setting via 'CONFIG SET', or allocation of large objects via sets or sorted sets intersections, STORE option of SORT, Valkey Cluster large keys migrations (RESTORE command), may create sudden memory pressure forcing the server to block trying to evict keys. \n"); |
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.
Maybe pin for a second issue. We incrementally do mass evictions now, so I'm not sure this is much of an issue anymore.
I'm fine with removing most of these, if the rest of you want that.
This one is in the 2nd of the two youtube videos I posted above. |
I had seen the movie and had no recollection of that phrase 😓. If other people do think it's popular, I would be fine keeping it, I just do think it's much less more recognizable than the "I can't do that" line. |
Can we agree on keeping only one "i can't do that dave" and delete the others? (👍 = yes) |
Replaced by #644 |
I think this should be not break change.