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

Retry rename on lock induced failures (#16435) #16439

Merged
merged 1 commit into from
Jul 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cmd/embedded.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/modules/public"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/templates"
"code.gitea.io/gitea/modules/util"

"github.com/gobwas/glob"
"github.com/urfave/cli"
Expand Down Expand Up @@ -271,7 +272,7 @@ func extractAsset(d string, a asset, overwrite, rename bool) error {
} else if !fi.Mode().IsRegular() {
return fmt.Errorf("%s already exists, but it's not a regular file", dest)
} else if rename {
if err := os.Rename(dest, dest+".bak"); err != nil {
if err := util.Rename(dest, dest+".bak"); err != nil {
return fmt.Errorf("Error creating backup for %s: %v", dest, err)
}
// Attempt to respect file permissions mask (even if user:group will be set anew)
Expand Down
4 changes: 2 additions & 2 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -1215,7 +1215,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err
}

newRepoPath := RepoPath(repo.Owner.Name, newRepoName)
if err = os.Rename(repo.RepoPath(), newRepoPath); err != nil {
if err = util.Rename(repo.RepoPath(), newRepoPath); err != nil {
return fmt.Errorf("rename repository directory: %v", err)
}

Expand All @@ -1226,7 +1226,7 @@ func ChangeRepositoryName(doer *User, repo *Repository, newRepoName string) (err
return err
}
if isExist {
if err = os.Rename(wikiPath, WikiPath(repo.Owner.Name, newRepoName)); err != nil {
if err = util.Rename(wikiPath, WikiPath(repo.Owner.Name, newRepoName)); err != nil {
return fmt.Errorf("rename repository wiki: %v", err)
}
}
Expand Down
8 changes: 4 additions & 4 deletions models/repo_transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,13 +210,13 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
}

if repoRenamed {
if err := os.Rename(RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name)); err != nil {
if err := util.Rename(RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name)); err != nil {
log.Critical("Unable to move repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, RepoPath(newOwnerName, repo.Name), RepoPath(oldOwnerName, repo.Name), err)
}
}

if wikiRenamed {
if err := os.Rename(WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name)); err != nil {
if err := util.Rename(WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name)); err != nil {
log.Critical("Unable to move wiki for repository %s/%s directory from %s back to correct place %s: %v", oldOwnerName, repo.Name, WikiPath(newOwnerName, repo.Name), WikiPath(oldOwnerName, repo.Name), err)
}
}
Expand Down Expand Up @@ -358,7 +358,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
return fmt.Errorf("Failed to create dir %s: %v", dir, err)
}

if err := os.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
if err := util.Rename(RepoPath(oldOwner.Name, repo.Name), RepoPath(newOwner.Name, repo.Name)); err != nil {
return fmt.Errorf("rename repository directory: %v", err)
}
repoRenamed = true
Expand All @@ -370,7 +370,7 @@ func TransferOwnership(doer *User, newOwnerName string, repo *Repository) (err e
log.Error("Unable to check if %s exists. Error: %v", wikiPath, err)
return err
} else if isExist {
if err := os.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil {
if err := util.Rename(wikiPath, WikiPath(newOwner.Name, repo.Name)); err != nil {
return fmt.Errorf("rename repository wiki: %v", err)
}
wikiRenamed = true
Expand Down
4 changes: 2 additions & 2 deletions models/ssh_key.go
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ func rewriteAllPublicKeys(e Engine) error {
}

t.Close()
return os.Rename(tmpPath, fPath)
return util.Rename(tmpPath, fPath)
}

// RegeneratePublicKeys regenerates the authorized_keys file
Expand Down Expand Up @@ -1316,7 +1316,7 @@ func rewriteAllPrincipalKeys(e Engine) error {
}

t.Close()
return os.Rename(tmpPath, fPath)
return util.Rename(tmpPath, fPath)
}

// ListPrincipalKeys returns a list of principals belongs to given user.
Expand Down
4 changes: 2 additions & 2 deletions models/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ func ChangeUserName(u *User, newUserName string) (err error) {
}

// Do not fail if directory does not exist
if err = os.Rename(UserPath(oldUserName), UserPath(newUserName)); err != nil && !os.IsNotExist(err) {
if err = util.Rename(UserPath(oldUserName), UserPath(newUserName)); err != nil && !os.IsNotExist(err) {
return fmt.Errorf("Rename user directory: %v", err)
}

Expand All @@ -1020,7 +1020,7 @@ func ChangeUserName(u *User, newUserName string) (err error) {
}

if err = sess.Commit(); err != nil {
if err2 := os.Rename(UserPath(newUserName), UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) {
if err2 := util.Rename(UserPath(newUserName), UserPath(oldUserName)); err2 != nil && !os.IsNotExist(err2) {
log.Critical("Unable to rollback directory change during failed username change from: %s to: %s. DB Error: %v. Filesystem Error: %v", oldUserName, newUserName, err, err2)
return fmt.Errorf("failed to rollback directory change during failed username change from: %s to: %s. DB Error: %w. Filesystem Error: %v", oldUserName, newUserName, err, err2)
}
Expand Down
2 changes: 1 addition & 1 deletion modules/log/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (log *FileLogger) DoRotate() error {

// close fd before rename
// Rename the file to its newfound home
if err = os.Rename(log.Filename, fname); err != nil {
if err = util.Rename(log.Filename, fname); err != nil {
return fmt.Errorf("Rotate: %v", err)
}

Expand Down
2 changes: 1 addition & 1 deletion modules/storage/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (l *LocalStorage) Save(path string, r io.Reader, size int64) (int64, error)
return 0, err
}

if err := os.Rename(tmp.Name(), p); err != nil {
if err := util.Rename(tmp.Name(), p); err != nil {
return 0, err
}

Expand Down
29 changes: 28 additions & 1 deletion modules/util/remove.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func Remove(name string) error {
return err
}

// RemoveAll removes the named file or (empty) directory with at most 5 attempts.Remove
// RemoveAll removes the named file or (empty) directory with at most 5 attempts.
func RemoveAll(name string) error {
var err error
for i := 0; i < 5; i++ {
Expand All @@ -55,3 +55,30 @@ func RemoveAll(name string) error {
}
return err
}

// Rename renames (moves) oldpath to newpath with at most 5 attempts.
func Rename(oldpath, newpath string) error {
var err error
for i := 0; i < 5; i++ {
err = os.Rename(oldpath, newpath)
if err == nil {
break
}
unwrapped := err.(*os.PathError).Err
Copy link
Contributor Author

@zeripath zeripath Jul 16, 2021

Choose a reason for hiding this comment

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

this needs to be:

Suggested change
unwrapped := err.(*os.PathError).Err
unwrapped := err.(*os.LinkError).Err

if unwrapped == syscall.EBUSY || unwrapped == syscall.ENOTEMPTY || unwrapped == syscall.EPERM || unwrapped == syscall.EMFILE || unwrapped == syscall.ENFILE {
// try again
<-time.After(100 * time.Millisecond)
continue
}

if i == 0 && os.IsNotExist(err) {
return err
}

if unwrapped == syscall.ENOENT {
// it's already gone
return nil
}
}
return err
}