Skip to content

Commit

Permalink
fix: Export exportTSInSecs field in TSDB identifier to make sure co…
Browse files Browse the repository at this point in the history
…rrect gap calculation (#13116)

 In summary, the Meta(now in loki/pkg/storage/stores/shipper/bloomshipper/client.go) structure contains a Sources []tsdb.SingleTenantTSDBIdentifier field, and within tsdb.SingleTenantTSDBIdentifier, there is an exportTSInSecs field. When Meta is serialized and written to OSS storage, the exportTSInSecs field is being set to its default value, false, which is not the expected behavior. It should retain its actual value; otherwise, this will lead to issues during subsequent gap calculations.

Co-authored-by: Christian Haudum <christian.haudum@gmail.com>
  • Loading branch information
honganan and chaudum authored Jan 29, 2025
1 parent 65f5990 commit 99d9f1c
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 7 deletions.
8 changes: 4 additions & 4 deletions pkg/storage/stores/shipper/indexshipper/tsdb/identifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,11 @@ func (p prefixedIdentifier) Name() string {
// Identifier has all the information needed to resolve a TSDB index
// Notably this abstracts away OS path separators, etc.
type SingleTenantTSDBIdentifier struct {
// exportTSInSecs tells whether creation timestamp should be exported in unix seconds instead of nanoseconds.
// ExportTSInSecs tells whether creation timestamp should be exported in unix seconds instead of nanoseconds.
// timestamp in filename could be a unix second or a unix nanosecond so
// helps us to be able to reproduce filename back from parsed identifier.
// Should be true ideally for older files with creation timestamp in seconds.
exportTSInSecs bool
ExportTSInSecs bool
TS time.Time
From, Through model.Time
Checksum uint32
Expand All @@ -83,7 +83,7 @@ func (i SingleTenantTSDBIdentifier) Hash(h hash.Hash32) (err error) {
// str builds filename with format <file-creation-ts> + `-` + `compactor` + `-` + <oldest-chunk-start-ts> + `-` + <latest-chunk-end-ts> `-` + <index-checksum>
func (i SingleTenantTSDBIdentifier) str() string {
ts := int64(0)
if i.exportTSInSecs {
if i.ExportTSInSecs {
ts = i.TS.Unix()
} else {
ts = i.TS.UnixNano()
Expand Down Expand Up @@ -151,7 +151,7 @@ func ParseSingleTenantTSDBPath(p string) (id SingleTenantTSDBIdentifier, ok bool
parsedTS = time.Unix(0, ts)
}
return SingleTenantTSDBIdentifier{
exportTSInSecs: len(elems[0]) <= 10,
ExportTSInSecs: len(elems[0]) <= 10,
TS: parsedTS,
From: model.Time(from),
Through: model.Time(through),
Expand Down
33 changes: 30 additions & 3 deletions pkg/storage/stores/shipper/indexshipper/tsdb/identifier_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package tsdb

import (
"encoding/json"
"fmt"
"math"
"testing"
Expand All @@ -20,7 +21,7 @@ func TestParseSingleTenantTSDBPath(t *testing.T) {
desc: "simple_works",
input: "1-compactor-1-10-ff.tsdb",
id: SingleTenantTSDBIdentifier{
exportTSInSecs: true,
ExportTSInSecs: true,
TS: time.Unix(1, 0),
From: 1,
Through: 10,
Expand All @@ -32,7 +33,7 @@ func TestParseSingleTenantTSDBPath(t *testing.T) {
desc: "simple_works_with_nanosecond",
input: "1712534400000000000-compactor-1-10-ff.tsdb",
id: SingleTenantTSDBIdentifier{
exportTSInSecs: false,
ExportTSInSecs: false,
TS: time.Unix(0, 1712534400000000000),
From: 1,
Through: 10,
Expand All @@ -44,7 +45,7 @@ func TestParseSingleTenantTSDBPath(t *testing.T) {
desc: "uint32_max_checksum_works",
input: fmt.Sprintf("1-compactor-1-10-%x.tsdb", math.MaxUint32),
id: SingleTenantTSDBIdentifier{
exportTSInSecs: true,
ExportTSInSecs: true,
TS: time.Unix(1, 0),
From: 1,
Through: 10,
Expand Down Expand Up @@ -78,3 +79,29 @@ func TestParseSingleTenantTSDBPath(t *testing.T) {
})
}
}

func TestSingleTenantTSDBIdentifierSerialization(t *testing.T) {
for _, tc := range []struct {
desc string
input SingleTenantTSDBIdentifier
}{
{
desc: "simple_works",
input: SingleTenantTSDBIdentifier{ExportTSInSecs: true, TS: time.Unix(1, 0).UTC(), From: 1, Through: 10, Checksum: 255},
},
{
desc: "simple_works_with_nanosecond",
input: SingleTenantTSDBIdentifier{ExportTSInSecs: false, TS: time.Unix(0, 1712534400000000000).UTC(), From: 1, Through: 10, Checksum: 255},
},
} {
t.Run(tc.desc, func(t *testing.T) {
b, err := json.Marshal(tc.input)
require.NoError(t, err)

var id SingleTenantTSDBIdentifier
require.NoError(t, json.Unmarshal(b, &id))
require.Equal(t, tc.input.Name(), id.Name())
require.Equal(t, tc.input, id)
})
}
}

0 comments on commit 99d9f1c

Please sign in to comment.