Skip to content

create-cluster clean now will clean appendonlydir#10223

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

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

Conversation

@enjoy-binbin

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

Copy link
Copy Markdown
Contributor

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 left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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

oranagra commented Feb 1, 2022

Copy link
Copy Markdown
Member

@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

yossigo commented Feb 1, 2022

Copy link
Copy Markdown
Collaborator

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

@chenyang8094

chenyang8094 commented Feb 2, 2022

Copy link
Copy Markdown
Contributor

@oranagra Got, thank you.

@madolson

madolson commented Feb 7, 2022

Copy link
Copy Markdown
Contributor

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