Fix premature session close causing large file download failures - #102
Open
sven1103-agent wants to merge 6 commits into
Open
Fix premature session close causing large file download failures#102sven1103-agent wants to merge 6 commits into
sven1103-agent wants to merge 6 commits into
Conversation
The openBIS session in OpenBisConnector.loadFile() was closed immediately via try-with-resources before the InputStream was consumed by the StreamingResponseBody. The stream is backed by an HTTP connection to the openBIS data store server that requires an active session, so closing the session prematurely caused 'Connection reset by peer' errors during large file downloads (failing after ~2 hours when the DSS session timeout of 2 hours expired). Changes: - Keep the session alive by wrapping the InputStream in a SessionAwareInputStream (already used by loadData()) that releases the session when the stream is closed - Distinguish client disconnects from server errors in the controller to avoid noisy ERROR logs for expected client cancellations - Use container-agnostic client abort detection (works with both Tomcat and Jetty)
- Log warnings when read operations take >5 seconds (backpressure indicator) - Log progress every 30 seconds with throughput metrics - Track total bytes transferred to correlate with failure point - Helps identify if 120GB failure is caused by backpressure stalling
Include file path and measurement ID in backpressure warnings and progress logs so they can be correlated with specific downloads in the server logs.
Steffengreiner
requested changes
Aug 25, 2026
Steffengreiner
left a comment
Steffengreiner
left a comment
There was a problem hiding this comment.
Heya @sven1103 and @sven1103-agent Nice work! I added some questions which i would be happy to learn more about. Good Idea with the session management 👍
Comment on lines
-111
to
+120
| return new DataFile(toFileInfo(dataSetFile), inputStream); | ||
| return new DataFile(toFileInfo(dataSetFile), new SessionAwareInputStream(inputStream, session)); |
There was a problem hiding this comment.
Stupid question maybe but shouldn't the session be also closed if no exception is thrown after the datafile is provided? Otherwise the session stays open and idle?
Comment on lines
+229
to
+231
| // Backpressure detection: if a read takes longer than this threshold, log a warning | ||
| long backpressureThresholdMs = 5000; // 5 seconds | ||
| long progressLogIntervalMs = 30000; // Log progress every 30 seconds |
There was a problem hiding this comment.
Would it make sense to move these into configurable variables outside of the application? Otherwise the logging interval can only be adjusted via a new version?
Comment on lines
+186
to
+191
| // Check for well-known client abort exception types by name to avoid | ||
| // hard dependencies on a specific servlet container (Tomcat vs Jetty). | ||
| String className = cause.getClass().getName(); | ||
| if (className.equals("org.apache.catalina.connector.ClientAbortException") | ||
| || className.equals("org.eclipse.jetty.io.EofException")) { | ||
| return true; |
There was a problem hiding this comment.
Since openbis is based on jetty do we really need to have a distinct check for apache as well here?
Measure read time (DSS → download server) and write time (download server → client) separately to distinguish upstream vs downstream backpressure: - Upstream backpressure: DSS slow to send data (disk I/O, network to DSS) - Downstream backpressure: client slow to consume data (slow network, disk write) Also track cumulative stats and log a final summary at transfer completion for post-mortem analysis.
…tern The ss output from the DSS showed Recv-Q: 3,878,713 and rwnd_limited: 99.6% before the connection died. This proves the download server was the bottleneck: when the client write blocked (slow client), the DSS read also blocked, causing the TCP receive window to close and the DSS to eventually reset the connection. The writeRange() method now uses a dedicated producer thread that reads from the DSS into a bounded ArrayBlockingQueue (16 buffers), while the consumer (calling thread) reads from the queue and writes to the client. This decoupling ensures DSS reads continue even when the client write is temporarily blocked. The bounded queue provides natural backpressure: when the queue is full, the producer blocks, which closes the TCP receive window and signals the DSS to slow down gracefully. Only affects MeasurementFileController (single file download endpoint). The deprecated MeasurementZipDownloadController is left untouched.
The queue capacity was hardcoded to 16 buffers (16MB at default 1MB buffer size). For large file downloads this only absorbs ~0.8s of client stall at 20MB/s throughput, which is too tight. Now configurable via server.download.queue.capacity (env: DOWNLOAD_QUEUE_CAPACITY) with a default of 64 buffers (64MB). At 20MB/s this absorbs ~3.2s of client stall, giving the DSS connection much more resilience against temporary client slowdowns. Total memory per concurrent download = queue capacity × buffer size. With 10 concurrent downloads at defaults: 640MB total.
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Problem
When downloading very large files (>120GB), the download fails with:
Investigation with
ss -tinon the DSS showed the download server's TCP receive buffer filling up (Recv-Q: 3,878,713) andrwnd_limited: 99.6%before the connection died. The download server was the bottleneck.Root Cause
Two issues:
1. Premature session close
OpenBisConnector.loadFile()closed the openBIS session immediately viatry-with-resourcesbefore theInputStreamwas consumed. The session token was invalidated on the Application Server, and the DSS eventually detected this and terminated the connection.2. Synchronous read-write loop (backpressure deadlock)
The
writeRange()method used a synchronous loop: read from DSS → write to client → read from DSS → write to client. When the client write blocked (slow client), the DSS read also blocked. This caused the DSS TCP receive window to close, and after enough backpressure the DSS reset the connection.Fix
1. Keep session alive (
OpenBisConnector.loadFile())Wrap the
InputStreamin aSessionAwareInputStream(already used byloadData()) that releases the session when the stream is closed by the caller.2. Async producer-consumer pattern (
MeasurementFileController.writeRange())Replace the synchronous read-write loop with a decoupled producer-consumer pattern:
ArrayBlockingQueue(16 buffers)This ensures DSS reads continue even when the client write is temporarily blocked.
3. Improved error handling
Files Changed
openbis-connector/.../OpenBisConnector.java— session lifecycle fixrest-api/.../MeasurementFileController.java— async write + error handlingNot changed:
MeasurementZipDownloadController.java(deprecated zip download endpoint) — left untouched to avoid impacting production zip downloads.