-
Notifications
You must be signed in to change notification settings - Fork 52
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
Client panics when initializing on a read only filesystem #2387
Comments
@irwinp-p Hi, thanks for bringing it up. The default behavior still will be to log errors to a file, and in any case of different need it can be configured different. I'd leave the issue open so in the future we will add special handling for read-only envs, but since there will be a way to mitigate it, the specific handling will have a bit lower priority. Please explain the other issue you see possible. |
It's possible the fix isn't actually sufficient. The client seems to initialize a log appender always, and then reconfigure it, causing the issue. A possible workaround for now is to change current directory to |
Calling RollingFileAppender::new tries to create the directory for the log files even though logging is turned off by Logger.init('off', null). This change simply postpones the initialization of RollingFileAppender until it is actually needed. This way if we do eg. Logger.init("info", null) it works as expected and does not throw an error on readonly filesystems. Fixes valkey-io#2387
Calling RollingFileAppender::new tries to create the directory for the log files even though logging is turned off by Logger.init('off', null). This change simply postpones the initialization of RollingFileAppender until it is actually needed. This way if we do eg. Logger.init("info", null) it works as expected and does not throw an error on readonly filesystems. Fixes valkey-io#2387
Setting the log level to OFF does not fix this issue because the log directory is created in RollingFileAppender::new call which is always done no matter the log level. I tried to come up with a simple non-breaking change to fix this in PR #2391 |
@avifenesh the other issue I alluded is the Unix Socket that's created in |
@avifenesh , does it justifies to release a patch version? if yes, lets release 1.1.1. |
This together with password rotation, i believe so, yes. |
Calling RollingFileAppender::new tries to create the directory for the log files even though logging is turned off by Logger.init('off', null). This change simply postpones the initialization of RollingFileAppender until it is actually needed. This way if we do eg. Logger.init("info", null) it works as expected and does not throw an error on readonly filesystems. Fixes valkey-io#2387
Calling RollingFileAppender::new tries to create the directory for the log files even though logging is turned off by Logger.init('off', null). This change simply postpones the initialization of RollingFileAppender until it is actually needed. This way if we do eg. Logger.init("info", null) it works as expected and does not throw an error on readonly filesystems. Fixes valkey-io#2387 Signed-off-by: Ville Nukarinen <[email protected]>
Calling RollingFileAppender::new tries to create the directory for the log files even though logging is turned off by Logger.init('off', null). This change simply postpones the initialization of RollingFileAppender until it is actually needed. This way if we do eg. Logger.init("info", null) it works as expected and does not throw an error on readonly filesystems. Fixes #2387 Signed-off-by: Ville Nukarinen <[email protected]>
Describe the bug
Client initialisation panics on read only filesystems. The failure originates from the rolling file appender in
logger_core
.A
RollingFilleAppender
is created even when file based logging is disabled by settingLogger.setLoggerConfig("warn")
Expected Behavior
When file based logging is disabled, the
RollingFileAppender
should not be used.Current Behavior
A panic occurs when calling
GlideClient.createClient
. Detailed error information below in theLogs
section.Reproduction Steps
Use docker in read-only mode to reproduce
4 build the docker image and run
Logs
sectionPossible Solution
Don't create a
RollingFilleAppender
when file based logging is disabled.Additional Information/Context
We plan to use Glide on AWS Fargate. All our services there run there with the following setting
readonlyRootFilesystem: true
Also, there may possibly be other issues in addition to the
RollingFileAppender
that prevents running on read only file systems, e.g. https://github.com/valkey-io/valkey-glide/blob/main/glide-core/src/socket_listener.rs#L793Client version used
1.1.0
Engine type and version
Valkey 7.2
OS
MacOS
Language
TypeScript
Language Version
node 20
Cluster information
No response
Logs
Other information
No response
The text was updated successfully, but these errors were encountered: