pkg / sftp

SFTP support for the go.crypto/ssh package
BSD 2-Clause "Simplified" License
1.52k stars 380 forks source link

"data":"sftp: \"Unable to create the file/directory. Reason:DESTINATION_ENTITY_EXISTS\" (SSH_FX_FAILURE)', #367

Closed sahanashivu-sixt closed 4 years ago

sahanashivu-sixt commented 4 years ago

Getting above error intermittently when trying to open the file using sftpClient.OpenFile(fileName, os.O_WRONLY|os.O_CREATE|os.O_TRUNC).

File already exists on the sftp server, for every 7minutes, we are creating new sftp client and write to the file, and close the client. However once in a week or so, we are getting above error.

Could not find much resource on this, hence raising this issue.

puellanivis commented 4 years ago

Which SFTP server? This honestly sounds like something that is happening on the server side, rather than something with the client.

sahanashivu-sixt commented 4 years ago

SFTP server details have been given by the google team. Please let me know if you need anymore details of server. Any idea when do we get this kind of error, what is the cause for it? Is there any problem with how we are opening the file or so?

puellanivis commented 4 years ago

I don’t think there is anything wrong with what you’re doing, or with our code. It’s probably some sort of race-condition with file creation, where two processes might be trying to make the same file at the same time, and one of them fails. :woman_shrugging:

It’s hard to tell, but the error string you’re seeing (Unable to create the file/directory. Reason:DESTINATION_ENTITY_EXISTS): is not from our package, and I also cannot seem to find any references to it from a Google search.

But my money is on some sort of race-condition on the server, where there’s something like if !fileExists() { fileCreate() } but being done without any locking, so maybe two test for file existence at the same time, and both get that it doesn’t exist, then both try to create it, and the second one to execute fails, because the file already exists.

sahanashivu-sixt commented 4 years ago

Yes, I didn't find this error string in the pkg too. Is there any way to avoid this race condition through the sftp client?

One more thing, file already exists on the server, so we are just trying overwrite the same using os.O_WRONLY|os.O_CREATE|os.O_TRUNC.

If I understood correctly, os.O_WRONLY|os.O_CREATE|os.O_TRUNC would create the file if doesnot exists and truncate the file if already exists.

puellanivis commented 4 years ago

Yes, that is the intended semantics. However, on the backend SFTP server, it can be unclear how exactly this is being performed. But as I noted, the simple solution of if fileExists() { fileCreate() } will not work, unless the server is holding a properly scoped lock to ensure that only one check of a file existing through to the creation and truncation itself. It should be equally unsurprising to everyone at all times that programmers write insufficiently safe multithreaded/multiprocess code. Even those of us that are good at it, are bound to make mistakes unless everything has been rigorously proven mathematically. (And even then, if you make a false assumption as a premise, then your proof goes out the window as well.)

If my presumption (guess) of what is going on is true, then really the only way to prevent this issue is to ensure that you are only ever doing one operation on this server at a time. That’s almost certainly not practicable. I would contact the responsible party that you are working with, and figure out what is causing this, and how you can prevent it from happening in the future.

There’s simply nothing this library/package can do to help you with a server that is breaking proper semantics.

sahanashivu-sixt commented 4 years ago

Thank you for the support @puellanivis, as there is nothing much we can do here from the library point of view, I have added retry logic in my code to upload the file again on any failure.

puellanivis commented 4 years ago

Closing as we cannot help any further.