Skip to content

create-cluster clean now will clean appendonlydir#10223

Merged
oranagra merged 3 commits intoredis:unstablefrom
enjoy-binbin:cluster_clean
Feb 7, 2022
Merged

create-cluster clean now will clean appendonlydir#10223
oranagra merged 3 commits intoredis:unstablefrom
enjoy-binbin:cluster_clean

Conversation

@enjoy-binbin
Copy link
Contributor

@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
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
Contributor 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 oranagra requested a review from madolson February 1, 2022 13:50
@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
Collaborator

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
Contributor

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
@enjoy-binbin enjoy-binbin deleted the cluster_clean branch February 7, 2022 06:04
@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

Development

Successfully merging this pull request may close these issues.

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

5 participants