Fix history scavenger bug on delete mutable state (#3588)

* Check workflow state in scavenger * Log deleted workflow as record
temporalio · Nov 15, 2022 · f103b19 · f103b19
1 parent ec332ef
commit f103b19
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 1 deletion.
diff --git a/service/worker/scanner/executions/task.go b/service/worker/scanner/executions/task.go
@@ -269,7 +269,7 @@ func printValidationResult(
 			metrics.FailureTag(result.failureType),
 		).IncCounter(metrics.ScavengerValidationFailuresCount)
 
-		logger.Error(
+		logger.Info(
 			"validation failed for execution.",
 			tag.WorkflowNamespaceID(mutableState.GetExecutionInfo().GetNamespaceId()),
 			tag.WorkflowID(mutableState.GetExecutionInfo().GetWorkflowId()),

diff --git a/service/worker/scanner/history/scavenger.go b/service/worker/scanner/history/scavenger.go
@@ -34,6 +34,7 @@ import (
 	"go.temporal.io/sdk/activity"
 
 	"go.temporal.io/server/api/adminservice/v1"
+	"go.temporal.io/server/api/enums/v1"
 	"go.temporal.io/server/api/historyservice/v1"
 	persistencepb "go.temporal.io/server/api/persistence/v1"
 	"go.temporal.io/server/common"
@@ -348,6 +349,11 @@ func (s *Scavenger) cleanUpWorkflowPastRetention(
 	ctx context.Context,
 	mutableState *persistencepb.WorkflowMutableState,
 ) error {
+	if mutableState.GetExecutionState().GetState() != enums.WORKFLOW_EXECUTION_STATE_COMPLETED {
+		// Skip running workflow
+		return nil
+	}
+
 	executionInfo := mutableState.GetExecutionInfo()
 	ns, err := s.registry.GetNamespaceByID(namespace.ID(executionInfo.GetNamespaceId()))
 	switch err.(type) {
@@ -379,7 +385,13 @@ func (s *Scavenger) cleanUpWorkflowPastRetention(
 				tag.WorkflowID(executionInfo.GetWorkflowId()),
 				tag.WorkflowRunID(mutableState.GetExecutionState().GetRunId()),
 			)
+			return nil
 		}
+		s.logger.Info("Delete workflow data past retention via history scavenger",
+			tag.WorkflowNamespace(ns.Name().String()),
+			tag.WorkflowID(executionInfo.GetWorkflowId()),
+			tag.WorkflowRunID(mutableState.GetExecutionState().GetRunId()),
+		)
 	}
 	return nil
 }

diff --git a/service/worker/scanner/history/scavenger_test.go b/service/worker/scanner/history/scavenger_test.go
@@ -38,6 +38,7 @@ import (
 
 	"go.temporal.io/server/api/adminservice/v1"
 	"go.temporal.io/server/api/adminservicemock/v1"
+	enumsspb "go.temporal.io/server/api/enums/v1"
 	"go.temporal.io/server/api/historyservice/v1"
 	"go.temporal.io/server/api/historyservicemock/v1"
 	persistencepb "go.temporal.io/server/api/persistence/v1"
@@ -598,6 +599,7 @@ func (s *ScavengerTestSuite) TestDeleteWorkflowAfterRetention() {
 			},
 			ExecutionState: &persistencepb.WorkflowExecutionState{
 				RunId: "runID2",
+				State: enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED,
 			},
 		},
 	}
@@ -610,6 +612,7 @@ func (s *ScavengerTestSuite) TestDeleteWorkflowAfterRetention() {
 			},
 			ExecutionState: &persistencepb.WorkflowExecutionState{
 				RunId: "runID4",
+				State: enumsspb.WORKFLOW_EXECUTION_STATE_COMPLETED,
 			},
 		},
 	}