GH-10137: Cache session.list result in the AbstractInboundFileSynchronizer
#10344
+193
−123
Conversation
This file contains hidden or 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
…InboundFileSynchronizer` Fixes: spring-projects#10137 When `maxFetchSize > 0`, the `AbstractInboundFileSynchronizer` performs unnecessary `Session.list()` every time. Then we go over all those files to filter. And only after filtering we apply the `maxFetchSize`. * Add a cache for remote directory entries in the `AbstractInboundFileSynchronizer` * Guard cache interaction with a `DefaultLockRegistry` per directory * Rework `AbstractInboundFileSynchronizer.transferFilesFromRemoteToLocal()` to filter all the entries of the `Session.list()` on a first call. If `maxFetchSize > 0`, cache the remaining remote files
artembilan
changed the title
session.list result in the `AbstractInboundFileSync…session.list result in the AbstractInboundFileSynchronizer
cppwfs
requested changes
Aug 24, 2025
|
|
||
| @Override | ||
| protected boolean removeEldestEntry(Map.Entry<String, List<F>> eldest) { | ||
| return size() > 100; |
There was a problem hiding this comment.
Do we want the 100 to be configurable?
| @SuppressWarnings("unchecked") | ||
| Map<String, List<String>> fetchCache = TestUtils.getPropertyValue(sync, "fetchCache", Map.class); | ||
| List<String> cachedFiles = fetchCache.get("testRemoteDirectory"); | ||
| assertThat(cachedFiles).containsExactly("bar", "baz"); |
There was a problem hiding this comment.
Since we are replacing foo. We may go ahead and replace bar and baz.
| Starting with version 7.0, the `AbstractInboundFileSynchronizer` caches a filtered `Session.list(remoteDirectory)` after applying a `maxFetchSize` slicing. | ||
| The logic of the `AbstractInboundFileSynchronizer.transferFilesFromRemoteToLocal()` method is the following: | ||
|
|
||
| - If `maxFetchSize > 0`, the lock is acquired against `remoteDirecy` to avoid race condition from different threads, when work is done around cache. |
There was a problem hiding this comment.
remoteDirectory is misspelled.
|
|
||
| - If `maxFetchSize > 0`, the lock is acquired against `remoteDirecy` to avoid race condition from different threads, when work is done around cache. | ||
| The performance degradation is minimal since all the later synchronizations deal only with in-memory cached leftover; | ||
| - If no cache entry for the `remoteDirecy`, the `Session.list(remoteDirectory)` is called and all returned remote files are filtered; |
| - the filtered result then sliced by the `maxFetchSize`; | ||
| - then these file entries are being transferred to the local directory; | ||
| - the rest of filtered remote files are cached for later synchronizations; | ||
| - if there is a cache entry for the `remoteDirecy`, such a list is sliced `maxFetchSize` and iterated for the transfer to the local directory; |
There was a problem hiding this comment.
same here.
"such a list is sliced maxFetchSize" could be rewritten as " such a list is sliced to the maxFetchSize".
| - then these file entries are being transferred to the local directory; | ||
| - the rest of filtered remote files are cached for later synchronizations; | ||
| - if there is a cache entry for the `remoteDirecy`, such a list is sliced `maxFetchSize` and iterated for the transfer to the local directory; | ||
| - if one of the transfers fails, the `filter` is reset from this remote file to the end of the filtered list. |
There was a problem hiding this comment.
"if one of the transfers fails, the filter is reset from this remote file "
can be rewritten as
"if one of the transfers fails, the filter is reset from from the failed remote file".
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: #10137
When
maxFetchSize > 0, theAbstractInboundFileSynchronizerperforms unnecessarySession.list()every time.Then we go over all those files to filter.
And only after filtering we apply the
maxFetchSize.AbstractInboundFileSynchronizerDefaultLockRegistryper directoryAbstractInboundFileSynchronizer.transferFilesFromRemoteToLocal()to filter all the entries of theSession.list()on a first call. IfmaxFetchSize > 0, cache the remaining remote files