Skip to content

Commit

Permalink
better logging
Browse files Browse the repository at this point in the history
  • Loading branch information
nicksanford committed Sep 24, 2024
1 parent dd20651 commit 5a20c35
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 8 deletions.
2 changes: 1 addition & 1 deletion services/datamanager/builtin/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func (b *builtIn) Reconfigure(ctx context.Context, deps resource.Dependencies, c
}

syncSensor, syncSensorEnabled := syncSensorFromDeps(c.SelectiveSyncerName, deps, b.logger)
syncConfig := c.syncConfig(syncSensor, syncSensorEnabled)
syncConfig := c.syncConfig(syncSensor, syncSensorEnabled, b.logger)

b.mu.Lock()
defer b.mu.Unlock()
Expand Down
7 changes: 5 additions & 2 deletions services/datamanager/builtin/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"go.viam.com/rdk/components/sensor"
"go.viam.com/rdk/internal/cloud"
"go.viam.com/rdk/logging"
"go.viam.com/rdk/services/datamanager/builtin/capture"
datasync "go.viam.com/rdk/services/datamanager/builtin/sync"
"go.viam.com/rdk/utils"
Expand All @@ -23,7 +24,7 @@ const (
// 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
syncIntervalMinsEpsilon = 0.0001
)

// Capture Defaults
Expand Down Expand Up @@ -91,7 +92,7 @@ func (c *Config) captureConfig() capture.Config {
}
}

func (c *Config) syncConfig(syncSensor sensor.Sensor, syncSensorEnabled bool) datasync.Config {
func (c *Config) syncConfig(syncSensor sensor.Sensor, syncSensorEnabled bool, logger logging.Logger) datasync.Config {
newMaxSyncThreadValue := runtime.NumCPU() / 2
if c.MaximumNumSyncThreads != 0 {
newMaxSyncThreadValue = c.MaximumNumSyncThreads
Expand All @@ -113,6 +114,8 @@ func (c *Config) syncConfig(syncSensor sensor.Sensor, syncSensorEnabled bool) da
syncIntervalMins := c.SyncIntervalMins
if utils.Float64AlmostEqual(c.SyncIntervalMins, 0, syncIntervalMinsEpsilon) {
syncIntervalMins = defaultSyncIntervalMins
logger.Infof("sync_interval_mins set to %f which is below allowed minimum of %f, setting to default of %f",
c.SyncIntervalMins, syncIntervalMinsEpsilon, defaultSyncIntervalMins)
}

return datasync.Config{
Expand Down
8 changes: 5 additions & 3 deletions services/datamanager/builtin/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"go.viam.com/test"

"go.viam.com/rdk/internal/cloud"
"go.viam.com/rdk/logging"
"go.viam.com/rdk/services/datamanager/builtin/capture"
"go.viam.com/rdk/services/datamanager/builtin/sync"
"go.viam.com/rdk/testutils/inject"
Expand All @@ -28,6 +29,7 @@ var fullConfig = &Config{
}

func TestConfig(t *testing.T) {
logger := logging.NewTestLogger(t)
t.Run("Validate ", func(t *testing.T) {
type testCase struct {
name string
Expand Down Expand Up @@ -116,7 +118,7 @@ func TestConfig(t *testing.T) {
t.Run("syncConfig())", func(t *testing.T) {
t.Run("returns a sync config with defaults when called on an empty config", func(t *testing.T) {
c := &Config{}
test.That(t, c.syncConfig(nil, false), test.ShouldResemble, sync.Config{
test.That(t, c.syncConfig(nil, false, logger), test.ShouldResemble, sync.Config{
CaptureDir: viamCaptureDotDir,
DeleteEveryNthWhenDiskFull: 5,
FileLastModifiedMillis: 10000,
Expand All @@ -127,7 +129,7 @@ func TestConfig(t *testing.T) {

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{
test.That(t, c.syncConfig(nil, false, logger), test.ShouldResemble, sync.Config{
CaptureDir: viamCaptureDotDir,
DeleteEveryNthWhenDiskFull: 5,
FileLastModifiedMillis: 10000,
Expand All @@ -137,7 +139,7 @@ func TestConfig(t *testing.T) {
})
t.Run("returns a sync config with overridden defaults when called on a full config", func(t *testing.T) {
s := &inject.Sensor{}
test.That(t, fullConfig.syncConfig(s, true), test.ShouldResemble, sync.Config{
test.That(t, fullConfig.syncConfig(s, true, logger), test.ShouldResemble, sync.Config{
AdditionalSyncPaths: []string{"/tmp/a", "/tmp/b"},
CaptureDir: "/tmp/some/path",
CaptureDisabled: true,
Expand Down
9 changes: 7 additions & 2 deletions services/datamanager/builtin/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,14 @@ func (s *Sync) Reconfigure(_ context.Context, config Config, cloudConnSvc cloud.
// to execute, don't stop workers
return
}
// config changed... stop workers
s.statsWorker.reconfigure(s.atomicUploadStats, syncStatsLogInterval)
s.config.logDiff(config, s.logger)

// config changed... stop workers
if s.config.schedulerEnabled() && !s.config.Equal(Config{}) {
// only log if the pool was previously started
s.logger.Info("stopping sync worker pool")
}
s.configCancelFunc()
s.FileDeletingWorkers.Stop()
s.Scheduler.Stop()
Expand Down Expand Up @@ -183,7 +187,7 @@ func (s *Sync) Reconfigure(_ context.Context, config Config, cloudConnSvc cloud.
}
}

// Close releases all resources managed by data_manager.
// Close releases all resources managed by data sync.
func (s *Sync) Close() {
s.configCancelFunc()
s.statsWorker.close()
Expand Down Expand Up @@ -309,6 +313,7 @@ func newCloudConn(
func (s *Sync) startWorkers(config Config) {
numThreads := config.MaximumNumSyncThreads
s.MaxSyncThreads = numThreads
s.logger.Infof("starting sync worker pool of size: %d", numThreads)
for i := 0; i < numThreads; i++ {
s.workersWg.Add(1)
goutils.ManagedGo(func() { s.runWorker(config) }, s.workersWg.Done)
Expand Down

0 comments on commit 5a20c35

Please sign in to comment.