Skip to content

Define bucket index struct #3553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 1, 2020
Merged

Define bucket index struct #3553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ee8704
Introduced bucket index structs
pracucci Dec 1, 2020
55494d0
Upgraded Thanos
pracucci Dec 1, 2020
3c76967
Addressed review comments
pracucci Dec 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ require (
github.com/sony/gobreaker v0.4.1
github.com/spf13/afero v1.2.2
github.com/stretchr/testify v1.6.1
github.com/thanos-io/thanos v0.13.1-0.20201125124348-2008ef009e88
github.com/thanos-io/thanos v0.13.1-0.20201130180807-84afc97e7d58
github.com/uber/jaeger-client-go v2.25.0+incompatible
github.com/weaveworks/common v0.0.0-20201119133501-0619918236ec
go.etcd.io/bbolt v1.3.5-0.20200615073812-232d8fc87f50
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1146,8 +1146,8 @@ github.com/thanos-io/thanos v0.13.1-0.20200807203500-9b578afb4763/go.mod h1:KyW0
github.com/thanos-io/thanos v0.13.1-0.20201019130456-f41940581d9a/go.mod h1:A3qUEEbsVkplJnxyDLwuIuvTDaJPByTH+hMdTl9ujAA=
github.com/thanos-io/thanos v0.13.1-0.20201030101306-47f9a225cc52 h1:z3hglXVwJ4HgU0OoDS+8+MvEipv/U83IQ+fMsDr00YQ=
github.com/thanos-io/thanos v0.13.1-0.20201030101306-47f9a225cc52/go.mod h1:OqqX4x21cg5N5MMHd/yGQAc/V3wg8a7Do4Jk8HfaFZQ=
github.com/thanos-io/thanos v0.13.1-0.20201125124348-2008ef009e88 h1:r3lyiYA58zA6yE4CqE0ncavgI2rHgyhLQom5z0usbNM=
github.com/thanos-io/thanos v0.13.1-0.20201125124348-2008ef009e88/go.mod h1:ffr9z+gefM664JBH/CEMHyHvShq2BQTejT/Ws+V+80Q=
github.com/thanos-io/thanos v0.13.1-0.20201130180807-84afc97e7d58 h1:Q5t3TKhiFQ2J3XQv1psoMBSBk/Dx6p4JqoETXiWQaYg=
github.com/thanos-io/thanos v0.13.1-0.20201130180807-84afc97e7d58/go.mod h1:ffr9z+gefM664JBH/CEMHyHvShq2BQTejT/Ws+V+80Q=
github.com/themihai/gomemcache v0.0.0-20180902122335-24332e2d58ab h1:7ZR3hmisBWw77ZpO1/o86g+JV3VKlk3d48jopJxzTjU=
github.com/themihai/gomemcache v0.0.0-20180902122335-24332e2d58ab/go.mod h1:eheTFp954zcWZXCU8d0AT76ftsQOTo4DTqkN/h3k1MY=
github.com/tidwall/pretty v0.0.0-20180105212114-65a9db5fad51/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
Expand Down
52 changes: 0 additions & 52 deletions pkg/querier/block_meta.go
View file
Open in desktop

This file was deleted.

19 changes: 10 additions & 9 deletions pkg/querier/blocks_consistency_checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
"github.com/oklog/ulid"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/promauto"
"github.com/thanos-io/thanos/pkg/block/metadata"

"github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex"
)

type BlocksConsistencyChecker struct {
Expand Down Expand Up @@ -36,7 +37,7 @@ func NewBlocksConsistencyChecker(uploadGracePeriod, deletionGracePeriod time.Dur
}
}

func (c *BlocksConsistencyChecker) Check(knownBlocks []*BlockMeta, knownDeletionMarks map[ulid.ULID]*metadata.DeletionMark, queriedBlocks []ulid.ULID) (missingBlocks []ulid.ULID) {
func (c *BlocksConsistencyChecker) Check(knownBlocks bucketindex.Blocks, knownDeletionMarks map[ulid.ULID]*bucketindex.BlockDeletionMark, queriedBlocks []ulid.ULID) (missingBlocks []ulid.ULID) {
c.checksTotal.Inc()

// Reverse the map of queried blocks, so that we can easily look for missing ones.
Expand All @@ -46,34 +47,34 @@ func (c *BlocksConsistencyChecker) Check(knownBlocks []*BlockMeta, knownDeletion
}

// Look for any missing block.
for _, meta := range knownBlocks {
for _, block := range knownBlocks {
// Some recently uploaded blocks, already discovered by the querier, may not have been discovered
// and loaded by the store-gateway yet. In order to avoid false positives, we grant some time
// to the store-gateway to discover them. It's safe to exclude recently uploaded blocks because:
// - Blocks uploaded by ingesters: we will continue querying them from ingesters for a while (depends
// on the configured retention period).
// - Blocks uploaded by compactor: the source blocks are marked for deletion but will continue to be
// queried by queriers for a while (depends on the configured deletion marks delay).
if c.uploadGracePeriod > 0 && time.Since(meta.UploadedAt) < c.uploadGracePeriod {
level.Debug(c.logger).Log("msg", "block skipped from consistency check because it was uploaded recently", "block", meta.ULID.String(), "uploadedAt", meta.UploadedAt.String())
if c.uploadGracePeriod > 0 && time.Since(block.GetUploadedAt()) < c.uploadGracePeriod {
level.Debug(c.logger).Log("msg", "block skipped from consistency check because it was uploaded recently", "block", block.ID.String(), "uploadedAt", block.GetUploadedAt().String())
continue
}

// The store-gateway may offload blocks before the querier. If that happens, the querier will run a consistency check
// on blocks that can't be queried because they were offloaded. For this reason, we don't run the consistency check on any block
// which has been marked for deletion more then "grace period" time ago. Basically, the grace period is the time
// we still expect a block marked for deletion to be still queried.
if mark := knownDeletionMarks[meta.ULID]; mark != nil {
if mark := knownDeletionMarks[block.ID]; mark != nil {
deletionTime := time.Unix(mark.DeletionTime, 0)

if c.deletionGracePeriod > 0 && time.Since(deletionTime) > c.deletionGracePeriod {
level.Debug(c.logger).Log("msg", "block skipped from consistency check because it is marked for deletion", "block", meta.ULID.String(), "deletionTime", deletionTime.String())
level.Debug(c.logger).Log("msg", "block skipped from consistency check because it is marked for deletion", "block", block.ID.String(), "deletionTime", deletionTime.String())
continue
}
}

if _, ok := actualBlocks[meta.ULID]; !ok {
missingBlocks = append(missingBlocks, meta.ULID)
if _, ok := actualBlocks[block.ID]; !ok {
missingBlocks = append(missingBlocks, block.ID)
}
}

Expand Down
75 changes: 37 additions & 38 deletions pkg/querier/blocks_consistency_checker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import (
"github.com/oklog/ulid"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/prometheus/prometheus/tsdb"
"github.com/stretchr/testify/assert"
"github.com/thanos-io/thanos/pkg/block/metadata"

"github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex"
"github.com/cortexproject/cortex/pkg/util"
)

Expand All @@ -25,78 +24,78 @@ func TestBlocksConsistencyChecker_Check(t *testing.T) {
block3 := ulid.MustNew(uint64(util.TimeToMillis(now.Add(-uploadGracePeriod*4))), nil)

tests := map[string]struct {
knownBlocks []*BlockMeta
knownDeletionMarks map[ulid.ULID]*metadata.DeletionMark
knownBlocks bucketindex.Blocks
knownDeletionMarks map[ulid.ULID]*bucketindex.BlockDeletionMark
queriedBlocks []ulid.ULID
expectedMissingBlocks []ulid.ULID
}{
"no known blocks": {
knownBlocks: []*BlockMeta{},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{},
knownBlocks: bucketindex.Blocks{},
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{},
queriedBlocks: []ulid.ULID{},
},
"all known blocks have been queried from a single store-gateway": {
knownBlocks: []*BlockMeta{
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block1}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block2}}, UploadedAt: now.Add(-time.Hour)},
knownBlocks: bucketindex.Blocks{
{ID: block1, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block2, UploadedAt: now.Add(-time.Hour).Unix()},
},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{},
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{},
queriedBlocks: []ulid.ULID{block1, block2},
},
"all known blocks have been queried from multiple store-gateway": {
knownBlocks: []*BlockMeta{
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block1}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block2}}, UploadedAt: now.Add(-time.Hour)},
knownBlocks: bucketindex.Blocks{
{ID: block1, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block2, UploadedAt: now.Add(-time.Hour).Unix()},
},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{},
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{},
queriedBlocks: []ulid.ULID{block1, block2},
},
"store-gateway has queried more blocks than expected": {
knownBlocks: []*BlockMeta{
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block1}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block2}}, UploadedAt: now.Add(-time.Hour)},
knownBlocks: bucketindex.Blocks{
{ID: block1, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block2, UploadedAt: now.Add(-time.Hour).Unix()},
},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{},
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{},
queriedBlocks: []ulid.ULID{block1, block2, block3},
},
"store-gateway has queried less blocks than expected": {
knownBlocks: []*BlockMeta{
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block1}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block2}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block3}}, UploadedAt: now.Add(-time.Hour)},
knownBlocks: bucketindex.Blocks{
{ID: block1, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block2, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block3, UploadedAt: now.Add(-time.Hour).Unix()},
},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{},
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{},
queriedBlocks: []ulid.ULID{block1, block3},
expectedMissingBlocks: []ulid.ULID{block2},
},
"store-gateway has queried less blocks than expected, but the missing block has been recently uploaded": {
knownBlocks: []*BlockMeta{
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block1}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block2}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block3}}, UploadedAt: now.Add(-uploadGracePeriod).Add(time.Minute)},
knownBlocks: bucketindex.Blocks{
{ID: block1, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block2, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block3, UploadedAt: now.Add(-uploadGracePeriod).Add(time.Minute).Unix()},
},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{},
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{},
queriedBlocks: []ulid.ULID{block1, block2},
},
"store-gateway has queried less blocks than expected and the missing block has been recently marked for deletion": {
knownBlocks: []*BlockMeta{
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block1}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block2}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block3}}, UploadedAt: now.Add(-time.Hour)},
knownBlocks: bucketindex.Blocks{
{ID: block1, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block2, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block3, UploadedAt: now.Add(-time.Hour).Unix()},
},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{
block3: {DeletionTime: now.Add(-deletionGracePeriod / 2).Unix()},
},
queriedBlocks: []ulid.ULID{block1, block2},
expectedMissingBlocks: []ulid.ULID{block3},
},
"store-gateway has queried less blocks than expected and the missing block has been marked for deletion long time ago": {
knownBlocks: []*BlockMeta{
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block1}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block2}}, UploadedAt: now.Add(-time.Hour)},
{Meta: metadata.Meta{BlockMeta: tsdb.BlockMeta{ULID: block3}}, UploadedAt: now.Add(-time.Hour)},
knownBlocks: bucketindex.Blocks{
{ID: block1, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block2, UploadedAt: now.Add(-time.Hour).Unix()},
{ID: block3, UploadedAt: now.Add(-time.Hour).Unix()},
},
knownDeletionMarks: map[ulid.ULID]*metadata.DeletionMark{
knownDeletionMarks: map[ulid.ULID]*bucketindex.BlockDeletionMark{
block3: {DeletionTime: now.Add(-deletionGracePeriod * 2).Unix()},
},
queriedBlocks: []ulid.ULID{block1, block2},
Expand Down
Loading