Skip to content

Increase max proctitle length from 255 to 1024#3843

Merged
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
pkhartsk:increase-max-proctitle-length
Jun 24, 2026
Merged

Increase max proctitle length from 255 to 1024#3843
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
pkhartsk:increase-max-proctitle-length

Conversation

@pkhartsk

@pkhartsk pkhartsk commented May 27, 2026

Copy link
Copy Markdown
Contributor

255 is too short if valkey-server is being run from a long path, especially if many fields are included such as in the "Process title set as expected" test in unit/other.tcl, where the max length for the cmdline is 1024, so having the same number in both makes sense.

Closes #3832.

255 is too short if valkey-server is being run from a long path,
especially if many fields are included such as in the
"Process title set as expected" test in `unit/other.tcl`,
where the max length for the cmdline is 1024, so having the
same number in both makes sense.

Signed-off-by: Petr Khartskhaev <pkhartsk@redhat.com>
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The SPT_MAXTITLE buffer size constant in src/setproctitle.c is increased from 255 to 1024, expanding the internal formatting buffer used by the setproctitle() function to accommodate longer process title strings.

Changes

Buffer Size Increase

Layer / File(s) Summary
SPT_MAXTITLE buffer size increase
src/setproctitle.c
The SPT_MAXTITLE macro constant increases from 255 to 1024, enlarging the internal char buf[SPT_MAXTITLE + 1] formatting buffer used by setproctitle().

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description check ✅ Passed The description matches the code change by explaining the proctitle buffer increase from 255 to 1024.
Title check ✅ Passed The title clearly matches the main change: increasing the maximum proctitle length from 255 to 1024.

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


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.

@dvkashapov

Copy link
Copy Markdown
Member

Looks like this will solve #3832

@codecov

codecov Bot commented May 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.57%. Comparing base (8ee3fd7) to head (8ee770c).
⚠️ Report is 46 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3843      +/-   ##
============================================
- Coverage     76.65%   76.57%   -0.09%     
============================================
  Files           162      162              
  Lines         80710    80710              
============================================
- Hits          61871    61805      -66     
- Misses        18839    18905      +66     
Files with missing lines Coverage Δ
src/setproctitle.c 76.19% <ø> (ø)

... and 22 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.

@dvkashapov dvkashapov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the actual bigger stack allocation would only happen if SPT.end - SPT.base is bigger than buf size

@sarthakaggarwal97 sarthakaggarwal97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@pkhartsk

Copy link
Copy Markdown
Contributor Author

Can this be merged?

@enjoy-binbin enjoy-binbin changed the title Increase max proctitle length Increase max proctitle length from 255 to 1024 Jun 24, 2026
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 7.2 Jun 24, 2026
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 8.0 Jun 24, 2026
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 8.1 Jun 24, 2026
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 9.0 Jun 24, 2026
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 9.1 Jun 24, 2026
@enjoy-binbin enjoy-binbin merged commit 2253589 into valkey-io:unstable Jun 24, 2026
64 checks passed
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 25, 2026
255 is too short if valkey-server is being run from a long path,
especially if many fields are included such as in the "Process title set
as expected" test in `unit/other.tcl`, where the max length for the
cmdline is 1024, so having the same number in both makes sense.

Closes #3832.

Signed-off-by: Petr Khartskhaev <pkhartsk@redhat.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 25, 2026
255 is too short if valkey-server is being run from a long path,
especially if many fields are included such as in the "Process title set
as expected" test in `unit/other.tcl`, where the max length for the
cmdline is 1024, so having the same number in both makes sense.

Closes #3832.

Signed-off-by: Petr Khartskhaev <pkhartsk@redhat.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 25, 2026
255 is too short if valkey-server is being run from a long path,
especially if many fields are included such as in the "Process title set
as expected" test in `unit/other.tcl`, where the max length for the
cmdline is 1024, so having the same number in both makes sense.

Closes #3832.

Signed-off-by: Petr Khartskhaev <pkhartsk@redhat.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 25, 2026
255 is too short if valkey-server is being run from a long path,
especially if many fields are included such as in the "Process title set
as expected" test in `unit/other.tcl`, where the max length for the
cmdline is 1024, so having the same number in both makes sense.

Closes #3832.

Signed-off-by: Petr Khartskhaev <pkhartsk@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: To be backported
Status: To be backported
Status: To be backported
Status: To be backported
Status: To be backported

Development

Successfully merging this pull request may close these issues.

[TEST-FAILURE] [err]: Process title set as expected in tests/unit/other.tcl

4 participants