Skip to content

Commit

Permalink
Avoid unnecessary reopening of HTTP streams in GetObject() (#1908)
Browse files Browse the repository at this point in the history
Sometimes consumers of Seek() might use it for purposes other than
chaging the current read/write offset. For example, using whence =
io.SeekCurrent to get the current seek position. However, minio-go
invalidates the underlying HTTP stream unconditionally when Seek() is
called, resulting in massive performance degradation in these scenarios.

This commit avoids these issues by only reopening the stream if the seek
position is actually changed.
  • Loading branch information
PeterCxy authored Dec 1, 2023
1 parent 54e115c commit 7c8e5d6
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions api-get-object.go
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,8 @@ func (o *Object) Seek(offset int64, whence int) (n int64, err error) {
}
}

newOffset := o.currOffset

// Switch through whence.
switch whence {
default:
Expand All @@ -558,12 +560,12 @@ func (o *Object) Seek(offset int64, whence int) (n int64, err error) {
if o.objectInfo.Size > -1 && offset > o.objectInfo.Size {
return 0, io.EOF
}
o.currOffset = offset
newOffset = offset
case 1:
if o.objectInfo.Size > -1 && o.currOffset+offset > o.objectInfo.Size {
return 0, io.EOF
}
o.currOffset += offset
newOffset += offset
case 2:
// If we don't know the object size return an error for io.SeekEnd
if o.objectInfo.Size < 0 {
Expand All @@ -579,16 +581,17 @@ func (o *Object) Seek(offset int64, whence int) (n int64, err error) {
if o.objectInfo.Size+offset < 0 {
return 0, errInvalidArgument(fmt.Sprintf("Seeking at negative offset not allowed for %d", whence))
}
o.currOffset = o.objectInfo.Size + offset
newOffset = o.objectInfo.Size + offset
}
// Reset the saved error since we successfully seeked, let the Read
// and ReadAt decide.
if o.prevErr == io.EOF {
o.prevErr = nil
}

// Ask lower level to fetch again from source
o.seekData = true
// Ask lower level to fetch again from source when necessary
o.seekData = (newOffset != o.currOffset) || o.seekData
o.currOffset = newOffset

// Return the effective offset.
return o.currOffset, nil
Expand Down

0 comments on commit 7c8e5d6

Please sign in to comment.