-
Notifications
You must be signed in to change notification settings - Fork 74
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
Fixes #19218: backup file are not copied correctly when the destination directory is on another FS #3616
Conversation
PR updated with a new commit |
@@ -855,28 +856,41 @@ class PolicyWriterServiceImpl( | |||
UIO.unit | |||
} else { | |||
for { | |||
mvOptions <- getMoveOptions(sortedFolder.head) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, you're right, we need to keep the atomic move if we are on the same FS. In all case, it will give information related to performance (the warning message).
_ <- newFolders.update( folder :: _ ) | ||
_ <- PolicyGenerationLoggerPure.trace(s"Backuping old policies from '${baseFolder}' to '${backupFolder}' ") | ||
_ <- backupNodeFolder(baseFolder, backupFolder) | ||
// _ <- newFolders.update( folder :: _ ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't add folder anymore here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rollback in error won't be done anymore, so either no rollback or update newFolders ref
@@ -314,6 +314,7 @@ rudder.community.port=5309 | |||
# CAUTION: For performance and consistency, it is necessary that the rudder.dir.backup is on the same | |||
# filesystem as /var/rudder/share/ , otherwise the policies change for nodes is non-atomic (move becomes | |||
# copy then delete), and can result to incomplete/partial policies being distributed to nodes. | |||
# Commant that option to disable policy backup. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment instead of commant
PR updated with a new commit |
…on directory is on another FS
PR rebased |
ab6ece7
to
e6ac344
Compare
This PR is not mergeable to upper versions. |
OK, merging this PR |
https://issues.rudder.io/issues/19218