Change smart retry to disabled by default

uber-archive · Aug 30, 2017 · 74cf8f9 · 74cf8f9
1 parent eac12a9
commit 74cf8f9
Show file tree

Hide file tree

Showing 7 changed files with 51 additions and 35 deletions.
diff --git a/clients/metadata/metadata_cassandra_test.go b/clients/metadata/metadata_cassandra_test.go
@@ -1756,7 +1756,7 @@ func (s *CassandraSuite) TestConsumerGroupCRUD() {
 
 	options := make(map[string]string)
 	options[common.FlagDisableNackThrottling] = "true"
-	options[common.FlagDisableSmartRetry] = "true"
+	options[common.FlagEnableSmartRetry] = "false"
 
 	cgName := s.generateName("/foo/bar_consumer")
 
@@ -1835,7 +1835,7 @@ func (s *CassandraSuite) TestConsumerGroupCRUD() {
 		assertConsumerGroupsEqual(s, expectedCG, gotCG)
 
 		options[common.FlagDisableNackThrottling] = "false"
-		options[common.FlagDisableSmartRetry] = "false"
+		options[common.FlagEnableSmartRetry] = "true"
 
 		readReq.ConsumerGroupUUID = common.StringPtr(gotCG.GetConsumerGroupUUID())
 		gotCG, err = s.client.ReadConsumerGroupByUUID(nil, readReq)
@@ -2113,7 +2113,7 @@ func (s *CassandraSuite) TestListAllConsumerGroups() {
 
 	options := make(map[string]string)
 	options[common.FlagDisableNackThrottling] = "true"
-	options[common.FlagDisableSmartRetry] = "true"
+	options[common.FlagEnableSmartRetry] = "false"
 
 	for i := 0; i < 10; i++ {
 		name := s.generateName(fmt.Sprintf("foobar-consumer-%v", i))

diff --git a/cmd/tools/common/lib.go b/cmd/tools/common/lib.go
@@ -225,8 +225,8 @@ func SetCommonCommands(
 							Usage: "Disable nack throttling for consumer group",
 						},
 						cli.BoolFlag{
-							Name:  common.FlagDisableSmartRetry,
-							Usage: "Disable smart retry for consumer group",
+							Name:  common.FlagEnableSmartRetry,
+							Usage: "Enable smart retry for consumer group",
 						},
 					},
 					Action: func(c *cli.Context) {
@@ -379,8 +379,8 @@ func SetCommonCommands(
 							Usage: "Disable nack throttling for consumer group",
 						},
 						cli.BoolFlag{
-							Name:  common.FlagDisableSmartRetry,
-							Usage: "Disable smart retry for consumer group",
+							Name:  common.FlagEnableSmartRetry,
+							Usage: "Enable smart retry for consumer group",
 						},
 					},
 					Action: func(c *cli.Context) {

diff --git a/common/constants.go b/common/constants.go
@@ -75,6 +75,6 @@ const (
 	// FlagDisableNackThrottling is the flag string for disabling Nack throttling
 	FlagDisableNackThrottling = "disable_nack_throttling"
 
-	// FlagDisableSmartRetry is the flag string for disabling smart retry
-	FlagDisableSmartRetry = "disable_smart_retry"
+	// FlagEnableSmartRetry is the flag string for enabling smart retry
+	FlagEnableSmartRetry = "enable_smart_retry"
 )
diff --git a/services/frontendhost/frontend_test.go b/services/frontendhost/frontend_test.go
@@ -829,7 +829,7 @@ func (s *FrontendHostSuite) TestFrontendHostCreateConsumerGroup() {
 
 	options := make(map[string]string)
 	options[common.FlagDisableNackThrottling] = "true"
-	options[common.FlagDisableSmartRetry] = "true"
+	options[common.FlagEnableSmartRetry] = "false"
 
 	req := c.NewCreateConsumerGroupRequest()
 	req.DestinationPath = common.StringPtr(testPath)
@@ -1135,7 +1135,7 @@ func (s *FrontendHostSuite) TestFrontendHostUpdateConsumerGroup() {
 
 	options := make(map[string]string)
 	options[common.FlagDisableNackThrottling] = "true"
-	options[common.FlagDisableSmartRetry] = "true"
+	options[common.FlagEnableSmartRetry] = "false"
 
 	req := new(c.UpdateConsumerGroupRequest)
 	req.DestinationPath = common.StringPtr(testPath)

diff --git a/services/outputhost/messagecache.go b/services/outputhost/messagecache.go
@@ -1075,14 +1075,16 @@ func (msgCache *cgMsgCache) checkTimer() {
 
 func (msgCache *cgMsgCache) isStalled() bool {
 	var m3St m3HealthState
-	var smartRetryDisabled bool
+	var smartRetryEnabled bool
 
-	smartRetryDisabledFlag, _ := msgCache.cgCache.cachedCGDesc.Options[common.FlagDisableSmartRetry]
+	smartRetryEnabledFlag, ok := msgCache.cgCache.cachedCGDesc.Options[common.FlagEnableSmartRetry]
+	if ok && smartRetryEnabledFlag == "true" {
+		smartRetryEnabled = true
+	}
 
 	if atomic.LoadInt32(&msgCache.cgCache.dlqMerging) > 0 ||
-		strings.Contains(msgCache.GetOwnerEmail(), SmartRetryDisableString) ||
-		smartRetryDisabledFlag == "true" {
-		smartRetryDisabled = true
+		strings.Contains(msgCache.GetOwnerEmail(), SmartRetryDisableString) {
+		smartRetryEnabled = false
 	}
 
 	now := common.Now()
@@ -1188,7 +1190,7 @@ func (msgCache *cgMsgCache) isStalled() bool {
 	msgCache.consumerM3Client.UpdateGauge(metrics.ConsConnectionScope, metrics.OutputhostCGHealthState, int64(m3St))
 	msgCache.consumerM3Client.UpdateGauge(metrics.ConsConnectionScope, metrics.OutputhostCGOutstandingDeliveries, int64(msgCache.countStateDelivered+msgCache.countStateEarlyNACK))
 
-	if smartRetryDisabled {
+	if !smartRetryEnabled {
 		if stalled {
 			msgCache.lclLg.Warn("Smart Retry disabled while consumer group stalled")
 		}

diff --git a/services/outputhost/messagecache_test.go b/services/outputhost/messagecache_test.go
@@ -56,7 +56,7 @@ func (s *MessageCacheSuite) SetupTest() {
 	cgStatus := shared.ConsumerGroupStatus_ENABLED
 
 	options := make(map[string]string)
-	options[common.FlagDisableSmartRetry] = "true"
+	options[common.FlagEnableSmartRetry] = "false"
 
 	cgDesc := &shared.ConsumerGroupDescription{
 		ConsumerGroupUUID:  common.StringPtr(uuid.New()),

diff --git a/tools/common/lib.go b/tools/common/lib.go
@@ -504,14 +504,12 @@ func CreateConsumerGroupSecure(
 
 	options := make(map[string]string)
 
-	disableNackThrottling := c.Bool(common.FlagDisableNackThrottling)
-	if disableNackThrottling == true {
+	if c.Bool(common.FlagDisableNackThrottling) == true {
 		options[common.FlagDisableNackThrottling] = "true"
 	}
 
-	disableSmartRetry := c.Bool(common.FlagDisableSmartRetry)
-	if disableSmartRetry == true {
-		options[common.FlagDisableSmartRetry] = "true"
+	if c.Bool(common.FlagEnableSmartRetry) == true {
+		options[common.FlagEnableSmartRetry] = "true"
 	}
 
 	desc, err := cClient.CreateConsumerGroup(&cherami.CreateConsumerGroupRequest{
@@ -579,6 +577,16 @@ func getCgZoneConfigs(c *cli.Context, mClient mcli.Client, cliHelper common.CliH
 	return zoneConfigs
 }
 
+func getCgFromMedatada(mClient mcli.Client, path string, name string) *shared.ConsumerGroupDescription {
+	cg, err := mClient.ReadConsumerGroup(&shared.ReadConsumerGroupRequest{
+		DestinationPath:   common.StringPtr(path),
+		ConsumerGroupName: common.StringPtr(name),
+	})
+	ExitIfError(err)
+
+	return cg
+}
+
 // UpdateConsumerGroup updates the consumer group
 func UpdateConsumerGroup(c *cli.Context, cliHelper common.CliHelper, serviceName string) {
 	UpdateConsumerGroupSecure(c, cliHelper, serviceName, nil)
@@ -639,7 +647,7 @@ func UpdateConsumerGroupSecure(
 		OwnerEmail:                 getIfSetString(c, `owner_email`, &setCount),
 		ActiveZone:                 getIfSetString(c, `active_zone`, &setCount),
 		ZoneConfigs:                getIfSetCgZoneConfig(c, mClient, cliHelper, path, &setCount),
-		Options:                    getIfSetOptions(c, &setCount),
+		Options:                    getIfSetOptions(c, mClient, path, name, &setCount),
 	}
 
 	if c.IsSet(`status`) {
@@ -1646,21 +1654,27 @@ func getIfSetCgZoneConfig(c *cli.Context, mClient mcli.Client, cliHelper common.
 	return
 }
 
-func getIfSetOptions(c *cli.Context, setCount *int) (options map[string]string) {
-	if c.IsSet(common.FlagDisableNackThrottling) || c.IsSet(common.FlagDisableSmartRetry) {
-		disableNackThrottling := "false"
-		if c.Bool(common.FlagDisableNackThrottling) == true {
-			disableNackThrottling = "true"
+func getIfSetOptions(c *cli.Context, mClient mcli.Client, path string, name string, setCount *int) (options map[string]string) {
+	if c.IsSet(common.FlagDisableNackThrottling) || c.IsSet(common.FlagEnableSmartRetry) {
+		cg := getCgFromMedatada(mClient, path, name)
+		options = cg.Options
+
+		if c.IsSet(common.FlagDisableNackThrottling) {
+			if c.Bool(common.FlagDisableNackThrottling) == true {
+				options[common.FlagDisableNackThrottling] = "true"
+			} else {
+				options[common.FlagDisableNackThrottling] = "false"
+			}
 		}
 
-		disableSmartRetry := "false"
-		if c.Bool(common.FlagDisableSmartRetry) == true {
-			disableSmartRetry = "true"
+		if c.IsSet(common.FlagEnableSmartRetry) {
+			if c.Bool(common.FlagEnableSmartRetry) == true {
+				options[common.FlagEnableSmartRetry] = "true"
+			} else {
+				options[common.FlagEnableSmartRetry] = "false"
+			}
 		}
 
-		options = make(map[string]string)
-		options[common.FlagDisableNackThrottling] = disableNackThrottling
-		options[common.FlagDisableSmartRetry] = disableSmartRetry
 		*setCount++
 		return options
 	}