From 6ab7cd0f1a7fd8b4540830d5fdef6c69e0d14510 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Fri, 15 Apr 2022 10:31:10 +0300 Subject: [PATCH] jobs: add config cleanup job (#19987) --- app/server.go | 21 +++++++++++++++ config/database.go | 9 +++++++ config/database_test.go | 47 +++++++++++++++++++++++++++++++++ config/store.go | 14 ++++++++++ model/config.go | 11 +++++--- services/telemetry/telemetry.go | 19 ++++++------- 6 files changed, 109 insertions(+), 12 deletions(-) diff --git a/app/server.go b/app/server.go index b699e77da2..1126ff13c0 100644 --- a/app/server.go +++ b/app/server.go @@ -733,6 +733,9 @@ func (s *Server) runJobs() { s.Go(func() { runCommandWebhookCleanupJob(s) }) + s.Go(func() { + runConfigCleanupJob(s) + }) if complianceI := s.Channels().Compliance; complianceI != nil { complianceI.StartComplianceDailyJob() @@ -1506,6 +1509,13 @@ func runJobsCleanupJob(s *Server) { }, time.Hour*24) } +func runConfigCleanupJob(s *Server) { + doConfigCleanup(s) + model.CreateRecurringTask("Configuration Cleanup", func() { + doConfigCleanup(s) + }, time.Hour*24) +} + func (s *Server) runInactivityCheckJob() { model.CreateRecurringTask("Server inactivity Check", func() { s.doInactivityCheck() @@ -1580,6 +1590,17 @@ func doJobsCleanup(s *Server) { } } +func doConfigCleanup(s *Server) { + if *s.Config().JobSettings.CleanupConfigThresholdDays < 0 || !config.IsDatabaseDSN(s.ConfigStore().Store.String()) { + return + } + mlog.Info("Cleaning up configuration store.") + + if err := s.ConfigStore().Store.CleanUp(); err != nil { + mlog.Warn("Error while cleaning up configurations", mlog.Err(err)) + } +} + func (s *Server) StopMetricsServer() { s.metricsLock.Lock() defer s.metricsLock.Unlock() diff --git a/config/database.go b/config/database.go index 982214eb30..ef546a7e5e 100644 --- a/config/database.go +++ b/config/database.go @@ -416,3 +416,12 @@ func (ds *DatabaseStore) String() string { func (ds *DatabaseStore) Close() error { return ds.db.Close() } + +// removes configurations from database if they are older than threshold. +func (ds *DatabaseStore) cleanUp(thresholdCreatAt int) error { + if _, err := ds.db.NamedExec("DELETE FROM Configurations Where CreateAt < :timestamp", map[string]interface{}{"timestamp": thresholdCreatAt}); err != nil { + return errors.Wrap(err, "unable to clean Configurations table") + } + + return nil +} diff --git a/config/database_test.go b/config/database_test.go index e837a7f893..7f95b36b3a 100644 --- a/config/database_test.go +++ b/config/database_test.go @@ -1113,3 +1113,50 @@ func TestDatabaseStoreString(t *testing.T) { assert.False(t, strings.Contains(maskedDSN, "mostest")) } } + +func TestCleanUp(t *testing.T) { + _, tearDown := setupConfigDatabase(t, emptyConfig, nil) + defer tearDown() + + ds, err := newTestDatabaseStore(nil) + require.NoError(t, err) + require.NotNil(t, ds) + defer ds.Close() + + dbs, ok := ds.backingStore.(*DatabaseStore) + require.True(t, ok, "should be a DatabaseStore instance") + + b, err := marshalConfig(ds.config) + require.NoError(t, err) + + ds.config.JobSettings.CleanupConfigThresholdDays = model.NewInt(30) // we set 30 days as threshold + + now := time.Now() + for i := 0; i < 5; i++ { + // 20 days, we expect to remove at least 3 configuration values from the store + // first 2 (0 and 1) will be within a month constraint, others will be older than + // a month hence we expect 3 configurations to be removed from the database. + m := -1 * i * 24 * 20 + params := map[string]interface{}{ + "id": model.NewId(), + "value": string(b), + "create_at": model.GetMillisForTime(now.Add(time.Duration(m) * time.Hour)), + } + + _, err = dbs.db.NamedExec("INSERT INTO Configurations (Id, Value, CreateAt) VALUES (:id, :value, :create_at)", params) + require.NoError(t, err) + } + var initialCount int + row := dbs.db.QueryRow("SELECT COUNT(*) FROM Configurations") + err = row.Scan(&initialCount) + require.NoError(t, err) + + err = ds.CleanUp() + require.NoError(t, err) + + var count int + row = dbs.db.QueryRow("SELECT COUNT(*) FROM Configurations") + err = row.Scan(&count) + require.NoError(t, err) + require.True(t, count+3 == initialCount) +} diff --git a/config/store.go b/config/store.go index 9bd5c23548..d8b9434bf4 100644 --- a/config/store.go +++ b/config/store.go @@ -7,6 +7,7 @@ import ( "encoding/json" "reflect" "sync" + "time" "github.com/pkg/errors" @@ -396,3 +397,16 @@ func (s *Store) IsReadOnly() bool { defer s.configLock.RUnlock() return s.readOnly } + +// Cleanup removes outdated configurations from the database. +// this is a no-op function for FileStore type backing store. +func (s *Store) CleanUp() error { + switch bs := s.backingStore.(type) { + case *DatabaseStore: + dur := time.Duration(*s.config.JobSettings.CleanupConfigThresholdDays) * time.Hour * 24 + expiry := model.GetMillisForTime(time.Now().Add(-dur)) + return bs.cleanUp(int(expiry)) + default: + return nil + } +} diff --git a/model/config.go b/model/config.go index c3cf579c82..5fb29be8c0 100644 --- a/model/config.go +++ b/model/config.go @@ -2646,9 +2646,10 @@ func (s *DataRetentionSettings) SetDefaults() { } type JobSettings struct { - RunJobs *bool `access:"write_restrictable,cloud_restrictable"` // telemetry: none - RunScheduler *bool `access:"write_restrictable,cloud_restrictable"` // telemetry: none - CleanupJobsThresholdDays *int `access:"write_restrictable,cloud_restrictable"` + RunJobs *bool `access:"write_restrictable,cloud_restrictable"` // telemetry: none + RunScheduler *bool `access:"write_restrictable,cloud_restrictable"` // telemetry: none + CleanupJobsThresholdDays *int `access:"write_restrictable,cloud_restrictable"` + CleanupConfigThresholdDays *int `access:"write_restrictable,cloud_restrictable"` } func (s *JobSettings) SetDefaults() { @@ -2663,6 +2664,10 @@ func (s *JobSettings) SetDefaults() { if s.CleanupJobsThresholdDays == nil { s.CleanupJobsThresholdDays = NewInt(-1) } + + if s.CleanupConfigThresholdDays == nil { + s.CleanupConfigThresholdDays = NewInt(-1) + } } type CloudSettings struct { diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index 25d5cc5148..6c6dc0921b 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -758,15 +758,16 @@ func (ts *TelemetryService) trackConfig() { ts.trackPluginConfig(cfg, model.PluginSettingsDefaultMarketplaceURL) ts.SendTelemetry(TrackConfigDataRetention, map[string]interface{}{ - "enable_message_deletion": *cfg.DataRetentionSettings.EnableMessageDeletion, - "enable_file_deletion": *cfg.DataRetentionSettings.EnableFileDeletion, - "enable_boards_deletion": *cfg.DataRetentionSettings.EnableBoardsDeletion, - "message_retention_days": *cfg.DataRetentionSettings.MessageRetentionDays, - "file_retention_days": *cfg.DataRetentionSettings.FileRetentionDays, - "boards_retention_days": *cfg.DataRetentionSettings.BoardsRetentionDays, - "deletion_job_start_time": *cfg.DataRetentionSettings.DeletionJobStartTime, - "batch_size": *cfg.DataRetentionSettings.BatchSize, - "cleanup_jobs_threshold_days": *cfg.JobSettings.CleanupJobsThresholdDays, + "enable_message_deletion": *cfg.DataRetentionSettings.EnableMessageDeletion, + "enable_file_deletion": *cfg.DataRetentionSettings.EnableFileDeletion, + "enable_boards_deletion": *cfg.DataRetentionSettings.EnableBoardsDeletion, + "message_retention_days": *cfg.DataRetentionSettings.MessageRetentionDays, + "file_retention_days": *cfg.DataRetentionSettings.FileRetentionDays, + "boards_retention_days": *cfg.DataRetentionSettings.BoardsRetentionDays, + "deletion_job_start_time": *cfg.DataRetentionSettings.DeletionJobStartTime, + "batch_size": *cfg.DataRetentionSettings.BatchSize, + "cleanup_jobs_threshold_days": *cfg.JobSettings.CleanupJobsThresholdDays, + "cleanup_config_threshold_days": *cfg.JobSettings.CleanupConfigThresholdDays, }) ts.SendTelemetry(TrackConfigMessageExport, map[string]interface{}{