GH-8562: Fix streaming source for remote calls #8564
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes spring-projects#8562 The `AbstractRemoteFileStreamingMessageSource.doReceive()` takes files first from a `toBeReceived` queue. When `AbstractRemoteFileStreamingMessageSource.remoteFileToMessage()` fails to fetch the file content because of interim connection issue, we reset this file from a filter and rethrow an exception. The next `receive()` call will just go ahead to the next entry in the `toBeReceived` queue, but the file we have just failed for will be retried only on the next list call to the remove directory. This essentially breaks a possible in-order target application logic. * Introduce `AbstractRemoteFileStreamingMessageSource.strictOrder` option to clear the `toBeReceived` queue when we fail in the `remoteFileToMessage()`, so the next `receive()` call would re-fetch files from remote dir, because the filter has been reset for those files. * Fix `AbstractFileInfo.toString()` to not perform remote calls when we just log this file. For example, we reset the file for connection failure and log the message about it, but it fails again because we request `size` of the file which may require a remote connection. **Cherry-pick to `6.0.x` & `5.5.x`**
garyrussell
suggested changes
Feb 28, 2023
| + ", RemoteDirectory=" + getRemoteDirectory() + ", Permissions=" + getPermissions() + "]"; | ||
| return "FileInfo [isLink=" + isLink() | ||
| + ", Filename=" + getFilename() | ||
| + ", RemoteDirectory=" + getRemoteDirectory() + "]"; |
There was a problem hiding this comment.
As discussed out of bounds; it would be better to leave this as-is and overload in any subclass that needs this change.
* Override `toString()` in `SmbFileInfo` instead - exactly the place where connection is used to obtain file attributes like `size` or `lastModified`
garyrussell
pushed a commit
that referenced
this pull request
Feb 28, 2023
* GH-8562: Fix streaming source for remote calls Fixes #8562 The `AbstractRemoteFileStreamingMessageSource.doReceive()` takes files first from a `toBeReceived` queue. When `AbstractRemoteFileStreamingMessageSource.remoteFileToMessage()` fails to fetch the file content because of interim connection issue, we reset this file from a filter and rethrow an exception. The next `receive()` call will just go ahead to the next entry in the `toBeReceived` queue, but the file we have just failed for will be retried only on the next list call to the remove directory. This essentially breaks a possible in-order target application logic. * Introduce `AbstractRemoteFileStreamingMessageSource.strictOrder` option to clear the `toBeReceived` queue when we fail in the `remoteFileToMessage()`, so the next `receive()` call would re-fetch files from remote dir, because the filter has been reset for those files. * Fix `AbstractFileInfo.toString()` to not perform remote calls when we just log this file. For example, we reset the file for connection failure and log the message about it, but it fails again because we request `size` of the file which may require a remote connection. **Cherry-pick to `6.0.x` & `5.5.x`** * * Revert `AbstractFileInfo` changes * Override `toString()` in `SmbFileInfo` instead - exactly the place where connection is used to obtain file attributes like `size` or `lastModified`
|
..and cherry-picked to 5.5.x as efab65b after resolving conflicts. |
garyrussell
pushed a commit
that referenced
this pull request
Feb 28, 2023
* GH-8562: Fix streaming source for remote calls Fixes #8562 The `AbstractRemoteFileStreamingMessageSource.doReceive()` takes files first from a `toBeReceived` queue. When `AbstractRemoteFileStreamingMessageSource.remoteFileToMessage()` fails to fetch the file content because of interim connection issue, we reset this file from a filter and rethrow an exception. The next `receive()` call will just go ahead to the next entry in the `toBeReceived` queue, but the file we have just failed for will be retried only on the next list call to the remove directory. This essentially breaks a possible in-order target application logic. * Introduce `AbstractRemoteFileStreamingMessageSource.strictOrder` option to clear the `toBeReceived` queue when we fail in the `remoteFileToMessage()`, so the next `receive()` call would re-fetch files from remote dir, because the filter has been reset for those files. * Fix `AbstractFileInfo.toString()` to not perform remote calls when we just log this file. For example, we reset the file for connection failure and log the message about it, but it fails again because we request `size` of the file which may require a remote connection. **Cherry-pick to `6.0.x` & `5.5.x`** * * Revert `AbstractFileInfo` changes * Override `toString()` in `SmbFileInfo` instead - exactly the place where connection is used to obtain file attributes like `size` or `lastModified`
|
...and to 6.0.x as d9f3821 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #8562
The
AbstractRemoteFileStreamingMessageSource.doReceive()takes files first from atoBeReceivedqueue. WhenAbstractRemoteFileStreamingMessageSource.remoteFileToMessage()fails to fetch the file content because of interim connection issue, we reset this file from a filter and rethrow an exception. The nextreceive()call will just go ahead to the next entry in thetoBeReceivedqueue, but the file we have just failed for will be retried only on the next list call to the remove directory. This essentially breaks a possible in-order target application logic.AbstractRemoteFileStreamingMessageSource.strictOrderoption to clear thetoBeReceivedqueue when we fail in theremoteFileToMessage(), so the nextreceive()call would re-fetch files from remote dir, because the filter has been reset for those files.AbstractFileInfo.toString()to not perform remote calls when we just log this file. For example, we reset the file for connection failure and log the message about it, but it fails again because we requestsizeof the file which may require a remote connection.Cherry-pick to
6.0.x&5.5.x