fix calculation of expected tables and create tables from upcoming sc…

…hema considering grace period (#1976) * fix calculation of expected tables and create tables from upcoming schema considering grace period Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com> * updated changelog Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com> * updated comment to make it clearer Signed-off-by: Sandeep Sukhani <sandeep.d.sukhani@gmail.com>
cortexproject · Jan 22, 2020 · ab3e836 · ab3e836
1 parent a0da2c8
commit ab3e836
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 3 deletions.
diff --git 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)
 

diff --git 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 {
+	// If interval ends exactly on a period boundary, don’t include the upcoming period
+	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
@@ -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
@@ -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