Files
Lina JodoinandGitHub 2171c0f82a Fix: RespondWorkflowTaskCompleted should the workflow termination event into the same event batch as the workflowtask failure event (#6304)
## What changed?
We [previously updated
RespondWorkflowTaskCompleted](https://github.com/temporalio/temporal/pull/6180)
to terminate on certain non-retryable errors (payload size exceeded)
instead of fail. Termination indicates that the failure was sourced from
the service, rather than the customer code/SDK. The change inadvertedly
resulted in the wrong `CompletionEventBatchID` being written into
mutable state, since both `failWorkflowTask` and `TerminateWorkflow`
allocate a new event batch. `CompletionEventBatchID` should be set to
the batch started in `failWorkflowTask`, but it was overridden from
`TerminateWorkflow`.

When replication to another cluster was involved, or the events cache is
otherwise evicted, `MutableState.GetCompletionEvent()` was no longer
able to load the failed event, as the `CompletionEventBatchID` used to
load from persistence is past the WFT failure event. This caused active
transfer tasks involving the terminated workflow to be sent to the DLQ
on secondary/standby clusters.

This fix:
- correctly writes the right event batch ID into the workflow
termination event by using `AddWorkflowExecutionTerminatedEvent` over
`TerminateWorkflow`
- updates `GetCompletionEvent()` to handle internal errors for
terminated workflows by iterating through the last batch of events via
persistence

## How did you test it?
Set up a local temporal server, and a simple test worker/workflow. The
test workflow simply launches a child workflow and awaits the result;
the child workflow returns an output with a large (2MB) payload, to
trigger termination.

✅ Validating test case:
- start with unpatched temporal
- run test workflow, observe termination
- run `tdbg w d` and observe an incorrect `completionEventBatchId`
- restart server
- run `tdbg w rt` to refresh tasks
- observe `"error":"unable to get workflow completion event"` message in
logs

✅  Validating backfill for corrupted `completionEventBatchId` workflows:
- (continue from "Validated test case" steps)
- stop unpatched temporal server
- start patched temporal server
- run `tdbg w rt` to refresh tasks
- observe server no longer shows error message/makes progress

✅  Validating newly-terminated workflows:
- start with patched temporal server
- run test workflow, observe termination
- run `tdbg w d` and observe the correct `completionEventBatchId`,
ensures it matches the `WorkflowTaskFailed` event ID

## Potential risks
- introduces new reverse-history search logic into
`GetCompletionEvent()` for edge cases which previously wasn't there;
tried to mitigate impact by only requesting a single page + batch of
results
- once again changes the `CompletionEventBatchID` for workflow
termination events, which may cause transfer task errors if incorrect

## Documentation

## Is hotfix candidate?
Yes
2024-07-17 22:42:28 +00:00

197 lines
5.9 KiB
Go

// The MIT License
//
// Copyright (c) 2020 Temporal Technologies Inc. All rights reserved.
//
// Copyright (c) 2020 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.
package workflow
import (
commonpb "go.temporal.io/api/common/v1"
enumspb "go.temporal.io/api/enums/v1"
historypb "go.temporal.io/api/history/v1"
"go.temporal.io/api/serviceerror"
workflowpb "go.temporal.io/api/workflow/v1"
enumsspb "go.temporal.io/server/api/enums/v1"
"go.temporal.io/server/common/clock"
"go.temporal.io/server/common/primitives/timestamp"
"go.temporal.io/server/internal/effect"
"go.temporal.io/server/service/history/consts"
)
func failWorkflowTask(
mutableState MutableState,
workflowTask *WorkflowTaskInfo,
workflowTaskFailureCause enumspb.WorkflowTaskFailedCause,
) (*historypb.HistoryEvent, error) {
// IMPORTANT: wtFailedEvent can be nil under some circumstances. Specifically, if WT is transient.
wtFailedEvent, err := mutableState.AddWorkflowTaskFailedEvent(
workflowTask,
workflowTaskFailureCause,
nil,
consts.IdentityHistoryService,
nil,
"",
"",
"",
0,
)
if err != nil {
return nil, err
}
mutableState.FlushBufferedEvents()
return wtFailedEvent, nil
}
func ScheduleWorkflowTask(
mutableState MutableState,
) error {
if mutableState.HasPendingWorkflowTask() {
return nil
}
_, err := mutableState.AddWorkflowTaskScheduledEvent(false, enumsspb.WORKFLOW_TASK_TYPE_NORMAL)
if err != nil {
return serviceerror.NewInternal("Failed to add workflow task scheduled event.")
}
return nil
}
func TimeoutWorkflow(
mutableState MutableState,
retryState enumspb.RetryState,
continuedRunID string,
) error {
// Check TerminateWorkflow comment bellow.
eventBatchFirstEventID := mutableState.GetNextEventID()
if workflowTask := mutableState.GetStartedWorkflowTask(); workflowTask != nil {
wtFailedEvent, err := failWorkflowTask(
mutableState,
workflowTask,
enumspb.WORKFLOW_TASK_FAILED_CAUSE_FORCE_CLOSE_COMMAND,
)
if err != nil {
return err
}
if wtFailedEvent != nil {
eventBatchFirstEventID = wtFailedEvent.GetEventId()
}
}
_, err := mutableState.AddTimeoutWorkflowEvent(
eventBatchFirstEventID,
retryState,
continuedRunID,
)
return err
}
// TerminateWorkflow will write a WorkflowExecutionTerminated event with a fresh
// batch ID. Do not use for situations where the WorkflowExecutionTerminated
// event must fall within an existing event batch (for example, if you've already
// failed a workflow task via `failWorkflowTask` and have an event batch ID).
func TerminateWorkflow(
mutableState MutableState,
terminateReason string,
terminateDetails *commonpb.Payloads,
terminateIdentity string,
deleteAfterTerminate bool,
) error {
// Terminate workflow is written as a separate batch and might result in more than one event
// if there is started WT which needs to be failed before.
// Failing speculative WT creates 3 events: WTScheduled, WTStarted, and WTFailed.
// First 2 goes to separate batch and eventBatchFirstEventID has to point to WTFailed event.
// Failing transient WT doesn't create any events at all and wtFailedEvent is nil.
// WTFailed event wasn't created (because there were no WT or WT was transient),
// then eventBatchFirstEventID points to TerminateWorkflow event (which is next event).
eventBatchFirstEventID := mutableState.GetNextEventID()
if workflowTask := mutableState.GetStartedWorkflowTask(); workflowTask != nil {
wtFailedEvent, err := failWorkflowTask(
mutableState,
workflowTask,
enumspb.WORKFLOW_TASK_FAILED_CAUSE_FORCE_CLOSE_COMMAND,
)
if err != nil {
return err
}
if wtFailedEvent != nil {
eventBatchFirstEventID = wtFailedEvent.GetEventId()
}
}
_, err := mutableState.AddWorkflowExecutionTerminatedEvent(
eventBatchFirstEventID,
terminateReason,
terminateDetails,
terminateIdentity,
deleteAfterTerminate,
)
return err
}
// FindAutoResetPoint returns the auto reset point
func FindAutoResetPoint(
timeSource clock.TimeSource,
verifyChecksum func(string) error,
autoResetPoints *workflowpb.ResetPoints,
) (string, *workflowpb.ResetPointInfo) {
if autoResetPoints == nil {
return "", nil
}
now := timeSource.Now()
for _, p := range autoResetPoints.Points {
if err := verifyChecksum(p.GetBinaryChecksum()); err != nil && p.GetResettable() {
expireTime := timestamp.TimeValue(p.GetExpireTime())
if !expireTime.IsZero() && now.After(expireTime) {
// reset point has expired and we may already deleted the history
continue
}
return err.Error(), p
}
}
return "", nil
}
func WithEffects(effects effect.Controller, ms MutableState) MutableStateWithEffects {
return MutableStateWithEffects{
MutableState: ms,
Controller: effects,
}
}
type MutableStateWithEffects struct {
MutableState
effect.Controller
}
func (mse MutableStateWithEffects) CanAddEvent() bool {
// Event can be added to the history if workflow is still running.
return mse.MutableState.IsWorkflowExecutionRunning()
}