Skip to content

Don't exit the process when locale-collate fails to be set from env during startup#3067

Merged
enjoy-binbin merged 2 commits into
valkey-io:unstablefrom
enjoy-binbin:locale-collate-exit
Jan 29, 2026
Merged

Don't exit the process when locale-collate fails to be set from env during startup#3067
enjoy-binbin merged 2 commits into
valkey-io:unstablefrom
enjoy-binbin:locale-collate-exit

Conversation

@enjoy-binbin

@enjoy-binbin enjoy-binbin commented Jan 16, 2026

Copy link
Copy Markdown
Member

Before ca6aead, even if setlocale fails, we
won't do anything.

setlocale(LC_COLLATE,"");

And now we will exit the process if the setlocale fails, this resulted in process
startup failures on some machines. (some kind of breaking change?)

if (setlocale(LC_COLLATE,server.locale_collate) == NULL) {
    serverLog(LL_WARNING, "Failed to configure LOCALE for invalid locale name.");
    exit(1);
}

In this commit, if we fail to set the locale_collate through environment variables,
we maintain backward compatibility and do not exit.

In a case where we did exit before, we don't exit now, seems safe. It's not a breaking
change and the behavior in this case was undocumented.

…uring startup

Before ca6aead, even if setlocale fails, we
won't do anything.
```
setlocale(LC_COLLATE,"");
```

And now we will exit the process if the setlocale fails, this resulted in process
startup failures on some machines. (some kind of breaking change?)
```
if (setlocale(LC_COLLATE,server.locale_collate) == NULL) {
    serverLog(LL_WARNING, "Failed to configure LOCALE for invalid locale name.");
    exit(1);
}
```

In this commit, if we fail to set the locale_collate through environment variables,
we maintain backward compatibility and do not exit.

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

codecov Bot commented Jan 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.34%. Comparing base (33a1b51) to head (c420f4d).
⚠️ Report is 38 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3067      +/-   ##
============================================
+ Coverage     74.16%   74.34%   +0.18%     
============================================
  Files           129      129              
  Lines         70988    71012      +24     
============================================
+ Hits          52649    52796     +147     
+ Misses        18339    18216     -123     
Files with missing lines Coverage Δ
src/server.c 89.46% <100.00%> (+<0.01%) ⬆️

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

@zuiderkwast zuiderkwast 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.

OK, but I don't think we need to mention in in valkey.conf.

Comment thread valkey.conf Outdated
Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin enjoy-binbin added the release-notes This issue should get a line item in the release notes label Jan 19, 2026
@enjoy-binbin

Copy link
Copy Markdown
Member Author

I am merging it, please feel free to leave a comment if i missed something.

@enjoy-binbin enjoy-binbin merged commit 8a9c371 into valkey-io:unstable Jan 29, 2026
57 checks passed
@enjoy-binbin enjoy-binbin deleted the locale-collate-exit branch January 29, 2026 02:35
harrylin98 pushed a commit to harrylin98/valkey_forked that referenced this pull request Feb 19, 2026
…uring startup (valkey-io#3067)

Before ca6aead, even if setlocale fails, we
won't do anything.
```
setlocale(LC_COLLATE,"");
```

And now we will exit the process if the setlocale fails, this resulted in process
startup failures on some machines. (some kind of breaking change?)
```
if (setlocale(LC_COLLATE,server.locale_collate) == NULL) {
    serverLog(LL_WARNING, "Failed to configure LOCALE for invalid locale name.");
    exit(1);
}
```

In this commit, if we fail to set the locale_collate through environment variables,
we maintain backward compatibility and do not exit.

In a case where we did exit before, we don't exit now, seems safe. It's not a breaking
change and the behavior in this case was undocumented.

Signed-off-by: Binbin <binloveplay1314@qq.com>
hpatro pushed a commit to hpatro/valkey that referenced this pull request Mar 5, 2026
…uring startup (valkey-io#3067)

Before ca6aead, even if setlocale fails, we
won't do anything.
```
setlocale(LC_COLLATE,"");
```

And now we will exit the process if the setlocale fails, this resulted in process
startup failures on some machines. (some kind of breaking change?)
```
if (setlocale(LC_COLLATE,server.locale_collate) == NULL) {
    serverLog(LL_WARNING, "Failed to configure LOCALE for invalid locale name.");
    exit(1);
}
```

In this commit, if we fail to set the locale_collate through environment variables,
we maintain backward compatibility and do not exit.

In a case where we did exit before, we don't exit now, seems safe. It's not a breaking
change and the behavior in this case was undocumented.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Harkrishn Patro <bunty.hari@gmail.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

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants