Merge pull request #15036 from SimonRichardson/remove-batch-fsm-contr…

…oller-config

[JUJU-2416] Remove batch fsm controller config

agent/agent.go

@@ -282,10 +282,6 @@ type Config interface {
 	// performed after each write to the raft log.
 	NonSyncedWritesToRaftLog() bool
 
-	// BatchRaftFSM returns true if the raft calls should use the batching
-	// FSM.
-	BatchRaftFSM() bool
-
 	// AgentLogfileMaxSizeMB returns the maximum file size in MB of each
 	// agent/controller log file.
 	AgentLogfileMaxSizeMB() int
@@ -344,10 +340,6 @@ type configSetterOnly interface {
 	// SetNonSyncedWritesToRaftLog selects whether fsync calls are performed
 	// after each write to the raft log.
 	SetNonSyncedWritesToRaftLog(bool)
-
-	// SetBatchRaftFSM select whether raft should use the batching for writing
-	// to the FSM.
-	SetBatchRaftFSM(bool)
 }
 
 // LogFileName returns the filename for the Agent's log file.
@@ -422,7 +414,6 @@ type configInternal struct {
 	mongoMemoryProfile       string
 	jujuDBSnapChannel        string
 	nonSyncedWritesToRaftLog bool
-	batchRaftFSM             bool
 	agentLogfileMaxSizeMB    int
 	agentLogfileMaxBackups   int
 }
@@ -444,7 +435,6 @@ type AgentConfigParams struct {
 	MongoMemoryProfile       mongo.MemoryProfile
 	JujuDBSnapChannel        string
 	NonSyncedWritesToRaftLog bool
-	BatchRaftFSM             bool
 	AgentLogfileMaxSizeMB    int
 	AgentLogfileMaxBackups   int
 }
@@ -509,7 +499,6 @@ func NewAgentConfig(configParams AgentConfigParams) (ConfigSetterWriter, error)
 		mongoMemoryProfile:       configParams.MongoMemoryProfile.String(),
 		jujuDBSnapChannel:        configParams.JujuDBSnapChannel,
 		nonSyncedWritesToRaftLog: configParams.NonSyncedWritesToRaftLog,
-		batchRaftFSM:             configParams.BatchRaftFSM,
 		agentLogfileMaxSizeMB:    configParams.AgentLogfileMaxSizeMB,
 		agentLogfileMaxBackups:   configParams.AgentLogfileMaxBackups,
 	}
@@ -834,16 +823,6 @@ func (c *configInternal) SetNonSyncedWritesToRaftLog(nonSyncedWrites bool) {
 	c.nonSyncedWritesToRaftLog = nonSyncedWrites
 }
 
-// BatchRaftFSM implements Config.
-func (c *configInternal) BatchRaftFSM() bool {
-	return c.batchRaftFSM
-}
-
-// SetBatchRaftFSM implements configSetterOnly.
-func (c *configInternal) SetBatchRaftFSM(batchRaftFSM bool) {
-	c.batchRaftFSM = batchRaftFSM
-}
-
 // AgentLogfileMaxSizeMB implements Config.
 func (c *configInternal) AgentLogfileMaxSizeMB() int {
 	return c.agentLogfileMaxSizeMB

agent/agent_test.go

@@ -387,7 +387,6 @@ var attributeParams = agent.AgentConfigParams{
 	Model:                    testing.ModelTag,
 	JujuDBSnapChannel:        controller.DefaultJujuDBSnapChannel,
 	NonSyncedWritesToRaftLog: false,
-	BatchRaftFSM:             false,
 	AgentLogfileMaxSizeMB:    150,
 	AgentLogfileMaxBackups:   4,
 }
@@ -405,7 +404,6 @@ func (*suite) TestAttributes(c *gc.C) {
 	c.Assert(conf.UpgradedToVersion(), jc.DeepEquals, jujuversion.Current)
 	c.Assert(conf.JujuDBSnapChannel(), gc.Equals, "4.4/stable")
 	c.Assert(conf.NonSyncedWritesToRaftLog(), jc.IsFalse)
-	c.Assert(conf.BatchRaftFSM(), jc.IsFalse)
 	c.Assert(conf.AgentLogfileMaxSizeMB(), gc.Equals, 150)
 	c.Assert(conf.AgentLogfileMaxBackups(), gc.Equals, 4)
 }
@@ -716,15 +714,3 @@ func (*suite) TestSetSyncWritesToRaftLog(c *gc.C) {
 	nonSyncedWritesToRaftLog = conf.NonSyncedWritesToRaftLog()
 	c.Assert(nonSyncedWritesToRaftLog, jc.IsTrue, gc.Commentf("sync writes to raft log settings not updated"))
 }
-
-func (*suite) TestSetBatchRaftFSM(c *gc.C) {
-	conf, err := agent.NewAgentConfig(attributeParams)
-	c.Assert(err, jc.ErrorIsNil)
-
-	nonSyncedWritesToRaftLog := conf.BatchRaftFSM()
-	c.Assert(nonSyncedWritesToRaftLog, jc.IsFalse)
-
-	conf.SetBatchRaftFSM(true)
-	nonSyncedWritesToRaftLog = conf.BatchRaftFSM()
-	c.Assert(nonSyncedWritesToRaftLog, jc.IsTrue, gc.Commentf("batch raft FSM settings not updated"))
-}

agent/format-2.0.go

@@ -64,7 +64,6 @@ type format_2_0Serialization struct {
 	MongoMemoryProfile       string `yaml:"mongomemoryprofile,omitempty"`
 	JujuDBSnapChannel        string `yaml:"juju-db-snap-channel,omitempty"`
 	NonSyncedWritesToRaftLog bool   `yaml:"no-sync-writes-to-raft-log,omitempty"`
-	BatchRaftFSM             bool   `yaml:"batch-raft-fsm,omitempty"`
 }
 
 func init() {
@@ -117,7 +116,6 @@ func (formatter_2_0) unmarshal(data []byte) (*configInternal, error) {
 		agentLogfileMaxBackups: format.AgentLogfileMaxBackups,
 
 		nonSyncedWritesToRaftLog: format.NonSyncedWritesToRaftLog,
-		batchRaftFSM:             format.BatchRaftFSM,
 	}
 	if len(format.APIAddresses) > 0 {
 		config.apiDetails = &apiDetails{
@@ -187,7 +185,6 @@ func (formatter_2_0) marshal(config *configInternal) ([]byte, error) {
 		AgentLogfileMaxBackups: config.agentLogfileMaxBackups,
 
 		NonSyncedWritesToRaftLog: config.nonSyncedWritesToRaftLog,
-		BatchRaftFSM:             config.batchRaftFSM,
 	}
 	if config.servingInfo != nil {
 		format.ControllerCert = config.servingInfo.Cert

cmd/containeragent/initialize/config.go

@@ -126,10 +126,6 @@ func (c *configFromEnv) NonSyncedWritesToRaftLog() bool {
 	panic("not implemented")
 }
 
-func (c *configFromEnv) BatchRaftFSM() bool {
-	panic("not implemented")
-}
-
 func (c *configFromEnv) AgentLogfileMaxSizeMB() int {
 	panic("not implemented")
 }

cmd/jujud/agent/bootstrap.go

@@ -278,7 +278,6 @@ func (c *BootstrapCommand) Run(_ *cmd.Context) error {
 		}
 		agentConfig.SetJujuDBSnapChannel(args.ControllerConfig.JujuDBSnapChannel())
 		agentConfig.SetNonSyncedWritesToRaftLog(args.ControllerConfig.NonSyncedWritesToRaftLog())
-		agentConfig.SetBatchRaftFSM(args.ControllerConfig.BatchRaftFSM())
 		return nil
 	}); err != nil {
 		return fmt.Errorf("cannot write agent config: %v", err)

controller/config.go

@@ -227,9 +227,6 @@ const (
 	// when writing to the raft log by setting this value to true.
 	NonSyncedWritesToRaftLog = "non-synced-writes-to-raft-log"
 
-	// BatchRaftFSM allows the operator to batch raft FSM calls.
-	BatchRaftFSM = "batch-raft-fsm"
-
 	// MigrationMinionWaitMax is the maximum time that the migration-master
 	// worker will wait for agents to report for a migration phase when
 	// executing a model migration.
@@ -376,10 +373,6 @@ const (
 	// non-synced-writes-to-raft-log value. It is set to false by default.
 	DefaultNonSyncedWritesToRaftLog = false
 
-	// DefaultBatchRaftFSM is the default value for batch-raft-fsm value.
-	// It is set to false by default.
-	DefaultBatchRaftFSM = false
-
 	// DefaultMigrationMinionWaitMax is the default value for
 	DefaultMigrationMinionWaitMax = "15m"
 )
@@ -432,7 +425,6 @@ var (
 		MaxCharmStateSize,
 		MaxAgentStateSize,
 		NonSyncedWritesToRaftLog,
-		BatchRaftFSM,
 		MigrationMinionWaitMax,
 		ApplicationResourceDownloadLimit,
 		ControllerResourceDownloadLimit,
@@ -484,7 +476,6 @@ var (
 		MaxCharmStateSize,
 		MaxAgentStateSize,
 		NonSyncedWritesToRaftLog,
-		BatchRaftFSM,
 		MigrationMinionWaitMax,
 		ApplicationResourceDownloadLimit,
 		ControllerResourceDownloadLimit,
@@ -1025,14 +1016,6 @@ func (c Config) NonSyncedWritesToRaftLog() bool {
 	return DefaultNonSyncedWritesToRaftLog
 }
 
-// BatchRaftFSM returns true if raft should use batch writing to the FSM.
-func (c Config) BatchRaftFSM() bool {
-	if v, ok := c[BatchRaftFSM]; ok {
-		return v.(bool)
-	}
-	return DefaultBatchRaftFSM
-}
-
 // MigrationMinionWaitMax returns a duration for the maximum time that the
 // migration-master worker should wait for migration-minion reports during
 // phases of a model migration.
@@ -1380,7 +1363,6 @@ var configChecker = schema.FieldMap(schema.Fields{
 	MaxCharmStateSize:                schema.ForceInt(),
 	MaxAgentStateSize:                schema.ForceInt(),
 	NonSyncedWritesToRaftLog:         schema.Bool(),
-	BatchRaftFSM:                     schema.Bool(),
 	MigrationMinionWaitMax:           schema.String(),
 	ApplicationResourceDownloadLimit: schema.ForceInt(),
 	ControllerResourceDownloadLimit:  schema.ForceInt(),
@@ -1427,7 +1409,6 @@ var configChecker = schema.FieldMap(schema.Fields{
 	MaxCharmStateSize:                DefaultMaxCharmStateSize,
 	MaxAgentStateSize:                DefaultMaxAgentStateSize,
 	NonSyncedWritesToRaftLog:         DefaultNonSyncedWritesToRaftLog,
-	BatchRaftFSM:                     DefaultBatchRaftFSM,
 	MigrationMinionWaitMax:           DefaultMigrationMinionWaitMax,
 	ApplicationResourceDownloadLimit: schema.Omit,
 	ControllerResourceDownloadLimit:  schema.Omit,
@@ -1618,10 +1599,6 @@ Use "caas-image-repo" instead.`,
 		Type:        environschema.Tbool,
 		Description: `Do not perform fsync calls after appending entries to the raft log. Disabling sync improves performance at the cost of reliability`,
 	},
-	BatchRaftFSM: {
-		Type:        environschema.Tbool,
-		Description: `Allow raft to use batch writing to the FSM.`,
-	},
 	MigrationMinionWaitMax: {
 		Type:        environschema.Tstring,
 		Description: `The maximum during model migrations that the migration worker will wait for agents to report on phases of the migration`,

controller/config_test.go

@@ -370,12 +370,6 @@ var newConfigTests = []struct {
 			controller.NonSyncedWritesToRaftLog: "I live dangerously",
 		},
 		expectError: `non-synced-writes-to-raft-log: expected bool, got string\("I live dangerously"\)`,
-	}, {
-		about: "invalid batch-raft-fsm - string",
-		config: controller.Config{
-			controller.BatchRaftFSM: "I live dangerously",
-		},
-		expectError: `batch-raft-fsm: expected bool, got string\("I live dangerously"\)`,
 	}, {
 		about: "public-dns-address: expect string, got number",
 		config: controller.Config{

state/controller_test.go

@@ -62,7 +62,6 @@ func (s *ControllerSuite) TestControllerAndModelConfigInitialisation(c *gc.C) {
 		controller.MaxCharmStateSize,
 		controller.MaxAgentStateSize,
 		controller.NonSyncedWritesToRaftLog,
-		controller.BatchRaftFSM,
 		controller.MigrationMinionWaitMax,
 		controller.AgentLogfileMaxBackups,
 		controller.AgentLogfileMaxSize,

worker/agentconfigupdater/manifold.go

@@ -103,10 +103,6 @@ func Manifold(config ManifoldConfig) dependency.Manifold {
 			configNonSyncedWritesToRaftLog := controllerConfig.NonSyncedWritesToRaftLog()
 			nonSyncedWritesToRaftLogChanged := agentsNonSyncedWritesToRaftLog != configNonSyncedWritesToRaftLog
 
-			agentsBatchRaftFSM := agent.CurrentConfig().BatchRaftFSM()
-			configBatchRaftFSM := controllerConfig.BatchRaftFSM()
-			batchRaftFSMChanged := agentsBatchRaftFSM != configBatchRaftFSM
-
 			info, err := apiState.StateServingInfo()
 			if err != nil {
 				return nil, errors.Annotate(err, "getting state serving info")
@@ -137,10 +133,6 @@ func Manifold(config ManifoldConfig) dependency.Manifold {
 					logger.Debugf("setting non synced writes to raft log: %t => %t", agentsNonSyncedWritesToRaftLog, configNonSyncedWritesToRaftLog)
 					config.SetNonSyncedWritesToRaftLog(configNonSyncedWritesToRaftLog)
 				}
-				if batchRaftFSMChanged {
-					logger.Debugf("setting batch raft fsm: %t => %t", agentsBatchRaftFSM, configBatchRaftFSM)
-					config.SetBatchRaftFSM(configBatchRaftFSM)
-				}
 				return nil
 			})
 			if err != nil {
@@ -169,7 +161,6 @@ func Manifold(config ManifoldConfig) dependency.Manifold {
 				MongoProfile:             configMongoMemoryProfile,
 				JujuDBSnapChannel:        configJujuDBSnapChannel,
 				NonSyncedWritesToRaftLog: configNonSyncedWritesToRaftLog,
-				BatchRaftFSM:             configBatchRaftFSM,
 				Logger:                   config.Logger,
 			})
 		},

worker/agentconfigupdater/manifold_test.go

@@ -357,9 +357,6 @@ type mockConfig struct {
 
 	nonSyncedWritesToRaftLog    bool
 	nonSyncedWritesToRaftLogSet bool
-
-	batchRaftFSM    bool
-	batchRaftFSMSet bool
 }
 
 func (mc *mockConfig) Tag() names.Tag {
@@ -415,15 +412,6 @@ func (mc *mockConfig) SetNonSyncedWritesToRaftLog(nonSynced bool) {
 	mc.nonSyncedWritesToRaftLogSet = true
 }
 
-func (mc *mockConfig) BatchRaftFSM() bool {
-	return mc.batchRaftFSM
-}
-
-func (mc *mockConfig) SetBatchRaftFSM(batchRaftFSM bool) {
-	mc.batchRaftFSM = batchRaftFSM
-	mc.batchRaftFSMSet = true
-}
-
 func (mc *mockConfig) LogDir() string {
 	return "log-dir"
 }

worker/agentconfigupdater/worker.go

@@ -25,7 +25,6 @@ type WorkerConfig struct {
 	MongoProfile             mongo.MemoryProfile
 	JujuDBSnapChannel        string
 	NonSyncedWritesToRaftLog bool
-	BatchRaftFSM             bool
 	Logger                   Logger
 }
 
@@ -65,7 +64,6 @@ func NewWorker(config WorkerConfig) (worker.Worker, error) {
 		mongoProfile:             config.MongoProfile,
 		jujuDBSnapChannel:        config.JujuDBSnapChannel,
 		nonSyncedWritesToRaftLog: config.NonSyncedWritesToRaftLog,
-		batchRaftFSM:             config.BatchRaftFSM,
 	}
 	w.tomb.Go(func() error {
 		return w.loop(started)
@@ -108,10 +106,7 @@ func (w *agentConfigUpdater) onConfigChanged(topic string, data controllermsg.Co
 	nonSyncedWritesToRaftLog := data.Config.NonSyncedWritesToRaftLog()
 	nonSyncedWritesToRaftLogChanged := nonSyncedWritesToRaftLog != w.nonSyncedWritesToRaftLog
 
-	batchRaftFSM := data.Config.BatchRaftFSM()
-	batchRaftFSMChanged := batchRaftFSM != w.batchRaftFSM
-
-	changeDetected := mongoProfileChanged || jujuDBSnapChannelChanged || nonSyncedWritesToRaftLogChanged || batchRaftFSMChanged
+	changeDetected := mongoProfileChanged || jujuDBSnapChannelChanged || nonSyncedWritesToRaftLogChanged
 	if !changeDetected {
 		// Nothing to do, all good.
 		return
@@ -130,10 +125,6 @@ func (w *agentConfigUpdater) onConfigChanged(topic string, data controllermsg.Co
 			w.config.Logger.Debugf("setting no sync writes to raft log: %t => %t", w.nonSyncedWritesToRaftLog, nonSyncedWritesToRaftLog)
 			setter.SetNonSyncedWritesToRaftLog(nonSyncedWritesToRaftLog)
 		}
-		if batchRaftFSMChanged {
-			w.config.Logger.Debugf("setting batch raft fsm: %t => %t", w.batchRaftFSM, batchRaftFSM)
-			setter.SetBatchRaftFSM(batchRaftFSM)
-		}
 		return nil
 	})
 	if err != nil {

worker/agentconfigupdater/worker_test.go

@@ -43,7 +43,6 @@ func (s *WorkerSuite) SetUpTest(c *gc.C) {
 			profile:                  controller.DefaultMongoMemoryProfile,
 			snapChannel:              controller.DefaultJujuDBSnapChannel,
 			nonSyncedWritesToRaftLog: controller.DefaultNonSyncedWritesToRaftLog,
-			batchRaftFSM:             controller.DefaultBatchRaftFSM,
 		},
 	}
 	s.config = agentconfigupdater.WorkerConfig{
@@ -52,15 +51,13 @@ func (s *WorkerSuite) SetUpTest(c *gc.C) {
 		MongoProfile:             controller.DefaultMongoMemoryProfile,
 		JujuDBSnapChannel:        controller.DefaultJujuDBSnapChannel,
 		NonSyncedWritesToRaftLog: controller.DefaultNonSyncedWritesToRaftLog,
-		BatchRaftFSM:             controller.DefaultBatchRaftFSM,
 		Logger:                   s.logger,
 	}
 	s.initialConfigMsg = controllermsg.ConfigChangedMessage{
 		Config: controller.Config{
 			controller.MongoMemoryProfile:       controller.DefaultMongoMemoryProfile,
 			controller.JujuDBSnapChannel:        controller.DefaultJujuDBSnapChannel,
 			controller.NonSyncedWritesToRaftLog: controller.DefaultNonSyncedWritesToRaftLog,
-			controller.BatchRaftFSM:             controller.DefaultBatchRaftFSM,
 		},
 	}
 }
@@ -219,34 +216,3 @@ func (s *WorkerSuite) TestUpdateSyncWritesToRaftLog(c *gc.C) {
 
 	c.Assert(err, gc.Equals, jworker.ErrRestartAgent)
 }
-
-func (s *WorkerSuite) TestUpdateBatchRaftFSM(c *gc.C) {
-	w, err := agentconfigupdater.NewWorker(s.config)
-	c.Assert(w, gc.NotNil)
-	c.Check(err, jc.ErrorIsNil)
-
-	newConfig := s.initialConfigMsg
-	handled, err := s.hub.Publish(controllermsg.ConfigChanged, newConfig)
-	c.Assert(err, jc.ErrorIsNil)
-	select {
-	case <-pubsub.Wait(handled):
-	case <-time.After(testing.LongWait):
-		c.Fatalf("event not handled")
-	}
-
-	// No sync flag is the same, worker still alive.
-	workertest.CheckAlive(c, w)
-
-	newConfig.Config[controller.BatchRaftFSM] = !controller.DefaultBatchRaftFSM
-	handled, err = s.hub.Publish(controllermsg.ConfigChanged, newConfig)
-	c.Assert(err, jc.ErrorIsNil)
-	select {
-	case <-pubsub.Wait(handled):
-	case <-time.After(testing.LongWait):
-		c.Fatalf("event not handled")
-	}
-
-	err = workertest.CheckKilled(c, w)
-
-	c.Assert(err, gc.Equals, jworker.ErrRestartAgent)
-}