Skip to content

NIFI-15683 Fix SFTP Move strategy failure when destination file alrea…#11281

Open
rakesh-rsky wants to merge 1 commit into
apache:mainfrom
rakesh-rsky:fix/NIFI-15683-sftp-move-rename-fallback
Open

NIFI-15683 Fix SFTP Move strategy failure when destination file alrea…#11281
rakesh-rsky wants to merge 1 commit into
apache:mainfrom
rakesh-rsky:fix/NIFI-15683-sftp-move-rename-fallback

Conversation

@rakesh-rsky
Copy link
Copy Markdown
Contributor

NIFI-15683 Fix SFTP Move strategy failure when destination file already exists

When FetchSFTP uses completion strategy 'Move', some SFTP servers (notably OpenSSH) return SSH_FX_FAILURE on rename if the destination file already exists, rather than atomically overwriting it. This causes a warning and leaves the source file in place.

Add a delete-then-rename fallback: if the initial rename fails, attempt to delete the destination and retry the rename. If the destination does not exist (FileNotFoundException), the original rename exception is re-thrown to preserve the error signal for unrelated failures.

Summary

NIFI-15683

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
  • Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using ./mvnw clean install -P contrib-check
    • JDK 21
    • JDK 25

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

Copy link
Copy Markdown
Contributor

@exceptionfactory exceptionfactory left a comment

Choose a reason for hiding this comment

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

Thanks for proposing this change @rakesh-rsky. A similar effort recently stalled, attempting to handle failures related to moving files. There are several problems with potential solutions, including existing expectations, and lack of clear failure paths for post-fetch operations. For these reasons, falling back to deleting a file may not be appropriate. In addition, catching the general IOException is too broad to be a safe approach. This implementation needs further consideration of potential options, before moving forward.

…dy exists

When FetchSFTP uses completion strategy 'Move', some SFTP servers (notably
OpenSSH) return SSH_FX_FAILURE on rename if the destination file already
exists, rather than atomically overwriting it. This causes a warning and
leaves the source file in place.

Add a delete-then-rename fallback: if the initial rename fails, attempt to
delete the destination and retry the rename. If the destination does not
exist (FileNotFoundException), the original rename exception is re-thrown to
preserve the error signal for unrelated failures.
@rakesh-rsky rakesh-rsky force-pushed the fix/NIFI-15683-sftp-move-rename-fallback branch from 6a2d345 to 40ff482 Compare May 27, 2026 07:00
@rakesh-rsky
Copy link
Copy Markdown
Contributor Author

Thank you for the review feedback @exceptionfactory.

You're right on both points — the automatic delete-then-rename fallback was too aggressive, and catching the general IOException was too broad and could trigger a remote file deletion for unrelated failures like network errors or permission issues.

I've reworked the approach:

  • Added a Move Conflict Resolution property with two values: Fail (default, preserves existing behaviour) and Replace (explicit opt-in).
  • When Replace is selected, the processor proactively checks if the destination exists using getRemoteFileInfo() before attempting the rename. If it exists, it deletes it and renames. If not, it simply renames as normal.
  • This removes the broad IOException catch entirely — no exception-driven control flow, no silent destructive fallback.
  • The default is Fail, so existing deployments are unaffected.

This gives users explicit control over the conflict resolution behaviour while keeping the failure paths clean and predictable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants