From 28d03d72ddbfa9919ff9c0c40846497845df40c4 Mon Sep 17 00:00:00 2001 From: Sandeep Sukhani Date: Mon, 13 Jan 2020 16:18:37 +0530 Subject: [PATCH 1/3] fix calculation of expected tables and create tables from upcoming schema considering grace period Signed-off-by: Sandeep Sukhani --- pkg/chunk/schema_config.go | 4 ++-- pkg/chunk/table_manager.go | 3 ++- pkg/chunk/table_manager_test.go | 29 +++++++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/chunk/schema_config.go b/pkg/chunk/schema_config.go index 4e3e7683395..8409fe1714b 100644 --- a/pkg/chunk/schema_config.go +++ b/pkg/chunk/schema_config.go @@ -424,8 +424,8 @@ func (cfg *PeriodicTableConfig) periodicTables(from, through model.Time, pCfg Pr nowWeek = now / periodSecs result = []TableDesc{} ) - // If through ends on 00:00 of the day, don't include the upcoming day - if through.Unix()%secondsInDay == 0 { + // Make sure we don't have an extra table + if through.Unix()%periodSecs == 0 { lastTable-- } // Don't make tables further back than the configured retention diff --git a/pkg/chunk/table_manager.go b/pkg/chunk/table_manager.go index eba9b23f358..6001cb7f12b 100644 --- a/pkg/chunk/table_manager.go +++ b/pkg/chunk/table_manager.go @@ -239,7 +239,8 @@ func (m *TableManager) calculateExpectedTables() []TableDesc { result := []TableDesc{} for i, config := range m.schemaCfg.Configs { - if config.From.Time.Time().After(mtime.Now()) { + // Consider configs which we are about to hit and requires tables to be created due to grace period + if config.From.Time.Time().After(mtime.Now().Add(m.cfg.CreationGracePeriod)) { continue } if config.IndexTables.Period == 0 { // non-periodic table diff --git a/pkg/chunk/table_manager_test.go b/pkg/chunk/table_manager_test.go index a6951d7ab8f..cecc7edfa2f 100644 --- a/pkg/chunk/table_manager_test.go +++ b/pkg/chunk/table_manager_test.go @@ -285,6 +285,21 @@ func TestTableManager(t *testing.T) { }, ) + // Move ahead where we are short by just grace period before hitting next section + tmTest(t, client, tableManager, + "Move ahead where we are short by just grace period before hitting next section", + weeklyTable2Start.Add(-gracePeriod+time.Second), + []TableDesc{ + {Name: baseTableName, ProvisionedRead: inactiveRead, ProvisionedWrite: inactiveWrite}, + {Name: tablePrefix + week1Suffix, ProvisionedRead: inactiveRead, ProvisionedWrite: inactiveWrite, WriteScale: inactiveScalingConfig}, + {Name: tablePrefix + week2Suffix, ProvisionedRead: read, ProvisionedWrite: write, WriteScale: activeScalingConfig}, + {Name: table2Prefix + "5", ProvisionedRead: read, ProvisionedWrite: write, WriteScale: activeScalingConfig}, + {Name: chunkTablePrefix + week1Suffix, ProvisionedRead: inactiveRead, ProvisionedWrite: inactiveWrite}, + {Name: chunkTablePrefix + week2Suffix, ProvisionedRead: read, ProvisionedWrite: write}, + {Name: chunkTable2Prefix + "5", ProvisionedRead: read, ProvisionedWrite: write}, + }, + ) + // Move to the next section of the config tmTest(t, client, tableManager, "Move forward to next section of schema config", @@ -648,6 +663,20 @@ func TestTableManagerRetentionOnly(t *testing.T) { }, ) + // Check after three weeks and a day short by grace period, we have three tables (two previous periods and the new one), table 0 was deleted + tmTest(t, client, tableManager, + "Move forward by three table periods and a day short by grace period", + baseTableStart.Add(tablePeriod*3+24*time.Hour-gracePeriod), + []TableDesc{ + {Name: tablePrefix + "1", ProvisionedRead: inactiveRead, ProvisionedWrite: inactiveWrite, WriteScale: inactiveScalingConfig}, + {Name: tablePrefix + "2", ProvisionedRead: inactiveRead, ProvisionedWrite: inactiveWrite, WriteScale: inactiveScalingConfig}, + {Name: tablePrefix + "3", ProvisionedRead: read, ProvisionedWrite: write}, + {Name: chunkTablePrefix + "1", ProvisionedRead: inactiveRead, ProvisionedWrite: inactiveWrite}, + {Name: chunkTablePrefix + "2", ProvisionedRead: inactiveRead, ProvisionedWrite: inactiveWrite}, + {Name: chunkTablePrefix + "3", ProvisionedRead: read, ProvisionedWrite: write}, + }, + ) + // Verify that without RetentionDeletesEnabled no tables are removed tableManager.cfg.RetentionDeletesEnabled = false // Retention > 0 will prevent older tables from being created so we need to create the old tables manually for the test From c08f40fcd4b6879c7ad9b5594aa1aa3035136a54 Mon Sep 17 00:00:00 2001 From: Sandeep Sukhani Date: Mon, 13 Jan 2020 16:55:39 +0530 Subject: [PATCH 2/3] updated changelog Signed-off-by: Sandeep Sukhani --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b3736ca418f..ebb383375c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ instructions below to upgrade your Postgres. * [BUGFIX] TSDB: Fixed `cortex_ingester_queried_samples` and `cortex_ingester_queried_series` metrics when using block storage. #1981 * [BUGFIX] TSDB: Fixed `cortex_ingester_memory_series` and `cortex_ingester_memory_users` metrics when using with the experimental TSDB blocks storage. #1982 * [BUGFIX] TSDB: Fixed `cortex_ingester_memory_series_created_total` and `cortex_ingester_memory_series_removed_total` metrics when using TSDB blocks storage. #1990 +* [BUGFIX] Table Manager: Fixed calculation of expected tables and creation of tables from next active schema considering grace period. #1976 ### Upgrading Postgres (if you're using configs service) From b5f271ec25069eeca8a1328677a8ec2b9a302175 Mon Sep 17 00:00:00 2001 From: Sandeep Sukhani Date: Wed, 22 Jan 2020 14:35:32 +0530 Subject: [PATCH 3/3] updated comment to make it clearer Signed-off-by: Sandeep Sukhani --- pkg/chunk/schema_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/chunk/schema_config.go b/pkg/chunk/schema_config.go index 8409fe1714b..06312f827bf 100644 --- a/pkg/chunk/schema_config.go +++ b/pkg/chunk/schema_config.go @@ -424,7 +424,7 @@ func (cfg *PeriodicTableConfig) periodicTables(from, through model.Time, pCfg Pr nowWeek = now / periodSecs result = []TableDesc{} ) - // Make sure we don't have an extra table + // If interval ends exactly on a period boundary, don’t include the upcoming period if through.Unix()%periodSecs == 0 { lastTable-- }