check for retention period to be a multiple of periodic table duration and relevant test (#1564)

* added check for retention period to be a multiple of periodic table duration and relevant test

Signed-off-by: Sandeep Sukhani <[email protected]>

* updated changelog

Signed-off-by: Sandeep Sukhani <[email protected]>

* check for periodic table duration not 0

Signed-off-by: Sandeep Sukhani <[email protected]>

* moved update in changelog above features

Signed-off-by: Sandeep Sukhani <[email protected]>

Author: sandeepsukhani

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## master / unreleased
 
+* [CHANGE] Retention period should now be a multiple of periodic table duration #1564
 * [FEATURE] Add option to use jump hashing to load balance requests to memcached #1554
 * [FEATURE] Add status page for HA tracker to distributors #1546
 

pkg/chunk/table_manager.go

@@ -2,6 +2,7 @@ package chunk
 
 import (
 	"context"
+	"errors"
 	"flag"
 	"fmt"
 	"sort"
@@ -126,6 +127,15 @@ type TableManager struct {
 // NewTableManager makes a new TableManager
 func NewTableManager(cfg TableManagerConfig, schemaCfg SchemaConfig, maxChunkAge time.Duration, tableClient TableClient,
 	objectClient BucketClient) (*TableManager, error) {
+
+	if cfg.RetentionPeriod != 0 {
+		// Assume the newest config is the one to use for validation of retention
+		indexTablesPeriod := schemaCfg.Configs[len(schemaCfg.Configs)-1].IndexTables.Period
+		if indexTablesPeriod != 0 && cfg.RetentionPeriod%indexTablesPeriod != 0 {
+			return nil, errors.New("retention period should now be a multiple of periodic table duration")
+		}
+	}
+
 	return &TableManager{
 		cfg:          cfg,
 		schemaCfg:    schemaCfg,

pkg/chunk/table_manager_test.go

@@ -691,4 +691,9 @@ func TestTableManagerRetentionOnly(t *testing.T) {
 			{Name: chunkTablePrefix + "3", ProvisionedRead: read, ProvisionedWrite: write},
 		},
 	)
+
+	// Test table manager retention not multiple of periodic config
+	tbmConfig.RetentionPeriod++
+	_, err = NewTableManager(tbmConfig, cfg, maxChunkAge, client, nil)
+	require.Error(t, err)
 }