Skip to content
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

Log as primary role (M) instead of child process (C) during startup #1282

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

azuredream
Copy link
Contributor

@azuredream azuredream commented Nov 8, 2024

Init server.pid earlier to keep log message role consistent.

Closes #1206.

Before:

24881:C 21 Oct 2024 21:10:57.165 * oO0OoO0OoO0Oo Valkey is starting oO0OoO0OoO0Oo
24881:C 21 Oct 2024 21:10:57.165 * Valkey version=255.255.255, bits=64, commit=814e0f55, modified=1, pid=24881, just started
24881:C 21 Oct 2024 21:10:57.165 * Configuration loaded
24881:M 21 Oct 2024 21:10:57.167 * Increased maximum number of open files to 10032 (it was originally set to 1024).

After:

68560:M 08 Nov 2024 16:10:12.257 * oO0OoO0OoO0Oo Valkey is starting oO0OoO0OoO0Oo
68560:M 08 Nov 2024 16:10:12.257 * Valkey version=255.255.255, bits=64, commit=45d596e1, modified=1, pid=68560, just started
68560:M 08 Nov 2024 16:10:12.257 * Configuration loaded
68560:M 08 Nov 2024 16:10:12.258 * monotonic clock: POSIX clock_gettime

See also #1301 if you are interested.

Signed-off-by: azuredream <zhaozixuan67@gmail.com>
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.68%. Comparing base (45d596e) to head (6c7600a).
Report is 1 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1282      +/-   ##
============================================
+ Coverage     70.56%   70.68%   +0.11%     
============================================
  Files           114      114              
  Lines         63147    63147              
============================================
+ Hits          44560    44633      +73     
+ Misses        18587    18514      -73     
Files with missing lines Coverage Δ
src/server.c 87.69% <100.00%> (ø)

... and 13 files with indirect coverage changes

@zuiderkwast zuiderkwast added the release-notes This issue should get a line item in the release notes label Nov 11, 2024
@zuiderkwast zuiderkwast merged commit 0b5b2c7 into valkey-io:unstable Nov 11, 2024
48 checks passed
zvi-code pushed a commit to zvi-code/valkey-z that referenced this pull request Nov 13, 2024
…alkey-io#1282)

Init server.pid earlier to keep log message role consistent.

Closes valkey-io#1206.

Before:
```text
24881:C 21 Oct 2024 21:10:57.165 * oO0OoO0OoO0Oo Valkey is starting oO0OoO0OoO0Oo
24881:C 21 Oct 2024 21:10:57.165 * Valkey version=255.255.255, bits=64, commit=814e0f55, modified=1, pid=24881, just started
24881:C 21 Oct 2024 21:10:57.165 * Configuration loaded
24881:M 21 Oct 2024 21:10:57.167 * Increased maximum number of open files to 10032 (it was originally set to 1024).
```
After:
```text
68560:M 08 Nov 2024 16:10:12.257 * oO0OoO0OoO0Oo Valkey is starting oO0OoO0OoO0Oo
68560:M 08 Nov 2024 16:10:12.257 * Valkey version=255.255.255, bits=64, commit=45d596e1, modified=1, pid=68560, just started
68560:M 08 Nov 2024 16:10:12.257 * Configuration loaded
68560:M 08 Nov 2024 16:10:12.258 * monotonic clock: POSIX clock_gettime
```

Signed-off-by: azuredream <zhaozixuan67@gmail.com>
enjoy-binbin added a commit to enjoy-binbin/valkey that referenced this pull request Nov 13, 2024
In valkey-io#1282, we init server.pid earlier to keep log message role
consistent, but we forgot to consider daemonize. In daemonize
mode, we will always print the child role.

We need to reset server.pid after daemonize(), otherwise the
log printing role will always be the child.

Signed-off-by: Binbin <binloveplay1314@qq.com>
enjoy-binbin added a commit that referenced this pull request Nov 14, 2024
In #1282, we init server.pid earlier to keep log message role
consistent, but we forgot to consider daemonize. In daemonize
mode, we will always print the child role.

We need to reset server.pid after daemonize(), otherwise the
log printing role will always be the child. It also causes a
incorrect server.pid value, affecting the concatenation of
some pid names.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes This issue should get a line item in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log as primary role (M) instead of child process (C) during startup
3 participants