Skip to content

Commit

Permalink
fix: due to buffering in the underlying stream reads are limited in size
Browse files Browse the repository at this point in the history
Currently if you call `Read` and provide a buffer larger than a few k the stream returns only that few k, given the reader abstraction discards the stream between `Read` operations this results in many API calls.

To mitigate this I use `io.ReadFull` to ensure the full buffer is read in one hit, and avoid numerous API calls.

This PR adds tests that illustrate the issue.
  • Loading branch information
wolfeidau committed Jan 9, 2024
1 parent 956f3c7 commit a2fa574
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 3 deletions.
89 changes: 89 additions & 0 deletions integration/s3fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,17 @@ import (
)

var (
oneMegabyte = 1024 * 1024
twoMegabytes = 1024 * 1024 * 2
threeMegabytes = 1024 * 1024 * 3

oneKilobyte = bytes.Repeat([]byte("a"), 1024)
)

func generateData(len int) []byte {
return bytes.Repeat([]byte("a"), len)
}

func TestList(t *testing.T) {
assert := require.New(t)

Expand Down Expand Up @@ -131,3 +139,84 @@ func TestReaderAt(t *testing.T) {
assert.NoError(err)
assert.Equal(0, n)
}

func TestReaderAtBig(t *testing.T) {
assert := require.New(t)

_, err := client.PutObject(context.Background(), &s3.PutObjectInput{
Bucket: aws.String(testBucketName),
Key: aws.String("test_reader_at_big.txt"),
Body: bytes.NewReader(generateData(threeMegabytes)),
})
assert.NoError(err)

s3fs := s3iofs.NewWithClient(testBucketName, client)

f, err := s3fs.Open("test_reader_at_big.txt")
assert.NoError(err)

defer f.Close()

rdr, ok := f.(io.ReaderAt)
assert.True(ok)

n, err := rdr.ReadAt(make([]byte, oneMegabyte), 0)
assert.NoError(err)
assert.Equal(oneMegabyte, n)

n, err = rdr.ReadAt(make([]byte, twoMegabytes), 0)
assert.NoError(err)
assert.Equal(twoMegabytes, n)
}

func TestReadBig(t *testing.T) {
assert := require.New(t)

_, err := client.PutObject(context.Background(), &s3.PutObjectInput{
Bucket: aws.String(testBucketName),
Key: aws.String("test_read_big.txt"),
Body: bytes.NewReader(generateData(threeMegabytes)),
})
assert.NoError(err)

s3fs := s3iofs.NewWithClient(testBucketName, client)

f, err := s3fs.Open("test_read_big.txt")
assert.NoError(err)

defer f.Close()

n, err := f.Read(make([]byte, oneMegabyte))
assert.NoError(err)
assert.Equal(oneMegabyte, n)

n, err = f.Read(make([]byte, twoMegabytes))
assert.NoError(err)
assert.Equal(twoMegabytes, n)

n, err = f.Read(make([]byte, 1))
assert.Error(err)
assert.Equal(0, n)
}

func TestReadBigEOF(t *testing.T) {
assert := require.New(t)

_, err := client.PutObject(context.Background(), &s3.PutObjectInput{
Bucket: aws.String(testBucketName),
Key: aws.String("test_read_big_eof.txt"),
Body: bytes.NewReader(generateData(oneMegabyte)),
})
assert.NoError(err)

s3fs := s3iofs.NewWithClient(testBucketName, client)

f, err := s3fs.Open("test_read_big_eof.txt")
assert.NoError(err)

defer f.Close()

n, err := f.Read(make([]byte, twoMegabytes))
assert.ErrorIs(err, io.ErrUnexpectedEOF)
assert.Equal(oneMegabyte, n)
}
4 changes: 1 addition & 3 deletions s3file.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (s3f *s3File) Read(p []byte) (int, error) {
return 0, err
}

size, err := r.Read(p)
size, err := io.ReadFull(r, p)
s3f.offset += int64(size)
if err != nil {
if err != io.EOF {
Expand Down Expand Up @@ -94,8 +94,6 @@ func (s3f *s3File) ReadAt(p []byte, offset int64) (n int, err error) {
}
}

// fmt.Println("offset", offset, "size", s3f.size)

return size, r.Close()
}

Expand Down

0 comments on commit a2fa574

Please sign in to comment.