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

dont try set maxmemory_policy if setting does not exist #3888

Merged
merged 1 commit into from
Jul 13, 2023
Merged

dont try set maxmemory_policy if setting does not exist #3888

merged 1 commit into from
Jul 13, 2023

Conversation

p1u3o
Copy link
Contributor

@p1u3o p1u3o commented Jul 6, 2023

Apache KVRocks supports the needed Redis commands (see https://kvrocks.apache.org/docs/supported-commands) to be a backend for JuiceFS, however because it uses RocksDB, there is no max_memory policy, but JuiceFS will try and set this, resulting in an non-critical error at startup.

@CLAassistant
Copy link

CLAassistant commented Jul 6, 2023

CLA assistant check
All committers have signed the CLA.

@zhijian-pro
Copy link
Contributor

zhijian-pro commented Jul 7, 2023

Replacing

if rInfo.storageProvider == "" && rInfo.maxMemoryPolicy != "noeviction" {

with

if rInfo.storageProvider == "" && rInfo.maxMemoryPolicy != "" && rInfo.maxMemoryPolicy != "noeviction" {

is logically simpler

@p1u3o
Copy link
Contributor Author

p1u3o commented Jul 7, 2023

I agree, I'll change this MR.

@p1u3o p1u3o changed the title dont try set maxmemory_policy if kvrocks dont try set maxmemory_policy if setting does not exist Jul 13, 2023
@p1u3o
Copy link
Contributor Author

p1u3o commented Jul 13, 2023

I have updated this MR with the suggested change and confirmed it works at silencing the error.

@zhijian-pro zhijian-pro merged commit edb40b8 into juicedata:main Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants