Update link creation for use Java API instead of `ln` or `mklink` commands by melissalinkert · Pull Request #172 · ome/omero-blitz

melissalinkert · 2026-02-27T15:51:47Z

Imports with --transfer=ln or --transfer=ln_s may be a bit faster, but otherwise this should have no functional impact.

…mands

sbesson · 2026-03-20T17:42:24Z

Thanks for starting this overdue cleanup @melissalinkert. Since Java 8 is the minimal requirement (and soon Java 11), switch to java.nio.Files wherever appropriate seems very reasonably.

I'll start listing a set scenarios that should be evaluated functionally including error conditions. From an initial reading of the proposed code changes, a few high-level API questions:

there are three concrete subclasses of AbstractExecFileTransfer that implement the abstract createProcessBuilder API but this change only updates the hard-link and symlink transfers. Is there a reason not to update CopyFileTransfer to use java.nio.Files as well?
as the new implementation is not really returning a ProcessBuilder, would an alternative approach be to override the protected exec(String, String) API in each implementation?
related to the above, should we deprecate the ProcessBuilder APIs?

Deprecates `createProcessBuilder`, and uses `exec(File, File)` instead.

…lass

melissalinkert · 2026-03-20T20:50:46Z

Last few commits are one approach to the comments in #172 (comment). I don't have a good sense of the impact of wider refactoring/API changes here, so had originally stuck with the existing API to be safe.

sbesson

Using a synthetic test file, tested the main transfer types: ln, ln_s, cp and cp_rm with the omero import command. Under the appropriate filesystem requirements, confirmed that the import worked as expected.

Tested the error condition by using --transfer=ln with a file on a different filesystem

$ omero import --transfer=ln test.fake
...
2026-03-23 12:18:06,404 1950       [2-thread-1] INFO   ormats.importer.cli.LoggingImportMonitor - FILESET_UPLOAD_PREPARATION
2026-03-23 12:18:06,598 2144       [2-thread-1] INFO   ormats.importer.cli.LoggingImportMonitor - FILESET_UPLOAD_START
2026-03-23 12:18:06,617 2163       [3-thread-1] INFO   .importer.transfers.HardlinkFileTransfer - Transferring /mnt/data/seb/test.fake...
2026-03-23 12:18:06,657 2203       [3-thread-1] INFO   ormats.importer.cli.LoggingImportMonitor - FILE_UPLOAD_STARTED: /mnt/data/seb/test.fake
2026-03-23 12:18:06,672 2218       [3-thread-1] ERROR     ome.formats.importer.cli.ErrorHandler - FILE_EXCEPTION: /mnt/data/seb/test.fake
java.nio.file.FileSystemException: /OMERO/ManagedRepository/import.user_2/2026-03/23/12-18-06.472/test.fake -> /mnt/data/seb/test.fake: Invalid cross-device link
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:100)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixFileSystemProvider.createLink(UnixFileSystemProvider.java:481)
	at java.base/java.nio.file.Files.createLink(Files.java:1102)
	at ome.formats.importer.transfers.HardlinkFileTransfer.exec(HardlinkFileTransfer.java:44)
	at ome.formats.importer.transfers.AbstractExecFileTransfer.transfer(AbstractExecFileTransfer.java:65)
	at ome.formats.importer.ImportLibrary.uploadFile(ImportLibrary.java:531)
	at ome.formats.importer.ImportLibrary$3.call(ImportLibrary.java:634)
	at ome.formats.importer.ImportLibrary$3.call(ImportLibrary.java:631)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
2026-03-23 12:18:06,674 2220       [2-thread-1] ERROR        ome.formats.importer.ImportLibrary - Error on import
java.nio.file.FileSystemException: /OMERO/ManagedRepository/import.user_2/2026-03/23/12-18-06.472/test.fake -> /mnt/data/seb/test.fake: Invalid cross-device link
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:100)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixFileSystemProvider.createLink(UnixFileSystemProvider.java:481)
	at java.base/java.nio.file.Files.createLink(Files.java:1102)
	at ome.formats.importer.transfers.HardlinkFileTransfer.exec(HardlinkFileTransfer.java:44)
	at ome.formats.importer.transfers.AbstractExecFileTransfer.transfer(AbstractExecFileTransfer.java:65)
	at ome.formats.importer.ImportLibrary.uploadFile(ImportLibrary.java:531)
	at ome.formats.importer.ImportLibrary$3.call(ImportLibrary.java:634)
	at ome.formats.importer.ImportLibrary$3.call(ImportLibrary.java:631)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)
2026-03-23 12:18:06,675 2221       [2-thread-1] INFO         ome.formats.importer.ImportLibrary - Exiting on error

==> Summary
0 files uploaded, 0 filesets created, 0 images imported, 1 error in 0:00:00.277

Also tested functionally by using a real-world example HCS plate with 648 files. The fileset was imported 3 consecutive times using cp or ln_s transfer and omero-blitz 5.8.4 or omero-blitz 5.8.5-SNAPSHOT. The first result was discarded to account for any filesystem caching.

	cp	ln_s
omero-blitz 5.8.4	33.29s +/- 0.17s	47.17s +/- 0.37s
omero-blitz 5.8.5-SNAPSHOT	33.20s +/- 0.04s	46.31s +/- 0.28s

A few proposed inline changes in the AbstractExecFileTransfer class to avoid this counting as a backwards incompatible change but otherwise this looks good, simplifies the code and the maintenance and causes no noticeable performance degradation.

In addition to the many files scenario, I'll run a last functional test with a large single file.

sbesson · 2026-03-23T12:22:03Z

-            throw new RuntimeException(msg);
-        }
-    }
+    protected abstract void exec(File file, File location) throws IOException;


Although I think converting this into an abstract method is probably the right approach, this would have the impact of breaking any concrete implementation other than the ones shipped with omero-blitz.

To avoid calling this a backwards-compatible change, proposing to keep the current implementation in AbstractExecFileTransfer but override it in our subclasses and suggest other implementations should do so.

...with a comment that implementations should override this method.

sbesson

Rested using a 2G file (LuCa-7color_Scan1.ome.tiff) that was imported with the three primary local file transfer mechanisms as described above

	cp	ln	ln_s
omero-blitz 5.8.5	1.34s +/- 0.06s	0.15s +/- 0.01s	0.15s +/- 0.01s
omero-blitz 5.8.5-SNAPSHOT	1.70s +/- 0.06s	0.15s +/- 0.01s	0.15s +/- 0.01s

There is a slight performance degradation associated with using Files.copy. From experience, in most scenarios where a file can be directly copied from its original location into the binary repository, a linking strategy (soft or link) is usually preferred. Alternatively, under some conditions a ln_rm strategy will move the data under the binary repository.

From my perspective, the benefits of using the standard Java library vastly exceed this performance difference. This is amplified by the fact it only represents a small percentage of the total import time.

With the latest changes, the current AbstractExcecFileTransfer.exec implementations is unmodified, AbstractExcecFileTransfer.printLine which duplicated the superclass implementation is cleaned up and AbstractExcecFileTransfer.createProcessBuilder contains a default implementation and is deprecated.
The three local file transfer implementation are updated in a backwards compatible manner to use java.nio. One last comment about a Javadoc marker and this should be ready for integration in the next release of this component

sbesson · 2026-03-24T17:17:29Z

     * @param file File to be copied.
     * @param location Location to copy to.
-     * @throws IOException
+     * @param throws IOException


Should it be @throws?

melissalinkert · 2026-03-24T17:50:18Z

There is a slight performance degradation associated with using Files.copy. From experience, in most scenarios where a file can be directly copied from its original location into the binary repository, a linking strategy (soft or link) is usually preferred. Alternatively, under some conditions a ln_rm strategy will move the data under the binary repository.

From my perspective, the benefits of using the standard Java library vastly exceed this performance difference. This is amplified by the fact it only represents a small percentage of the total import time.

Would this be a problem for e.g. HCS data with thousands of files though?

sbesson · 2026-03-24T18:03:34Z

Would this be a problem for e.g. HCS data with thousands of files though?

#172 (review) tested the HCS scenario with ~500 1-2MB files with no evidence of an overhead associated.
Happy to give it a last run using the latest version of these changes tomorrow. Would 1-10K files be a good order of magnitude for testing? Would it be valuable to evaluate a range of HCS layouts i.e. many small files vs less larger files?

melissalinkert · 2026-03-25T15:00:40Z

Ah, apologies, I missed that you had already tested that case. My concern was that if the difference of 0.36s in the cp case in #172 (review) ends up being 0.36s per file, then that would add up when dealing with something in the ~5-10k file range.

sbesson · 2026-03-25T15:42:27Z

My concern was that if the difference of 0.36s in the cp case in #172 (review) ends up being 0.36s per file, then that would add up when dealing with something in the ~5-10k file range.

Agreed. I haven't seen any example of filesets containing 5-10K 2G files but I'll look into something intermediate with multiple files in the 100-500MB range

kkoz

This is definitely a step in the right direction. I don't have any issues with the code as such, but it still feels awkward. Some of the existing structure implies there is still execution of command line functions, which there aren't. Should we open an issue to clean this up or is it not really worth it?

kkoz · 2026-03-25T19:15:24Z

I don't think this is true.

Updated in 16ca7dd.

kkoz · 2026-03-25T19:16:26Z

Also not true anymore.

Updated in 16ca7dd.

sbesson · 2026-03-26T09:51:57Z

Adding another real-world HCS example with the idr0093 CV7000 plate (14099 files, 146G) imported via symlink with one upload thread with/without this PR

	ln_s
omero-blitz 5.8.4	888s +/- 2s
omero-blitz 5.8.5-SNAPSHOT	873s +/- 1s

melissalinkert · 2026-03-26T14:56:13Z

This is definitely a step in the right direction. I don't have any issues with the code as such, but it still feels awkward. Some of the existing structure implies there is still execution of command line functions, which there aren't. Should we open an issue to clean this up or is it not really worth it?

I definitely agree that there is more cleanup that could be done, e.g. 5f12456. Since this PR is intended to be backwards-compatible, though, capturing as an issue makes sense.

sbesson

Ran a final import test using idr0056 (311 files, 23G total) and transfer=cp

	transfer=cp
5.8.4	45.54s
5.8.5-SNAPSHOT	43.23s

Overall, these results suggest that the proposed changes do not significantly impact the transfer / import times across a variety of input and transfer types.

Update link creation for use Java API instead of ln or mklink com…

2c9c1bf

…mands

melissalinkert requested a review from sbesson February 27, 2026 15:51

Add missing imports

ceddf5b

melissalinkert added 3 commits March 20, 2026 15:37

Rework file transfer logic to get rid of outdated ProcessBuilder usage

5f12456

Deprecates `createProcessBuilder`, and uses `exec(File, File)` instead.

Add implementation to deprecated createProcessBuilder

0dcd3f4

Remove printLine() that duplicates the implementation in the superc…

684f0dc

…lass

sbesson requested changes Mar 23, 2026

View reviewed changes

melissalinkert added 2 commits March 23, 2026 09:20

Remove unused ArrayList imports

0950d3b

Restore exec implementation in AbstractExecFileTransfer

e829266

...with a comment that implementations should override this method.

melissalinkert requested a review from sbesson March 23, 2026 14:29

kkoz self-requested a review March 24, 2026 13:16

sbesson reviewed Mar 24, 2026

View reviewed changes

Fix Javadoc

4d3185d

kkoz reviewed Mar 25, 2026

View reviewed changes

Correct AbstractExecFileTransfer Javadocs

16ca7dd

melissalinkert mentioned this pull request Mar 26, 2026

Clean up AbstractExecFileTransfer and its implementations #173

Open

sbesson approved these changes Mar 30, 2026

View reviewed changes

sbesson requested a review from kkoz April 21, 2026 13:37

kkoz approved these changes Apr 22, 2026

View reviewed changes

sbesson merged commit 9fd172d into ome:master Apr 22, 2026
8 checks passed

sbesson mentioned this pull request May 13, 2026

Add version history for OMERO.server 5.6.18 release ome/openmicroscopy#6453

Merged

Uh oh!

Conversation

melissalinkert commented Feb 27, 2026

Uh oh!

sbesson commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melissalinkert commented Mar 20, 2026

Uh oh!

sbesson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sbesson Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

sbesson left a comment

Choose a reason for hiding this comment

Uh oh!

sbesson Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

melissalinkert commented Mar 24, 2026

Uh oh!

sbesson commented Mar 24, 2026

Uh oh!

melissalinkert commented Mar 25, 2026

Uh oh!

sbesson commented Mar 25, 2026

Uh oh!

kkoz left a comment

Choose a reason for hiding this comment

Uh oh!

kkoz Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

melissalinkert Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

kkoz Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

melissalinkert Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

sbesson commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

melissalinkert commented Mar 26, 2026

Uh oh!

sbesson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sbesson commented Mar 20, 2026 •

edited

Loading

sbesson commented Mar 26, 2026 •

edited

Loading