fs: ListObjects() was reading ETag at wrong offsets #4846
Conversation
|
This fixes the issue as I tested it. You mean, in a GNU/Linux, a call to a read() of a moved file returns io.EOF ? |
Well the underlying offset would have moved if there is a concurrent read. io.SectionReader helps in seeking back the offsets for the requested Size() and Offset. |
cmd/fs-v1.go
Outdated
| // `fs.json` can be empty due to previously failed | ||
| // PutObject() transaction, if we arrive at such | ||
| // a situation we just ignore and continue. | ||
| if len(fsMetaBuf) == 0 { |
There was a problem hiding this comment.
You could use fi.Size() to validate before ioutil.ReadAll()
cmd/fs-v1.go
Outdated
| @@ -749,14 +749,21 @@ func (fs fsObjects) getObjectETag(bucket, entry string) (string, error) { | |||
| // Read from fs metadata only if it exists. | |||
| defer fs.rwPool.Close(fsMetaPath) | |||
|
|
|||
| fsMetaBuf, err := ioutil.ReadAll(rlk.LockedFile) | |||
| fi, err := rlk.LockedFile.Stat() | |||
There was a problem hiding this comment.
You could add a comment why Stat() then ioutil.ReadAll() approach is required with ReadAt()
harshavardhana
force-pushed
the
fs-objectinfo
branch
from
b7276b3
to
bbe4966
Compare
Codecov Report
@@ Coverage Diff @@
## master #4846 +/- ##
==========================================
- Coverage 62.97% 62.97% -0.01%
==========================================
Files 191 191
Lines 27444 27449 +5
==========================================
+ Hits 17283 17285 +2
- Misses 8991 8992 +1
- Partials 1170 1172 +2
Continue to review full report at Codecov.
|
harshavardhana
force-pushed
the
fs-objectinfo
branch
2 times, most recently
from
13fa1f7
to
bcc3069
Compare
|
@balamurugana can you take a look again? |
Current code was just using io.ReadAll() on an fd() which might have moved underneath due to a concurrent read operation. Subsequent read will result in EOF We should always seek back and read again. pread() is allowed on all platforms use io.SectionReader to read from the beginning of the file. Fixes minio#4842
harshavardhana
force-pushed
the
fs-objectinfo
branch
from
bcc3069
to
2f15c60
Compare
Description
Current code was just using io.ReadAll() on an fd()
which might have moved underneath due to a concurrent
read operation. Subsequent read will result in EOF
We should always seek back and read again. pread()
is allowed on all platforms use io.SectionReader to
read from the beginning of the file.
Motivation and Context
Fixes #4842
How Has This Been Tested?
Manually and using the reproducer.
Types of changes
Checklist: