compact: Fixed minor logging issues. (#2353)

Fixes: thanos-io/thanos#2322

Signed-off-by: Bartlomiej Plotka <bwplotka@gmail.com>

objstore.go

@@ -4,6 +4,7 @@
 package objstore
 
 import (
+ "bytes"
  "context"
  "fmt"
  "io"
@@ -40,6 +41,24 @@ type Bucket interface {
  Name() string
 }
 
+// TryToGetSize tries to get upfront size from reader.
+// TODO(https://github.com/thanos-io/thanos/issues/678): Remove guessing length when minio provider will support multipart upload without this.
+func TryToGetSize(r io.Reader) (int64, error) {
+ switch f := r.(type) {
+ case *os.File:
+ fileInfo, err := f.Stat()
+ if err != nil {
+ return 0, errors.Wrap(err, "os.File.Stat()")
+ }
+ return fileInfo.Size(), nil
+ case *bytes.Buffer:
+ return int64(f.Len()), nil
+ case *strings.Reader:
+ return f.Size(), nil
+ }
+ return 0, errors.New("unsupported type of io.Reader")
+}
+
 // BucketReader provides read access to an object storage bucket.
 type BucketReader interface {
  // Iter calls f for each entry in the given directory (not recursive.). The argument to f is the full
@@ -53,7 +72,6 @@ type BucketReader interface {
  GetRange(ctx context.Context, name string, off, length int64) (io.ReadCloser, error)
 
  // Exists checks if the given object exists in the bucket.
- // TODO(bplotka): Consider removing Exists in favor of helper that do Get & IsObjNotFoundErr (less code to maintain).
  Exists(ctx context.Context, name string) (bool, error)
 
  // IsObjNotFoundErr returns true if error means that object is not found. Relevant to Get operations.

oss/oss.go

@@ -65,26 +65,16 @@ func NewTestBucket(t testing.TB) (objstore.Bucket, func(), error) {
  return NewTestBucketFromConfig(t, c, false)
 }
 
-func calculateChunks(name string, r io.Reader) (int, int64, error) {
- switch f := r.(type) {
- case *os.File:
- if fileInfo, err := f.Stat(); err == nil {
- s := fileInfo.Size()
- return int(math.Floor(float64(s) / PartSize)), s % PartSize, nil
- }
- case *strings.Reader:
- return int(math.Floor(float64(f.Size()) / PartSize)), f.Size() % PartSize, nil
- }
- return -1, 0, errors.New("unsupported implement of io.Reader")
-}
-
 // Upload the contents of the reader as an object into the bucket.
-func (b *Bucket) Upload(ctx context.Context, name string, r io.Reader) error {
- chunksnum, lastslice, err := calculateChunks(name, r)
+func (b *Bucket) Upload(_ context.Context, name string, r io.Reader) error {
+ // TODO(https://github.com/thanos-io/thanos/issues/678): Remove guessing length when minio provider will support multipart upload without this.
+ size, err := objstore.TryToGetSize(r)
  if err != nil {
- return err
+ return errors.Wrapf(err, "failed to get size apriori to upload %s", name)
  }
 
+ chunksnum, lastslice := int(math.Floor(float64(size)/PartSize)), size%PartSize
+
  ncloser := ioutil.NopCloser(r)
  switch chunksnum {
  case 0:

s3/s3.go

@@ -287,7 +287,7 @@ func (b *Bucket) GetRange(ctx context.Context, name string, off, length int64) (
 }
 
 // Exists checks if the given object exists.
-func (b *Bucket) Exists(ctx context.Context, name string) (bool, error) {
+func (b *Bucket) Exists(_ context.Context, name string) (bool, error) {
  _, err := b.client.StatObject(b.name, name, minio.StatObjectOptions{})
  if err != nil {
  if b.IsObjNotFoundErr(err) {
@@ -299,24 +299,14 @@ func (b *Bucket) Exists(ctx context.Context, name string) (bool, error) {
  return true, nil
 }
 
-func (b *Bucket) guessFileSize(name string, r io.Reader) int64 {
- if f, ok := r.(*os.File); ok {
- fileInfo, err := f.Stat()
- if err == nil {
- return fileInfo.Size()
- }
- level.Warn(b.logger).Log("msg", "could not stat file for multipart upload", "name", name, "err", err)
- return -1
- }
-
- level.Warn(b.logger).Log("msg", "could not guess file size for multipart upload", "name", name)
- return -1
-}
-
 // Upload the contents of the reader as an object into the bucket.
 func (b *Bucket) Upload(ctx context.Context, name string, r io.Reader) error {
  // TODO(https://github.com/thanos-io/thanos/issues/678): Remove guessing length when minio provider will support multipart upload without this.
- size := b.guessFileSize(name, r)
+ size, err := objstore.TryToGetSize(r)
+ if err != nil {
+ level.Warn(b.logger).Log("msg", "could not guess file size for multipart upload; upload might be not optimized", "name", name, "err", err)
+ size = -1
+ }
 
  // partSize cannot be larger than object size.
  partSize := b.partSize
@@ -342,7 +332,7 @@ func (b *Bucket) Upload(ctx context.Context, name string, r io.Reader) error {
 }
 
 // ObjectSize returns the size of the specified object.
-func (b *Bucket) ObjectSize(ctx context.Context, name string) (uint64, error) {
+func (b *Bucket) ObjectSize(_ context.Context, name string) (uint64, error) {
  objInfo, err := b.client.StatObject(b.name, name, minio.StatObjectOptions{})
  if err != nil {
  return 0, err
@@ -351,7 +341,7 @@ func (b *Bucket) ObjectSize(ctx context.Context, name string) (uint64, error) {
 }
 
 // Delete removes the object with the given name.
-func (b *Bucket) Delete(ctx context.Context, name string) error {
+func (b *Bucket) Delete(_ context.Context, name string) error {
  return b.client.RemoveObject(b.name, name)
 }