smart_open: Ranged read http header leaves TO bytes blank, causing S3 filesystems to read full file length

After sleeping on this here’s the solution I propose implementing (I’m working towards this and a pull request).

We have two use cases that are at odds here:

Use case 1: The file is opened for streaming and many read calls will be performed. This is common if you open a file and pass it to many underlying libraries which were optimized for local IO access. With many small reads of the file, repeating the HTTP headers would be an unacceptable overhead, it would also translate into an explosion of IOPS, which would not be good for the S3 filesystem either.

Use case 2: A single small ranged read of a large file. This use case is common in many applications such as machine learning, where a large binary data file contains many small samples. The file is opened, read once, and closed, often at a very high frequency. For example, my use case is to read up to 20 samples/s of 25 Mb segments of a multi-GB file, up to 500 MB/s, to feed a single GPU, and I often run 10’s of GPUs. In this use case, the overhead of an unspecified byte range is a showstopper because it doesn’t allow the S3 filesystem to optimize the internal queuing of the file.

These two use cases are fundamentally at odds, so any solution is a balancing act between them. I propose we remove defer_seek from transport_params, it’s unnecessary (see comment below), and add a transport_params called optimize_reads which takes on one of three user-specified values: auto (default), optimize_for_streaming, and optimize_for_partial_reads.

Description of these modes:

• optimize_for_streaming: operates like smart_open does now, the byte-range header is left unbounded and multiple reads pull data from the open body using the current method of buffer-size | read-size using one open HTTP request.
• optimize_for_partial_reads: closes the body after each read and specifies the byte-range header optimally for each read request. A new HTTP connection + headers is sent for each read call.
• auto: the default balance between these two modes. For the first read call optimize_for_partial_reads is used. This way the byte range is set appropriately for the single-read ML-style use case and the S3 filesystem is protected. For the second and subsequent calls optmize_for_streaming is then used, this satisfies the first use case where many small read calls are performed, and it only costs one extra connection + set of HTTP headers, which is a minimal cost to automatically satisfy the partial reads use case.

Tasks necessary to implement this (assuming no one raises a concern here then I’m working on a pull request with these changes):

1. seek will not open the body, it will only update the position.
2. Remove defer_seek (deprecate and make it a noop), it’s unnecessary when seek doesn’t open the body. There are 14 points in s3.py where self._content_length is referenced. Those 14 points need to all support a potential None value. This is not unreasonable.
3. Add optimize_reads to transport_params as discussed above.
4. Add a read call counter to Reader / _SeekableRawReader to support the auto option described above.
5. Add a conditional step in Reader / _SeekableRawReader that closes the body when optimize_for_partial_reads == True or (auto == True and read_call_counter > 1).

I’ve been looking through the smart_open code today to find a solution to this problem. I notice that the range is set by default in Reader.__init__() where a seek(0) is run. This seek doesn’t have the to-bytes information so the range header is set to bytes=from- at this point. There is an option in transport_params to defer_seek=True which skips this seek.

If you skip that first seek, the first call to read() will end up setting the same range:

class _SeekableRawReader(object):
    # ...
    def read(self, size=-1):
        """Read from the continuous connection with the remote peer."""
        if self._body is None:
            # This is necessary for the very first read() after __init__().
            self._open_body()

self._open_body() is called without parameters, which has the same effect of setting bytes=from-.

I guess the most obvious solution then is to set the to-bytes in the call to self._open_body() and to use the defer_seek transport param. I’ve tested that this works and solved the problem in SeaweedFS.

However, I question why defer_seek defaults to False, or why it’s there at all if the seek operation is going to effectively crash at least 2 major S3 filesystems (Ceph and SeaweedFS tested). So I’d still like to discuss how best to proceed. I’m happy to put in a pull request for this.

Yes, that makes sense.

1. Seeking should not close the body when the seek position is within the buffer
2. I think we can remove it, and provide documentation for people who may have been using that parameter.
3. 10MB default makes sense. Maybe call it download_bufsize?
4. Close the old body and open a new body to satisfy the read request, right?
5. Yes

Sounds like a fair bit of work, have fun 😉

So I only succeeded in solving the problem for a single read less than the buffer size (~132k by default). There’s a more fundamental issue at play.

smart_open is opening the file for streaming which explicitly does NOT generate multiple HTTP requests. Instead multiple calls to f.read(...) keep pulling data from the StreamingBody on the same open HTTP request. This has the obvious benefit of reducing network overhead (avoiding repeat HTTP headers). However, in that scenaro there is no way for smart_open to set an appropriate range for the file at the time it’s opened without the user explicitly restricting the file to a specific range. This has the negative side effect of forcing the filesystem to queue up the full file (SeaweedFS/S3) or a large chunk of it (Ceph/S3).

So we’re damned if we do and damned if we don’t.

One solution is to give the user the ability to specify a range in the open request. If they fail to do this then we fall back on multiple HTTP requests which can set the range correctly.

Another solution from the filesystem would be for the filesystem to only queue up what it needs as bytes are read. While we might be able to get one filesystem to implement this, ensuring all S3 filesystems operate that way is probably wishful thinking.

I can’t think of another way to handle this situation in a way that both protects the filesystem and keeps network overhead to a minimum. Can anyone else?