Skip to content

Align THP advice in createLatencyReport with checkTHPEnabled#3947

Merged
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
enjoy-binbin:thp_advise
Jun 10, 2026
Merged

Align THP advice in createLatencyReport with checkTHPEnabled#3947
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
enjoy-binbin:thp_advise

Conversation

@enjoy-binbin

@enjoy-binbin enjoy-binbin commented Jun 9, 2026

Copy link
Copy Markdown
Member

The latency report told users to run 'echo never > .../enabled' to
disable THP, but checkTHPEnabled recommends 'echo madvise' and notes
that 'madvise' or 'never' are both valid. Make createLatencyReport
consistent: suggest 'madvise' in the command and mention both values
in the accompanying note.

This advice was originally added in 1461f02.

@enjoy-binbin
Align THP advice in createLatencyReport with checkTHPEnabled
3707499 
The latency report told users to run 'echo never > .../enabled' to
disable THP, but checkTHPEnabled recommends 'echo madvise' and notes
that 'madvise' or 'never' are both valid. Make createLatencyReport
consistent: suggest 'madvise' in the command and mention both values
in the accompanying note.

This advice was originally added in 1461f02.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@coderabbitai

coderabbitai Bot commented Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 94dc1007-2f46-4155-b14c-53c8a9f02f29

📥 Commits

Reviewing files that changed from the base of the PR and between d0ffbab and 3707499.

📒 Files selected for processing (1)
  • src/latency.c
📝 Walkthrough

Walkthrough

This PR updates the diagnostic advice in Valkey's LATENCY DOCTOR feature. The recommended kernel setting for Transparent Huge Pages changes from never to madvise in the corresponding shell command shown to users, with adjustments to supporting guidance text.

Changes

THP Recommendation Update

Layer / File(s) Summary
THP recommendation text update
src/latency.c		 The LATENCY DOCTOR diagnostic message updates the recommended transparent_hugepage/enabled setting from never to madvise, and adjusts the guidance text about handling already-created huge pages without special-casing scenarios where THP was previously disabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: aligning THP advice in createLatencyReport with the checkTHPEnabled function.
Description check ✅ Passed The description is directly related to the changeset, explaining why the THP advice needed to be updated and what changes were made.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Infer (1.2.0)
src/latency.c

src/latency.c:37:10: fatal error: 'hdr_histogram.h' file not found
37 | #include "hdr_histogram.h"
| ^~~~~~~~~~~~~~~~~
1 error generated.
Error: the following clang command did not run successfully:
/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/bin/clang-18
@/tmp/coderabbit-infer/370749909ad48a0fd13d2cba15d3d3271d729c1d-7580fd8ad6cc632a/tmp/clang_command_.tmp.2291bf.txt
++Contents of '/tmp/coderabbit-infer/370749909ad48a0fd13d2cba15d3d3271d729c1d-7580fd8ad6cc632a/tmp/clang_command_.tmp.2291bf.txt':
"-cc1" "-load"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../../facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib"
"-add-plugin" "BiniouASTExporter" "-plugin-arg-BiniouASTExporter" "-"
"-plugin-arg-BiniouASTExporter" "PREPEND_CURRENT_DIR=1"
"-plugin-arg-BiniouASTExporter" "MAX_STRING_SIZE=65535" "-cc1" "-triple"
"x86_64-unknown-linux-gnu" "-emit-obj" "-mrelax-all" "-disable-free"

... [truncated 682 characters] ...

"/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/lib/clang/18/include"
"-internal-isystem" "/usr/local/include" "-internal-isystem"
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
"-internal-externc-isystem" "/usr/include/x86_64-linux-gnu"
"-internal-externc-isystem" "/include" "-internal-externc-isystem"
"/usr/include" "-Wno-ignored-optimization-argument" "-Wno-everything"
"-ferror-limit" "19" "-fgnuc-version=4.2.1" "-fskip-odr-check-in-gmf"
"-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o"
"/tmp/coderabbit-infer/7580fd8ad6cc632a/file.o" "-x" "c" "src/latency.c"
"-O0" "-fno-builtin" "-include"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../lib/clang_wrappers/global_defines.h"
"-Wno-everything"

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@enjoy-binbin enjoy-binbin requested a review from zuiderkwast June 9, 2026 16:43
@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.63%. Comparing base (d0ffbab) to head (3707499).

Additional details and impacted files 
@@             Coverage Diff              @@
##           unstable    #3947      +/-   ##
============================================
- Coverage     76.66%   76.63%   -0.04%     
============================================
  Files           162      162              
  Lines         80733    80733              
============================================
- Hits          61897    61871      -26     
- Misses        18836    18862      +26
Files with missing lines Coverage Δ
src/latency.c 83.33% <ø> (ø)

... and 20 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@enjoy-binbin enjoy-binbin merged commit c3766df into valkey-io:unstable Jun 10, 2026
64 checks passed
@enjoy-binbin enjoy-binbin deleted the thp_advise branch June 10, 2026 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zuiderkwast zuiderkwast zuiderkwast approved these changes

Assignees

@enjoy-binbin enjoy-binbin

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@enjoy-binbin @zuiderkwast