Optimize latency by removing HEAD checks #86
Comments
You're probably right that we won't care in many cases, but the current design behaves differently if a block doesn't exist (return null) vs fails to read an existing block (throw an error). I think that optimizing in the way you suggest would prevent treating these cases differently (I don't think we do much different right now) Note for later - Looks like these are logging block reads, so this is probably fundamentally coming from this code // HEAD
if (!getKeyValueAccess().isFile(path))
return null;
// GET
try (final LockedChannel lockedChannel = getKeyValueAccess().lockForReading(path)) {
return DefaultBlockReader.readBlock(lockedChannel.newInputStream(), datasetAttributes, gridPosition);
} catch (final IOException | UncheckedIOException e) {
throw new N5IOException( "Failed to read block " + Arrays.toString(gridPosition) + " from dataset " + path, e);
} |
|
If it fails to read a block then the API should not return a 404, but a 500 or something else. The code you showed currently does a blanket check for any IOException but it could be modified to throw a FileNotFoundException in cases of 404, and that could be caught before the IOException. I think you would get the same behavior with half the round trips. By the way, I came across another issue which happens if the API incorrectly returns a 200 for HEAD but then a 404 for the GET. In that case, Fiji enters a very tight infinite retry loop which freezes the application and requires a force quit. This should never happen with a correctly-behaving S3 API. But maybe this is what the HEAD is guarding against? My proxy log shows the retry loop: |
|
related: saalfeldlab/n5#124 |
|
This should behave better now when using the releases:
When you have a moment, could you repeat the investigation you did above to confirm? Thanks to @cmhulbert who did all the work. 🙏 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I noticed that the N5 Viewer seems to always send a HEAD before a GET. This is inefficient and may be unnecessary, since in most cases you can just check for the 404 when you do a GET. If this could be optimized away it would improve latency, while also reducing load on data servers.
Access log on my proxy server:
The text was updated successfully, but these errors were encountered: