From 231af330e51ee2bffd27229f17f4f2f03b3dfceb Mon Sep 17 00:00:00 2001 From: Nick Sanford Date: Fri, 20 Sep 2024 13:51:56 -0400 Subject: [PATCH] DATA-2506 --- services/datamanager/builtin/builtin.go | 16 ++++++------ services/datamanager/builtin/config.go | 26 ++++++++++++++++++- services/datamanager/builtin/config_test.go | 12 +++++++++ services/datamanager/builtin/sync/config.go | 8 +++--- .../datamanager/builtin/sync/config_test.go | 9 ++----- 5 files changed, 51 insertions(+), 20 deletions(-) diff --git a/services/datamanager/builtin/builtin.go b/services/datamanager/builtin/builtin.go index 23f2db4cab36..e84c6817df56 100644 --- a/services/datamanager/builtin/builtin.go +++ b/services/datamanager/builtin/builtin.go @@ -98,8 +98,8 @@ func New( connToConnectivityStateEnabled func(conn rpc.ClientConn) datasync.ConnectivityState, logger logging.Logger, ) (datamanager.Service, error) { - logger.Debug("New START") - defer logger.Debug("New END") + logger.Info("New START") + defer logger.Info("New END") capture := capture.New( clk, logger.Sublogger("capture"), @@ -130,8 +130,8 @@ func New( // Close releases all resources managed by data_manager. func (b *builtIn) Close(_ context.Context) error { - b.logger.Debug("Close START") - defer b.logger.Debug("Close END") + b.logger.Info("Close START") + defer b.logger.Info("Close END") b.mu.Lock() defer b.mu.Unlock() b.diskSummaryLogger.close() @@ -147,8 +147,8 @@ func (b *builtIn) Close(_ context.Context) error { // If automated sync is also enabled, calling Sync will upload the files, // regardless of whether or not is the scheduled time. func (b *builtIn) Sync(ctx context.Context, extra map[string]interface{}) error { - b.logger.Debug("Sync START") - defer b.logger.Debug("Sync END") + b.logger.Info("Sync START") + defer b.logger.Info("Sync END") b.mu.Lock() defer b.mu.Unlock() return b.sync.Sync(ctx, extra) @@ -171,8 +171,8 @@ func (b *builtIn) Sync(ctx context.Context, extra map[string]interface{}) error // If an error occurs after the first Reconfigure call, data capture & data sync will continue to function using the old config // until a successful Reconfigure call is made or Close is called. func (b *builtIn) Reconfigure(ctx context.Context, deps resource.Dependencies, conf resource.Config) error { - b.logger.Debug("Reconfigure START") - defer b.logger.Debug("Reconfigure END") + b.logger.Info("Reconfigure START") + defer b.logger.Info("Reconfigure END") c, err := resource.NativeConfig[*Config](conf) if err != nil { // If this error occurs it is due to the builtin.Config not being a native config which is a diff --git a/services/datamanager/builtin/config.go b/services/datamanager/builtin/config.go index 311fc5128f04..dc149bdd5b7d 100644 --- a/services/datamanager/builtin/config.go +++ b/services/datamanager/builtin/config.go @@ -1,12 +1,14 @@ package builtin import ( + "errors" "runtime" "go.viam.com/rdk/components/sensor" "go.viam.com/rdk/internal/cloud" "go.viam.com/rdk/services/datamanager/builtin/capture" datasync "go.viam.com/rdk/services/datamanager/builtin/sync" + "go.viam.com/rdk/utils" ) // Sync defaults. @@ -18,6 +20,10 @@ const ( // which is evaluated if the file deletion threshold has been reached. If `captureFileIndex % N == 0` // return true then the file will be deleted to free up space. defaultDeleteEveryNth = 5 + // defaultSyncIntervalMins is the sync interval that will be set if the config's sync_interval_mins is zero (including when it is unset) + defaultSyncIntervalMins = 0.1 + // syncIntervalMinsEpsilon is the value below which SyncIntervalMins is considered zero + syncIntervalMinsEpsilon = 0.00001 ) // Capture Defaults @@ -46,6 +52,18 @@ type Config struct { // Validate returns components which will be depended upon weakly due to the above matcher. func (c *Config) Validate(path string) ([]string, error) { + if c.SyncIntervalMins < 0 { + return nil, errors.New("sync_interval_mins can't be negative") + } + if c.FileLastModifiedMillis < 0 { + return nil, errors.New("file_last_modified_millis can't be negative") + } + if c.MaximumCaptureFileSizeBytes < 0 { + return nil, errors.New("maximum_capture_file_size_bytes can't be negative") + } + if c.DeleteEveryNthWhenDiskFull < 0 { + return nil, errors.New("delete_every_nth_when_disk_full can't be negative") + } return []string{cloud.InternalServiceName.String()}, nil } @@ -88,6 +106,12 @@ func (c *Config) syncConfig(syncSensor sensor.Sensor, syncSensorEnabled bool) da fileLastModifiedMillis = defaultFileLastModifiedMillis } c.FileLastModifiedMillis = fileLastModifiedMillis + + syncIntervalMins := c.SyncIntervalMins + if utils.Float64AlmostEqual(c.SyncIntervalMins, 0, syncIntervalMinsEpsilon) { + syncIntervalMins = defaultSyncIntervalMins + } + return datasync.Config{ AdditionalSyncPaths: c.AdditionalSyncPaths, Tags: c.Tags, @@ -98,7 +122,7 @@ func (c *Config) syncConfig(syncSensor sensor.Sensor, syncSensorEnabled bool) da MaximumNumSyncThreads: c.MaximumNumSyncThreads, ScheduledSyncDisabled: c.ScheduledSyncDisabled, SelectiveSyncerName: c.SelectiveSyncerName, - SyncIntervalMins: c.SyncIntervalMins, + SyncIntervalMins: syncIntervalMins, SelectiveSyncSensor: syncSensor, SelectiveSyncSensorEnabled: syncSensorEnabled, } diff --git a/services/datamanager/builtin/config_test.go b/services/datamanager/builtin/config_test.go index 7808030dda32..1f299483fdf3 100644 --- a/services/datamanager/builtin/config_test.go +++ b/services/datamanager/builtin/config_test.go @@ -73,6 +73,18 @@ func TestConfig(t *testing.T) { DeleteEveryNthWhenDiskFull: 5, FileLastModifiedMillis: 10000, MaximumNumSyncThreads: runtime.NumCPU() / 2, + SyncIntervalMins: 0.1, + }) + }) + + t.Run("returns a sync config with defaults when called on a config with SyncIntervalMins which is practically 0", func(t *testing.T) { + c := &Config{SyncIntervalMins: 0.000000000000000001} + test.That(t, c.syncConfig(nil, false), test.ShouldResemble, sync.Config{ + CaptureDir: viamCaptureDotDir, + DeleteEveryNthWhenDiskFull: 5, + FileLastModifiedMillis: 10000, + MaximumNumSyncThreads: runtime.NumCPU() / 2, + SyncIntervalMins: 0.1, }) }) t.Run("returns a sync config with overridden defaults when called on a full config", func(t *testing.T) { diff --git a/services/datamanager/builtin/sync/config.go b/services/datamanager/builtin/sync/config.go index 37545d1a7055..833bce9622f4 100644 --- a/services/datamanager/builtin/sync/config.go +++ b/services/datamanager/builtin/sync/config.go @@ -6,7 +6,6 @@ import ( "go.viam.com/rdk/components/sensor" "go.viam.com/rdk/logging" - "go.viam.com/rdk/utils" ) // Config is the sync config from builtin. @@ -89,7 +88,7 @@ type Config struct { } func (c Config) schedulerEnabled() bool { - configDisabled := c.ScheduledSyncDisabled || utils.Float64AlmostEqual(c.SyncIntervalMins, 0.0, 0.00001) + configDisabled := c.ScheduledSyncDisabled selectiveSyncerInvalid := c.SelectiveSyncSensorEnabled && c.SelectiveSyncSensor == nil return !configDisabled && !selectiveSyncerInvalid } @@ -135,7 +134,7 @@ func (c *Config) logDiff(o Config, logger logging.Logger) { } if c.FileLastModifiedMillis != o.FileLastModifiedMillis { - logger.Infof("file_last_modified_millis: old: %d, new: %Bd", c.FileLastModifiedMillis, o.FileLastModifiedMillis) + logger.Infof("file_last_modified_millis: old: %d, new: %d", c.FileLastModifiedMillis, o.FileLastModifiedMillis) } if c.MaximumNumSyncThreads != o.MaximumNumSyncThreads { @@ -167,8 +166,9 @@ func (c *Config) logDiff(o Config, logger logging.Logger) { if c.SelectiveSyncSensor != nil { oldName = c.SelectiveSyncSensor.Name().String() } + newName := "" - if c.SelectiveSyncSensor != nil { + if o.SelectiveSyncSensor != nil { newName = o.SelectiveSyncSensor.Name().String() } logger.Infof("SelectiveSyncSensor: old: %s, new: %s", oldName, newName) diff --git a/services/datamanager/builtin/sync/config_test.go b/services/datamanager/builtin/sync/config_test.go index 0eb857f7a783..f2eed96ae2a4 100644 --- a/services/datamanager/builtin/sync/config_test.go +++ b/services/datamanager/builtin/sync/config_test.go @@ -188,8 +188,8 @@ func TestConfig(t *testing.T) { }) t.Run("schedulerEnabled()", func(t *testing.T) { - t.Run("false by default", func(t *testing.T) { - test.That(t, Config{}.schedulerEnabled(), test.ShouldBeFalse) + t.Run("true by default", func(t *testing.T) { + test.That(t, Config{}.schedulerEnabled(), test.ShouldBeTrue) }) t.Run("false if ScheduledSyncDisabled", func(t *testing.T) { @@ -197,11 +197,6 @@ func TestConfig(t *testing.T) { test.That(t, Config{ScheduledSyncDisabled: true, SyncIntervalMins: 1.0}.schedulerEnabled(), test.ShouldBeFalse) }) - t.Run("false if SyncIntervalMins is almost 0", func(t *testing.T) { - test.That(t, Config{SyncIntervalMins: 0.0000001}.schedulerEnabled(), test.ShouldBeFalse) - test.That(t, Config{SelectiveSyncSensorEnabled: true, SelectiveSyncSensor: &inject.Sensor{}}.schedulerEnabled(), test.ShouldBeFalse) - }) - t.Run("false if SelectiveSyncSensorEnabled is true and SelectiveSyncSensor is nil", func(t *testing.T) { test.That(t, Config{SelectiveSyncSensorEnabled: true}.schedulerEnabled(), test.ShouldBeFalse) test.That(t, Config{SelectiveSyncSensorEnabled: true, SyncIntervalMins: 1.0}.schedulerEnabled(), test.ShouldBeFalse)