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

create-cluster clean now will clean appendonlydir #10223

Merged
merged 3 commits into from
Feb 7, 2022

Conversation

enjoy-binbin
Copy link
Collaborator

@enjoy-binbin enjoy-binbin commented Feb 1, 2022

In #9788, now we stores all persistent append-only files in
a dedicated directory. The name of the directory is determined
by the appenddirname configuration parameter in redis.conf

Update create-cluster clean to clean this default directory.
Each node have a separate folder appendonlydir-{PORT}.
This PR also do some cleanups, logs and stricter wildcard matching.
Fixes #10222

In redis#9788, now we stores all persistent append-only files in
a dedicated directory. The name of the directory is determined
by the appenddirname configuration parameter in redis.conf

Update create-cluster clean to clean this default directory.
Fixes redis#10222
@oranagra oranagra added this to Backlog in 7.0 via automation Feb 1, 2022
@oranagra oranagra moved this from Backlog to Awaits merge in 7.0 Feb 1, 2022
Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

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

@chenyang8094 FYI use case of multiple nodes in one folder.

maybe instead we wanna let each node have a separate folder?
current code gives each one a different appendfilename.
not sure if some users of this script expect it to sometimes start from old persistence files (on upgrade)?

@enjoy-binbin
Copy link
Collaborator Author

maybe instead we wanna let each node have a separate folder?

yes, i also found this after submitting. i also do some cleanups(see if this needed)

@oranagra
Copy link
Member

oranagra commented Feb 1, 2022

@madolson @yossigo do you know if some users of this script expect it to sometimes start from old persistence files (on upgrade)?
if they are, we need to avoid changing the appenddirname..

@yossigo
Copy link
Member

yossigo commented Feb 1, 2022

@oranagra Maybe it's a kind of wishful thinking, but I tend to assume this script is only used for toy / test environments.

@chenyang8094
Copy link
Collaborator

chenyang8094 commented Feb 2, 2022

@oranagra Got, thank you.

@madolson
Copy link
Contributor

madolson commented Feb 7, 2022

AFAIK it's only used for test environments as well.

@enjoy-binbin enjoy-binbin added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label Feb 7, 2022
@oranagra oranagra merged commit c5e3d13 into redis:unstable Feb 7, 2022
@oranagra oranagra moved this from Awaits merge to Done in 7.0 Feb 7, 2022
@enjoy-binbin enjoy-binbin deleted the cluster_clean branch February 7, 2022 06:04
@oranagra oranagra moved this from Done to Unreleased in 7.0 Feb 14, 2022
@oranagra oranagra moved this from Unreleased to Done in 7.0 Mar 1, 2022
@aryehlev aryehlev mentioned this pull request May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten
Projects
Archived in project
7.0
Done
Development

Successfully merging this pull request may close these issues.

[BUG]create-cluster doesn't clean appendonlydir in redis 7.0
5 participants