Update lateny report output message #426

hwware · 2024-05-02T18:23:15Z

Update "Redis" to "Valkey"
Remove Dave dialog, and update to normal output message

I think this should be not break change.

Signed-off-by: hwware <wen.hui.ware@gmail.com>

codecov · 2024-05-02T18:36:07Z

Codecov Report

Attention: Patch coverage is 42.85714% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 68.43%. Comparing base (443d80f) to head (d555d30).

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

Files	Coverage Δ
src/latency.c	`81.49% <42.85%> (ø)`

... and 13 files with indirect coverage changes

enjoy-binbin

yean, it look like a minor changes

zuiderkwast

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

zuiderkwast · 2024-05-07T03:41:19Z

src/latency.c

        }

        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");


The command pages are not dirs so I think a trailing slash makes it a broken link.

Suggested change

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");

enjoy-binbin · 2024-05-07T04:07:02Z

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.

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.

zuiderkwast · 2024-05-07T05:23:34Z

https://youtu.be/Mme2Aya_6Bc

https://youtu.be/twTESNFc1ww

zuiderkwast · 2024-05-07T05:27:23Z

@madolson Let's keep the Dave dialog?

hwware · 2024-05-07T13:57:43Z

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.

Sorry, I never see this movie. Now i know where Dave comes from, I am just curious why Dave here.
@madolson DO you think we should keep Dave here?

madolson · 2024-05-08T19:30:44Z

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.

madolson

Minor grammar fixes, and some of these events no longer seem relevant.

madolson · 2024-05-08T19:27:53Z

src/latency.c

    } 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");


Suggested change

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");

madolson · 2024-05-08T19:32:13Z

src/latency.c

    } 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");


Suggested change

report = sdscat(report,"\nHere are a few advices for you:\n\n");

report = sdscat(report,"\nHere is some advice for you:\n\n");

madolson · 2024-05-08T19:34:36Z

src/latency.c

@@ -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");


Maybe pin for a second issue, we should consider updating this to reference to include lazy deletion.

madolson · 2024-05-08T19:35:04Z

src/latency.c

        }

        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");


Maybe pin for a second issue. We incrementally do mass evictions now, so I'm not sure this is much of an issue anymore.

zuiderkwast · 2024-05-08T21:42:46Z

I'm fine with removing most of these, if the rest of you want that.

"I honestly think you ought to sit down calmly, take a stress pill, and think things over"

This one is in the 2nd of the two youtube videos I posted above.

madolson · 2024-05-13T00:21:25Z

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.

zuiderkwast · 2024-05-13T17:52:04Z

Can we agree on keeping only one "i can't do that dave" and delete the others? (👍 = yes)

Update lateny report output message

d555d30

Signed-off-by: hwware <wen.hui.ware@gmail.com>

hwware requested review from zuiderkwast and enjoy-binbin May 6, 2024 21:12

enjoy-binbin approved these changes May 7, 2024

View reviewed changes

zuiderkwast reviewed May 7, 2024

View reviewed changes

hwware requested a review from madolson May 7, 2024 14:00

madolson reviewed May 8, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update lateny report output message #426

Update lateny report output message #426

hwware commented May 2, 2024 •

edited

codecov bot commented May 2, 2024

enjoy-binbin left a comment

zuiderkwast left a comment •

edited by hwware

zuiderkwast May 7, 2024

enjoy-binbin commented May 7, 2024

zuiderkwast commented May 7, 2024

zuiderkwast commented May 7, 2024

hwware commented May 7, 2024

madolson commented May 8, 2024

madolson left a comment

madolson May 8, 2024

madolson May 8, 2024

madolson May 8, 2024

madolson May 8, 2024

zuiderkwast commented May 8, 2024

madolson commented May 13, 2024

zuiderkwast commented May 13, 2024

	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");

	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");

	report = sdscat(report,"\nHere are a few advices for you:\n\n");
	report = sdscat(report,"\nHere is some advice for you:\n\n");

Update lateny report output message #426

Are you sure you want to change the base?

Update lateny report output message #426

Conversation

hwware commented May 2, 2024 • edited

codecov bot commented May 2, 2024

Codecov Report

enjoy-binbin left a comment

Choose a reason for hiding this comment

zuiderkwast left a comment • edited by hwware

Choose a reason for hiding this comment

zuiderkwast May 7, 2024

Choose a reason for hiding this comment

enjoy-binbin commented May 7, 2024

zuiderkwast commented May 7, 2024

zuiderkwast commented May 7, 2024

hwware commented May 7, 2024

madolson commented May 8, 2024

madolson left a comment

Choose a reason for hiding this comment

madolson May 8, 2024

Choose a reason for hiding this comment

madolson May 8, 2024

Choose a reason for hiding this comment

madolson May 8, 2024

Choose a reason for hiding this comment

madolson May 8, 2024

Choose a reason for hiding this comment

zuiderkwast commented May 8, 2024

madolson commented May 13, 2024

zuiderkwast commented May 13, 2024

hwware commented May 2, 2024 •

edited

zuiderkwast left a comment •

edited by hwware