Fix task attempt reset (#4276)

Author: yycptt

service/history/queues/executable.go

@@ -212,10 +212,9 @@ func (e *executableImpl) Execute() (retErr error) {
 	e.taggedMetricsHandler = e.metricsHandler.WithTags(metricsTags...)
 
 	if isActive != e.lastActiveness {
-		// namespace did a failover, reset task attempt
-		e.Lock()
-		e.attempt = 0
-		e.Unlock()
+		// namespace did a failover,
+		// reset task attempt since the execution logic used will change
+		e.resetAttempt()
 	}
 	e.lastActiveness = isActive
 
@@ -524,6 +523,13 @@ func (e *executableImpl) updatePriority() {
 	}
 }
 
+func (e *executableImpl) resetAttempt() {
+	e.Lock()
+	defer e.Unlock()
+
+	e.attempt = 1
+}
+
 func (e *executableImpl) estimateTaskMetricTag() []metrics.Tag {
 	namespaceTag := metrics.NamespaceUnknownTag()
 	isActive := true

service/history/queues/executable_test.go

@@ -149,6 +149,20 @@ func (s *executableSuite) TestExecute_CallerInfo() {
 	s.NoError(executable.Execute())
 }
 
+func (s *executableSuite) TestExecuteHandleErr_ResetAttempt() {
+	executable := s.newTestExecutable()
+	s.mockExecutor.EXPECT().Execute(gomock.Any(), executable).Return(nil, true, errors.New("some random error"))
+	err := executable.Execute()
+	s.Error(err)
+	s.Error(executable.HandleErr(err))
+	s.Equal(2, executable.Attempt())
+
+	// isActive changed to false, should reset attempt
+	s.mockExecutor.EXPECT().Execute(gomock.Any(), executable).Return(nil, false, nil)
+	s.NoError(executable.Execute())
+	s.Equal(1, executable.Attempt())
+}
+
 func (s *executableSuite) TestExecuteHandleErr_Corrupted() {
 	executable := s.newTestExecutable()