From 3c1711e0bd41bd9db807471a9a4230ca85e7865c Mon Sep 17 00:00:00 2001 From: Bartlomiej Plotka Date: Fri, 3 Apr 2020 14:06:53 +0100 Subject: [PATCH 1/2] binaryHeader: Fixed partial write issue for index-header. Fixes https://github.com/thanos-io/thanos/issues/2213 This caused was indicated as regression of latency, and also causes potential critical issue for store GW, where manual delete of index-header from local storage was required. This might be considered as blocker for 0.12, so it would be worth to port it to 0.12 TBH @squat. Signed-off-by: Bartlomiej Plotka --- pkg/block/indexheader/binary_reader.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/block/indexheader/binary_reader.go b/pkg/block/indexheader/binary_reader.go index 9005cd443b..6c017d078e 100644 --- a/pkg/block/indexheader/binary_reader.go +++ b/pkg/block/indexheader/binary_reader.go @@ -71,20 +71,21 @@ type BinaryTOC struct { } // WriteBinary build index-header file from the pieces of index in object storage. -func WriteBinary(ctx context.Context, bkt objstore.BucketReader, id ulid.ULID, fn string) (err error) { +func WriteBinary(ctx context.Context, bkt objstore.BucketReader, id ulid.ULID, filename string) (err error) { ir, indexVersion, err := newChunkedIndexReader(ctx, bkt, id) if err != nil { return errors.Wrap(err, "new index reader") } + tmpFilename := filename + ".tmp" // Buffer for copying and encbuffers. // This also will control the size of file writer buffer. buf := make([]byte, 32*1024) - bw, err := newBinaryWriter(fn, buf) + bw, err := newBinaryWriter(tmpFilename, buf) if err != nil { return errors.Wrap(err, "new binary index header writer") } - defer runutil.CloseWithErrCapture(&err, bw, "close binary writer for %s", fn) + defer runutil.CloseWithErrCapture(&err, bw, "close binary writer for %s", tmpFilename) if err := bw.AddIndexMeta(indexVersion, ir.toc.PostingsTable); err != nil { return errors.Wrap(err, "add index meta") @@ -109,7 +110,9 @@ func WriteBinary(ctx context.Context, bkt objstore.BucketReader, id ulid.ULID, f if err := bw.WriteTOC(); err != nil { return errors.Wrap(err, "write index header TOC") } - return nil + + // Create index-header in atomic way, to avoid partial writes (e.g during restart or crash of store GW). + return os.Rename(tmpFilename, filename) } type chunkedIndexReader struct { From 8c017d95caddb3c912608ed163c96dd9b3c6e924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lucas=20Serv=C3=A9n=20Mar=C3=ADn?= Date: Mon, 6 Apr 2020 12:35:55 +0200 Subject: [PATCH 2/2] binary_reader: ensure fs is synced before renaming MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Lucas Servén Marín --- pkg/block/indexheader/binary_reader.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/block/indexheader/binary_reader.go b/pkg/block/indexheader/binary_reader.go index 6c017d078e..e14e98be3f 100644 --- a/pkg/block/indexheader/binary_reader.go +++ b/pkg/block/indexheader/binary_reader.go @@ -111,6 +111,14 @@ func WriteBinary(ctx context.Context, bkt objstore.BucketReader, id ulid.ULID, f return errors.Wrap(err, "write index header TOC") } + if err := bw.f.Flush(); err != nil { + return errors.Wrap(err, "flush") + } + + if err := bw.f.f.Sync(); err != nil { + return errors.Wrap(err, "sync") + } + // Create index-header in atomic way, to avoid partial writes (e.g during restart or crash of store GW). return os.Rename(tmpFilename, filename) }