Skip to content

Commit

Permalink
Fix bugs in sftp retrying
Browse files Browse the repository at this point in the history
* Fixed a bug caused by nil sftp client during retry
* Simplify the rework logic in UploadFile
* Change the number of tries from 6 to 8
  • Loading branch information
gilbertchen committed Jan 13, 2020
1 parent d43fe1a commit e888b6d
Showing 1 changed file with 14 additions and 34 deletions.
48 changes: 14 additions & 34 deletions src/duplicacy_sftpstorage.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func CreateSFTPStorage(server string, port int, username string, storageDir stri
storageDir: storageDir,
minimumNesting: minimumNesting,
numberOfThreads: threads,
numberOfTries: 6,
numberOfTries: 8,
serverAddress: serverAddress,
sftpConfig: sftpConfig,
}
Expand Down Expand Up @@ -129,22 +129,19 @@ func (storage *SFTPStorage) retry(f func () error) error {
delay *= 2

storage.clientLock.Lock()
if storage.client != nil {
storage.client.Close()
storage.client = nil
}

connection, err := ssh.Dial("tcp", storage.serverAddress, storage.sftpConfig)
if err != nil {
LOG_WARN("SFT_RECONNECT", "Failed to connect to %s: %v; retrying", storage.serverAddress, err)
storage.clientLock.Unlock()
return err
continue
}

client, err := sftp.NewClient(connection)
if err != nil {
LOG_WARN("SFT_RECONNECT", "Failed to create a new SFTP client to %s: %v; retrying", storage.serverAddress, err)
connection.Close()
storage.clientLock.Unlock()
return err
continue
}
storage.client = client
storage.clientLock.Unlock()
Expand Down Expand Up @@ -275,36 +272,19 @@ func (storage *SFTPStorage) UploadFile(threadIndex int, filePath string, content
fullPath := path.Join(storage.storageDir, filePath)

dirs := strings.Split(filePath, "/")
if len(dirs) > 1 {
fullDir := path.Dir(fullPath)
err = storage.retry(func() error {
_, err := storage.getSFTPClient().Stat(fullDir)
return err
})
if err != nil {
// The error may be caused by a non-existent fullDir, or a broken connection. In either case,
// we just assume it is the former because there isn't a way to tell which is the case.
for i := range dirs[1 : len(dirs)-1] {
subDir := path.Join(storage.storageDir, path.Join(dirs[0:i+2]...))
// We don't check the error; just keep going blindly but always store the last err
err = storage.getSFTPClient().Mkdir(subDir)
}
fullDir := path.Dir(fullPath)
return storage.retry(func() error {

// If there is an error creating the dirs, we check fullDir one more time, because another thread
// may happen to create the same fullDir ahead of this thread
if err != nil {
err = storage.retry(func() error {
_, err := storage.getSFTPClient().Stat(fullDir)
return err
})
if err != nil {
return err
if len(dirs) > 1 {
_, err := storage.getSFTPClient().Stat(fullDir)
if os.IsNotExist(err) {
for i := range dirs[1 : len(dirs)-1] {
subDir := path.Join(storage.storageDir, path.Join(dirs[0:i+2]...))
// We don't check the error; just keep going blindly
storage.getSFTPClient().Mkdir(subDir)
}
}
}
}

return storage.retry(func() error {

letters := "abcdefghijklmnopqrstuvwxyz"
suffix := make([]byte, 8)
Expand Down

1 comment on commit e888b6d

@gilbertchen
Copy link
Owner Author

Choose a reason for hiding this comment

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

This commit has been mentioned on Duplicacy Forum. There might be relevant details there:

https://forum.duplicacy.com/t/a-panic-while-backing-up-to-a-sftp-storage/3066/2

Please sign in to comment.