Fix regression with GetWorkflow (need to check if execution row exist…

…s before locking it; otherwise return EntityNotExists in order to pass DeleteWorkflow test)
kumar-sundaram · Aug 3, 2018 · ef41d12 · ef41d12
1 parent fd485d9
commit ef41d12
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 4 deletions.
diff --git a/common/persistence/persistence-tests/matchingPersistence_test.go b/common/persistence/persistence-tests/matchingPersistence_test.go
@@ -1225,7 +1225,7 @@ func (s *matchingPersistenceSuite) TestWorkflowMutableState_Timers() {
 	s.Equal(1, len(state.TimerInfos))
 	s.Equal(int64(3345), state.TimerInfos[timerID].Version)
 	s.Equal(timerID, state.TimerInfos[timerID].TimerID)
-	s.Equal(currentTime.Unix(), state.TimerInfos[timerID].ExpiryTime.Unix())
+	s.Equal(currentTime.Unix(), state.TimerInfos[timerID].ExpiryTime.Unix()) // flakey
 	s.Equal(int64(2), state.TimerInfos[timerID].TaskID)
 	s.Equal(int64(5), state.TimerInfos[timerID].StartedID)
 

diff --git a/common/persistence/sql/sqlMatchingPersistence.go b/common/persistence/sql/sqlMatchingPersistence.go
@@ -527,8 +527,15 @@ func (m *sqlMatchingManager) GetWorkflowExecution(request *persistence.GetWorkfl
 
 	if _, err := lockNextEventID(tx, m.shardID, request.DomainID, *request.Execution.WorkflowId, *request.Execution.RunId);
 	err != nil {
-		return nil, &workflow.InternalServiceError{
-			Message: fmt.Sprintf("GetWorkflowExecution operation failed. Failed to write-lock executions row. Error: %v", err),
+		switch err.(type) {
+		case *workflow.EntityNotExistsError:
+			return nil, &workflow.EntityNotExistsError{
+				Message: fmt.Sprintf("GetWorkflowExecution operation failed. Error: %v", err),
+			}
+		default:
+			return nil, &workflow.InternalServiceError{
+				Message: fmt.Sprintf("GetWorkflowExecution operation failed. Failed to write-lock executions row. Error: %v", err),
+			}
 		}
 	}
 
@@ -1188,6 +1195,11 @@ func lockAndCheckNextEventID(tx *sqlx.Tx, shardID int, domainID, workflowID, run
 func lockNextEventID(tx *sqlx.Tx, shardID int, domainID, workflowID, runID string) (*int64, error) {
 	var nextEventID int64
 	if err := tx.Get(&nextEventID, lockAndCheckNextEventIdSQLQuery, shardID, domainID, workflowID, runID); err != nil {
+		if err == sql.ErrNoRows {
+			return nil, &workflow.EntityNotExistsError{
+				Message: fmt.Sprintf("Failed to lock executions row with (shard, domain, workflow, run) = (%v,%v,%v,%v) which does not exist.", shardID, domainID, workflowID, runID),
+			}
+		}
 		return nil, &workflow.InternalServiceError{
 			Message: fmt.Sprintf("Failed to lock executions row. Error: %v", err),
 		}

diff --git a/common/persistence/sql/workflowStateNonMaps.go b/common/persistence/sql/workflowStateNonMaps.go
@@ -114,4 +114,5 @@ func getSignalsRequested(tx *sqlx.Tx,
 			ret[s] = struct{}{}
 		}
 		return ret, nil
-}
+}
+