Skip to content

Commit 4e89dc9

Browse files
committed
fix errors in tests, change version and stamp to VersionedTransition
1 parent 0b28fa4 commit 4e89dc9

13 files changed

Lines changed: 339 additions & 329 deletions

api/persistence/v1/executions.pb.go

Lines changed: 139 additions & 146 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

common/persistence/serialization/task_serializers.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,14 +144,13 @@ func serializeTimerTask(
144144

145145
func timeSkippingTimerTaskToProto(task *tasks.TimeSkippingTimerTask) *persistencespb.TimerTaskInfo {
146146
return &persistencespb.TimerTaskInfo{
147-
NamespaceId: task.NamespaceID,
148-
WorkflowId: task.WorkflowID,
149-
RunId: task.RunID,
150-
TaskId: task.TaskID,
151-
VisibilityTime: timestamppb.New(task.VisibilityTimestamp),
152-
TaskType: enumsspb.TASK_TYPE_TIMESKIPPING_TIMER,
153-
Version: task.Version,
154-
Stamp: task.Stamp,
147+
NamespaceId: task.NamespaceID,
148+
WorkflowId: task.WorkflowID,
149+
RunId: task.RunID,
150+
TaskId: task.TaskID,
151+
VisibilityTime: timestamppb.New(task.VisibilityTimestamp),
152+
TaskType: enumsspb.TASK_TYPE_TIMESKIPPING_TIMER,
153+
VersionedTransition: task.VersionedTransition,
155154
}
156155
}
157156

@@ -164,8 +163,7 @@ func timeSkippingTimerTaskFromProto(info *persistencespb.TimerTaskInfo) *tasks.T
164163
),
165164
VisibilityTimestamp: info.VisibilityTime.AsTime(),
166165
TaskID: info.TaskId,
167-
Version: info.Version,
168-
Stamp: info.Stamp,
166+
VersionedTransition: info.VersionedTransition,
169167
}
170168
}
171169

proto/internal/temporal/server/api/persistence/v1/executions.proto

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -352,14 +352,7 @@ message FastForwardInfo {
352352
// Indicates whether this fast-forward has already been reached, used for idempotency checks.
353353
bool has_reached = 2;
354354

355-
// This is a monotonically increasing counter that is used to identify the version of the fast-forward.
356-
// It is bumped every time the fast-forward is set.
357-
// It is used to validate the time skipping fast-forward timer task.
358-
int32 stamp = 4;
359-
360-
// This is the NamespaceFailoverVersion at which this fast-forward was configured.
361-
// It is used to validate the time skipping fast-forward timer task.
362-
int64 version = 5;
355+
VersionedTransition last_update_versioned_transition = 4;
363356
}
364357

365358
// Internal wrapper message to distinguish "never notified" (nil wrapper) from
@@ -529,6 +522,11 @@ message TimerTaskInfo {
529522
// If the task addresses a CHASM component, this field will be set.
530523
ChasmTaskInfo chasm_task_info = 17;
531524
}
525+
526+
// VersionedTransition provides a unified solution for both intra-cluster and inter-cluster
527+
// task replication. It is intended to replace previous validation mechanisms, such as
528+
// version + stamp.
529+
VersionedTransition versioned_transition = 18;
532530
}
533531

534532
message ArchivalTaskInfo {

service/history/tasks/timeskipping_timer_task.go

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"time"
66

77
enumsspb "go.temporal.io/server/api/enums/v1"
8+
persistencespb "go.temporal.io/server/api/persistence/v1"
89
"go.temporal.io/server/common/definition"
910
)
1011

@@ -13,41 +14,18 @@ var _ Task = (*TimeSkippingTimerTask)(nil)
1314
type (
1415
// TimeSkippingTimerTask wakes a workflow when the fast-forward configured
1516
// on its TimeSkippingConfig should take effect.
16-
//
17-
// Stamp identifies the fast-forward this task targets. It is matched against
18-
// TimeSkippingInfo.FastForwardInfo.Stamp at firing time to detect superseded tasks:
19-
// re-applying the fast-forward emits a new task with a bumped Stamp, and the old task is
20-
// silently dropped on mismatch. Version is the namespace failover version, validated via
21-
// CheckTaskVersion. Both are event-free, so the task works for workflow and CHASM executions.
2217
TimeSkippingTimerTask struct {
2318
definition.WorkflowKey
2419
VisibilityTimestamp time.Time
20+
VersionedTransition *persistencespb.VersionedTransition
2521
TaskID int64
26-
Version int64
27-
Stamp int32
2822
}
2923
)
3024

3125
func (t *TimeSkippingTimerTask) GetKey() Key {
3226
return NewKey(t.VisibilityTimestamp, t.TaskID)
3327
}
3428

35-
func (t *TimeSkippingTimerTask) GetVersion() int64 {
36-
return t.Version
37-
}
38-
39-
func (t *TimeSkippingTimerTask) SetVersion(version int64) {
40-
t.Version = version
41-
}
42-
43-
func (t *TimeSkippingTimerTask) GetStamp() int32 {
44-
return t.Stamp
45-
}
46-
47-
func (t *TimeSkippingTimerTask) SetStamp(stamp int32) {
48-
t.Stamp = stamp
49-
}
50-
5129
func (t *TimeSkippingTimerTask) GetTaskID() int64 {
5230
return t.TaskID
5331
}
@@ -73,11 +51,12 @@ func (t *TimeSkippingTimerTask) GetType() enumsspb.TaskType {
7351
}
7452

7553
func (t *TimeSkippingTimerTask) String() string {
76-
return fmt.Sprintf("TimeSkippingTimerTask{WorkflowKey: %s, VisibilityTimestamp: %v, TaskID: %v, Version: %v, Stamp: %v}",
54+
vt := t.VersionedTransition
55+
return fmt.Sprintf("TimeSkippingTimerTask{WorkflowKey: %s, VisibilityTimestamp: %v, TaskID: %v, FailoverVersion: %v, TransitionCount: %v}",
7756
t.WorkflowKey.String(),
7857
t.VisibilityTimestamp,
7958
t.TaskID,
80-
t.Version,
81-
t.Stamp,
59+
vt.GetNamespaceFailoverVersion(),
60+
vt.GetTransitionCount(),
8261
)
8362
}

service/history/timer_queue_active_task_executor.go

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package history
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"time"
78

@@ -939,21 +940,40 @@ func (t *timerQueueActiveTaskExecutor) executeTimeSkippingTimerTask(
939940
}
940941

941942
// task staleness check
943+
// 1) time skipping is disabled
942944
tsi := mutableState.GetExecutionInfo().GetTimeSkippingInfo()
943-
if tsi == nil || !tsi.GetConfig().GetEnabled() {
945+
tsiUtil := workflow.NewTimeSkippingInfoUtil(tsi)
946+
if !tsiUtil.IsEnabled() {
944947
release(nil)
945948
return errNoTimerFired
946949
}
947-
ff := tsi.GetFastForwardInfo()
948-
if ff.GetTargetTime() == nil || ff.GetHasReached() || ff.GetStamp() != task.Stamp {
950+
// 2) fast-forward is disabled/reached, or this task was superseded by a newer fast-forward.
951+
// The versioned transition's TransitionCount identifies the specific fast-forward instance
952+
// (a re-applied fast-forward records a fresh transition), so a mismatch means this task is
953+
// stale and is dropped silently.
954+
if !tsiUtil.HasPendingFastForward() {
949955
release(nil)
950956
return errNoTimerFired
951957
}
952-
ffVersion := ff.GetVersion()
953-
if err := CheckTaskVersion(t.shardContext, t.logger, mutableState.GetNamespaceEntry(), ffVersion, task.Version, task); err != nil {
954-
return err
958+
959+
ffVT := tsi.GetFastForwardInfo().GetLastUpdateVersionedTransition()
960+
if ffVT != nil && task.VersionedTransition != nil {
961+
if ffVT.GetTransitionCount() != task.VersionedTransition.GetTransitionCount() {
962+
release(nil)
963+
return errNoTimerFired
964+
}
965+
taskVersion := task.VersionedTransition.GetNamespaceFailoverVersion()
966+
ffVersion := ffVT.GetNamespaceFailoverVersion()
967+
if err := CheckTaskVersion(t.shardContext, t.logger, mutableState.GetNamespaceEntry(), ffVersion, taskVersion, task); err != nil {
968+
release(nil)
969+
return errNoTimerFired
970+
}
971+
} else {
972+
return errors.New("time skipping timer task validation failed: nil versioned transition")
955973
}
956974

975+
// 3) firing fast-forward timer (only turns off time skipping, and no task regeneration)
976+
// TODO@time-skipping: chasm execution path is not implemented yet.
957977
_, err = mutableState.AddWorkflowExecutionTimeSkippingTransitionedEvent(
958978
ctx, time.Time{}, true)
959979
if err != nil {

service/history/timer_queue_active_task_executor_test.go

Lines changed: 64 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -2407,101 +2407,96 @@ func (s *timerQueueActiveTaskExecutorSuite) makeTimeSkippingMS() (*persistencesp
24072407
return pms, workflowKey
24082408
}
24092409

2410-
// TestExecuteTimeSkippingTimerTask covers the full validity-check ladder of
2411-
// executeTimeSkippingTimerTask plus the happy-path disable transition.
24122410
func (s *timerQueueActiveTaskExecutorSuite) TestExecuteTimeSkippingTimerTask() {
24132411
target := timestamppb.New(s.now.Add(time.Hour))
2414-
enabledFF := func(stamp int32, hasReached bool) *persistencespb.TimeSkippingInfo {
2412+
// The fast-forward's versioned transition carries the failover version (NamespaceFailoverVersion)
2413+
// and the fast-forward instance identity (TransitionCount, the successor of the old Stamp).
2414+
enabledFF := func(version int64, transitionCount int64, hasReached bool) *persistencespb.TimeSkippingInfo {
24152415
return &persistencespb.TimeSkippingInfo{
24162416
Config: &commonpb.TimeSkippingConfig{Enabled: true},
24172417
FastForwardInfo: &persistencespb.FastForwardInfo{
24182418
TargetTime: target,
2419-
Stamp: stamp,
24202419
HasReached: hasReached,
2421-
Version: s.version,
2420+
LastUpdateVersionedTransition: &persistencespb.VersionedTransition{
2421+
NamespaceFailoverVersion: version,
2422+
TransitionCount: transitionCount,
2423+
},
24222424
},
24232425
}
24242426
}
2425-
24262427
for _, tc := range []struct {
2427-
name string
2428-
completed bool // mark workflow execution completed
2429-
tsi *persistencespb.TimeSkippingInfo
2430-
taskVersion int64
2431-
taskStamp int32
2432-
wantErr error // nil => happy path: disable transition is applied
2428+
name string
2429+
completed bool // mark workflow execution completed
2430+
tsi *persistencespb.TimeSkippingInfo
2431+
taskVersion int64
2432+
taskTransitionCount int64
2433+
wantErr error // nil => happy path: disable transition is applied
24332434
}{
24342435
{
2435-
name: "WorkflowNotRunning",
2436-
completed: true,
2437-
tsi: enabledFF(1, false),
2438-
taskVersion: s.version,
2439-
taskStamp: 1,
2440-
wantErr: consts.ErrWorkflowCompleted,
2436+
name: "WorkflowNotRunning",
2437+
completed: true,
2438+
tsi: enabledFF(s.version, 1, false),
2439+
taskVersion: s.version,
2440+
taskTransitionCount: 1,
2441+
wantErr: consts.ErrWorkflowCompleted,
24412442
},
24422443
{
2443-
name: "TSINil",
2444-
tsi: nil,
2445-
taskVersion: s.version,
2446-
taskStamp: 1,
2447-
wantErr: errNoTimerFired,
2444+
name: "TSINil",
2445+
tsi: nil,
2446+
taskVersion: s.version,
2447+
taskTransitionCount: 1,
2448+
wantErr: errNoTimerFired,
24482449
},
24492450
{
24502451
name: "ConfigDisabled",
24512452
tsi: &persistencespb.TimeSkippingInfo{
2452-
Config: &commonpb.TimeSkippingConfig{Enabled: false},
2453-
FastForwardInfo: &persistencespb.FastForwardInfo{TargetTime: target, Stamp: 1},
2453+
Config: &commonpb.TimeSkippingConfig{Enabled: false},
2454+
FastForwardInfo: &persistencespb.FastForwardInfo{
2455+
TargetTime: target,
2456+
LastUpdateVersionedTransition: &persistencespb.VersionedTransition{NamespaceFailoverVersion: s.version, TransitionCount: 1},
2457+
},
24542458
},
2455-
taskVersion: s.version,
2456-
taskStamp: 1,
2457-
wantErr: errNoTimerFired,
2459+
taskVersion: s.version,
2460+
taskTransitionCount: 1,
2461+
wantErr: errNoTimerFired,
24582462
},
24592463
{
2460-
name: "NoFastForwardInfo",
2461-
tsi: &persistencespb.TimeSkippingInfo{Config: &commonpb.TimeSkippingConfig{Enabled: true}},
2462-
taskVersion: s.version,
2463-
taskStamp: 1,
2464-
wantErr: errNoTimerFired,
2464+
name: "FastForwardInfoDeleted",
2465+
tsi: &persistencespb.TimeSkippingInfo{Config: &commonpb.TimeSkippingConfig{Enabled: true}},
2466+
taskVersion: s.version,
2467+
taskTransitionCount: 1,
2468+
wantErr: errNoTimerFired,
24652469
},
24662470
{
2467-
// Stamp 0 is a degenerate fast-forward (a real one always has Stamp >= 1); even a
2468-
// matching zero-stamp task must be treated as invalid and dropped.
2469-
name: "StampZero",
2470-
tsi: enabledFF(0, false),
2471-
taskVersion: s.version,
2472-
taskStamp: 0,
2473-
wantErr: errNoTimerFired,
2471+
name: "HasReached",
2472+
tsi: enabledFF(s.version, 1, true),
2473+
taskVersion: s.version,
2474+
taskTransitionCount: 1,
2475+
wantErr: errNoTimerFired,
24742476
},
24752477
{
2476-
name: "HasReached",
2477-
tsi: enabledFF(1, true),
2478-
taskVersion: s.version,
2479-
taskStamp: 1,
2480-
wantErr: errNoTimerFired,
2478+
// fast-forward was reconfigured (transition count 2); the old task carries 1.
2479+
name: "TransitionCountMismatch",
2480+
tsi: enabledFF(s.version, 2, false),
2481+
taskVersion: s.version,
2482+
taskTransitionCount: 1,
2483+
wantErr: errNoTimerFired,
24812484
},
24822485
{
2483-
// fast-forward was reconfigured (stamp 2); the old task carries stamp 1.
2484-
name: "StampMismatch",
2485-
tsi: enabledFF(2, false),
2486-
taskVersion: s.version,
2487-
taskStamp: 1,
2488-
wantErr: errNoTimerFired,
2486+
// Transition count matches the live fast-forward, but the task carries a stale failover
2487+
// version, so CheckTaskVersion rejects it before the disable transition is applied.
2488+
name: "VersionMismatch",
2489+
tsi: enabledFF(s.version, 1, false),
2490+
taskVersion: s.version + 1,
2491+
taskTransitionCount: 1,
2492+
wantErr: consts.ErrTaskVersionMismatch,
24892493
},
24902494
{
2491-
// Stamp matches the live fast-forward, but the task carries a stale failover version,
2492-
// so CheckTaskVersion rejects it before the disable transition is applied.
2493-
name: "VersionMismatch",
2494-
tsi: enabledFF(1, false),
2495-
taskVersion: s.version + 1,
2496-
taskStamp: 1,
2497-
wantErr: consts.ErrTaskVersionMismatch,
2498-
},
2499-
{
2500-
name: "HappyPath",
2501-
tsi: enabledFF(1, false),
2502-
taskVersion: s.version,
2503-
taskStamp: 1,
2504-
wantErr: nil,
2495+
name: "HappyPath",
2496+
tsi: enabledFF(s.version, 1, false),
2497+
taskVersion: s.version,
2498+
taskTransitionCount: 1,
2499+
wantErr: nil,
25052500
},
25062501
} {
25072502
s.Run(tc.name, func() {
@@ -2515,8 +2510,10 @@ func (s *timerQueueActiveTaskExecutorSuite) TestExecuteTimeSkippingTimerTask() {
25152510
WorkflowKey: workflowKey,
25162511
TaskID: s.mustGenerateTaskID(),
25172512
VisibilityTimestamp: s.now.Add(time.Hour),
2518-
Version: tc.taskVersion,
2519-
Stamp: tc.taskStamp,
2513+
VersionedTransition: &persistencespb.VersionedTransition{
2514+
NamespaceFailoverVersion: tc.taskVersion,
2515+
TransitionCount: tc.taskTransitionCount,
2516+
},
25202517
}
25212518
s.mockExecutionMgr.EXPECT().GetWorkflowExecution(gomock.Any(), gomock.Any()).
25222519
Return(&persistence.GetWorkflowExecutionResponse{State: pms}, nil)

service/history/timer_queue_standby_task_executor.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"go.temporal.io/server/common/log"
1919
"go.temporal.io/server/common/metrics"
2020
"go.temporal.io/server/common/namespace"
21+
"go.temporal.io/server/common/persistence/transitionhistory"
2122
"go.temporal.io/server/common/resource"
2223
"go.temporal.io/server/service/history/configs"
2324
"go.temporal.io/server/service/history/consts"
@@ -245,7 +246,9 @@ func (t *timerQueueStandbyTaskExecutor) executeTimeSkippingTimerTask(
245246
ffi := tsi.GetFastForwardInfo()
246247

247248
// the fast-forward this timer task is associated with is still valid and has not been reached so keep waiting
248-
if ffi != nil && ffi.GetStamp() == timerTask.Stamp && !ffi.GetHasReached() {
249+
if ffi != nil &&
250+
transitionhistory.Compare(ffi.GetLastUpdateVersionedTransition(), timerTask.VersionedTransition) == 0 &&
251+
!ffi.GetHasReached() {
249252
return &struct{}{}, nil
250253
}
251254
return nil, nil

0 commit comments

Comments
 (0)