Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions chasm/lib/scheduler/backfiller.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
schedulespb "go.temporal.io/server/api/schedule/v1"
"go.temporal.io/server/chasm"
"go.temporal.io/server/chasm/lib/scheduler/gen/schedulerpb/v1"
schedulescommon "go.temporal.io/server/common/schedules"
schedulerinternal "go.temporal.io/server/chasm/lib/scheduler/internal"
"google.golang.org/protobuf/types/known/timestamppb"
)

Expand Down Expand Up @@ -36,7 +36,7 @@ func addBackfiller(
ctx chasm.MutableContext,
scheduler *Scheduler,
) *Backfiller {
id := schedulescommon.GenerateBackfillerID()
id := schedulerinternal.GenerateBackfillerID()
backfiller := newBackfillerWithState(ctx, &schedulerpb.BackfillerState{
BackfillId: id,
LastProcessedTime: timestamppb.New(ctx.Now(scheduler)),
Expand Down
4 changes: 2 additions & 2 deletions chasm/lib/scheduler/backfiller_tasks.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import (
schedulespb "go.temporal.io/server/api/schedule/v1"
"go.temporal.io/server/chasm"
"go.temporal.io/server/chasm/lib/scheduler/gen/schedulerpb/v1"
schedulerinternal "go.temporal.io/server/chasm/lib/scheduler/internal"
"go.temporal.io/server/common/log"
"go.temporal.io/server/common/log/tag"
"go.temporal.io/server/common/metrics"
schedulescommon "go.temporal.io/server/common/schedules"
queueerrors "go.temporal.io/server/service/history/queues/errors"
"go.uber.org/fx"
"google.golang.org/protobuf/types/known/timestamppb"
Expand Down Expand Up @@ -219,7 +219,7 @@ func (b *BackfillerTaskHandler) processTrigger(
nowpb := backfiller.GetLastProcessedTime()
now := nowpb.AsTime()
requestID := generateRequestID(scheduler, backfiller.GetBackfillId(), now, now)
workflowID := schedulescommon.GenerateWorkflowID(scheduler.WorkflowID(), now)
workflowID := schedulerinternal.GenerateWorkflowID(scheduler.WorkflowID(), now)
result.BufferedStarts = []*schedulespb.BufferedStart{
{
NominalTime: nowpb,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package schedules
package internal

import (
"fmt"
Expand Down Expand Up @@ -26,14 +26,18 @@ func GenerateRequestID(
if backfillID == "" {
backfillID = "auto"
}

return fmt.Sprintf(
"sched-%s-%s-%s-%d-%d-%d",
"sched-%s-%s",
backfillID,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you want to be completionist, it might be worth capping the backfill ID as well at a max length so that user-defined inputs can't exceed the field size.

namespaceID,
scheduleID,
conflictToken,
nominal.UnixMilli(),
actual.UnixMilli(),
uuid.NewSHA1(uuid.Nil, fmt.Appendf(nil,
"%q-%q-%d-%d-%d",
namespaceID,
scheduleID,
conflictToken,
nominal.UnixMilli(),
actual.UnixMilli(),
)),
)
}

Expand Down
48 changes: 48 additions & 0 deletions chasm/lib/scheduler/internal/request_id_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package internal

import (
"fmt"
"strings"
"testing"
"time"

"github.com/google/uuid"
"github.com/stretchr/testify/require"
)

func TestGenerateWorkflowID(t *testing.T) {
baseWorkflowID := "my-workflow"
nominalTime := time.Date(2024, 6, 15, 10, 30, 45, 123456789, time.UTC)

actual := GenerateWorkflowID(baseWorkflowID, nominalTime)
require.Equal(t, "my-workflow-2024-06-15T10:30:45Z", actual)
}

func TestGenerateRequestID(t *testing.T) {
nominalTime := time.UnixMilli(1_700_000_000_000)
actualTime := time.UnixMilli(1_700_000_000_001)
scheduleID := strings.Repeat("a", 1000)
requestIDUUID := uuid.NewSHA1(uuid.Nil, fmt.Appendf(nil,
"%q-%q-%d-%d-%d",
"nsid",
scheduleID,
10,
nominalTime.UnixMilli(),
actualTime.UnixMilli(),
))

for _, tc := range []struct {
name string
backfillID string
prefix string
}{
{name: "automatic", prefix: "auto"},
{name: "backfill", backfillID: "backfill-id", prefix: "backfill-id"},
} {
t.Run(tc.name, func(t *testing.T) {
requestID := GenerateRequestID("nsid", scheduleID, 10, tc.backfillID, nominalTime, actualTime)
require.Equal(t, fmt.Sprintf("sched-%s-%s", tc.prefix, requestIDUUID), requestID)
require.NotContains(t, requestID, scheduleID)
})
}
}
12 changes: 6 additions & 6 deletions chasm/lib/scheduler/migration/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (
schedulepb "go.temporal.io/api/schedule/v1"
schedulespb "go.temporal.io/server/api/schedule/v1"
schedulerpb "go.temporal.io/server/chasm/lib/scheduler/gen/schedulerpb/v1"
schedulerinternal "go.temporal.io/server/chasm/lib/scheduler/internal"
"go.temporal.io/server/common"
schedulescommon "go.temporal.io/server/common/schedules"
"go.temporal.io/server/common/searchattribute/sadefs"
"google.golang.org/protobuf/types/known/timestamppb"
)
Expand Down Expand Up @@ -216,7 +216,7 @@ func convertBufferedStartsLegacyToCHASM(
v2Start := common.CloneProto(v1Start)

if v2Start.RequestId == "" {
v2Start.RequestId = schedulescommon.GenerateRequestID(
v2Start.RequestId = schedulerinternal.GenerateRequestID(
namespaceID,
scheduleID,
conflictToken,
Expand All @@ -227,7 +227,7 @@ func convertBufferedStartsLegacyToCHASM(
}

if v2Start.WorkflowId == "" {
v2Start.WorkflowId = schedulescommon.GenerateWorkflowID(
v2Start.WorkflowId = schedulerinternal.GenerateWorkflowID(
baseWorkflowID,
v1Start.GetNominalTime().AsTime(),
)
Expand Down Expand Up @@ -268,7 +268,7 @@ func convertRunningWorkflowsToBufferedStarts(
// Include the RunId in the tag to ensure each running workflow
// gets a unique RequestId (important for ALLOW_ALL overlap
// policy where multiple workflows may be running concurrently).
RequestId: schedulescommon.GenerateRequestID(
RequestId: schedulerinternal.GenerateRequestID(
namespaceID,
scheduleID,
conflictToken,
Expand Down Expand Up @@ -332,7 +332,7 @@ func convertRecentActionsToBufferedStarts(
StartTime: action.ActualTime,
WorkflowId: action.StartWorkflowResult.WorkflowId,
RunId: action.StartWorkflowResult.RunId,
RequestId: schedulescommon.GenerateRequestID(
RequestId: schedulerinternal.GenerateRequestID(
namespaceID,
scheduleID,
conflictToken,
Expand All @@ -359,7 +359,7 @@ func convertBackfillsLegacyToCHASM(

backfillers := make(map[string]*schedulerpb.BackfillerState, len(legacyBackfills))
for _, v1Backfill := range legacyBackfills {
backfillID := schedulescommon.GenerateBackfillerID()
backfillID := schedulerinternal.GenerateBackfillerID()

backfillers[backfillID] = &schedulerpb.BackfillerState{
Request: &schedulerpb.BackfillerState_BackfillRequest{
Expand Down
4 changes: 2 additions & 2 deletions chasm/lib/scheduler/spec_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ import (

enumspb "go.temporal.io/api/enums/v1"
schedulespb "go.temporal.io/server/api/schedule/v1"
schedulerinternal "go.temporal.io/server/chasm/lib/scheduler/internal"
"go.temporal.io/server/common/log"
"go.temporal.io/server/common/log/tag"
"go.temporal.io/server/common/metrics"
schedulescommon "go.temporal.io/server/common/schedules"
legacyscheduler "go.temporal.io/server/service/worker/scheduler"
"google.golang.org/protobuf/types/known/timestamppb"
)
Expand Down Expand Up @@ -179,7 +179,7 @@ func (s *SpecProcessorImpl) ProcessTimeRange(
OverlapPolicy: overlapPolicy,
Manual: manual,
RequestId: generateRequestID(scheduler, backfillID, next.Nominal, next.Next),
WorkflowId: schedulescommon.GenerateWorkflowID(workflowID, next.Nominal),
WorkflowId: schedulerinternal.GenerateWorkflowID(workflowID, next.Nominal),
})

if limit != nil {
Expand Down
4 changes: 2 additions & 2 deletions chasm/lib/scheduler/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,17 @@ import (
"encoding/binary"
"time"

schedulerinternal "go.temporal.io/server/chasm/lib/scheduler/internal"
"go.temporal.io/server/common/log"
"go.temporal.io/server/common/log/tag"
"go.temporal.io/server/common/metrics"
schedulescommon "go.temporal.io/server/common/schedules"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/timestamppb"
)

func generateRequestID(scheduler *Scheduler, backfillID string, nominal, actual time.Time) string {
return schedulescommon.GenerateRequestID(
return schedulerinternal.GenerateRequestID(
scheduler.NamespaceId,
scheduler.ScheduleId,
scheduler.ConflictToken,
Expand Down
54 changes: 0 additions & 54 deletions common/schedules/id_test.go

This file was deleted.

25 changes: 25 additions & 0 deletions tests/schedule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ func TestScheduleCHASM(t *testing.T) {
t.Run("TestMigrationCallbackAttach", func(t *testing.T) { t.Parallel(); testMigrationCallbackAttach(t, newContext) })
t.Run("TestCreatesWorkflowSentinel", func(t *testing.T) { t.Parallel(); testCreatesWorkflowSentinel(t, newContext) })
t.Run("TestSkipsWorkflowSentinelWhenDisabled", func(t *testing.T) { t.Parallel(); testSkipsWorkflowSentinelWhenDisabled(t, newContext) })
t.Run("TestLargeScheduleID", func(t *testing.T) { t.Parallel(); testLargeScheduleID(t, newContext) })
t.Run("TestUpdateScheduleMemo", func(t *testing.T) { t.Parallel(); testUpdateScheduleMemo(t, newContext) })
t.Run("TestUpdateScheduleMemoOnly", func(t *testing.T) { t.Parallel(); testUpdateScheduleMemoOnly(t, newContext) })
t.Run("TestStateSizeBytesReported", func(t *testing.T) { t.Parallel(); testStateSizeBytesReported(t, newContext) })
Expand Down Expand Up @@ -4604,6 +4605,30 @@ func testUpdateScheduleRequestIDTooLong(t *testing.T, newContext contextFactory)
require.ErrorAs(t, err, &invalidArgReqID)
}

func testLargeScheduleID(t *testing.T, newContext contextFactory) {
s := newScheduleEnv(t, scheduleCommonOpts(t)...)
ctx := newContext(testcore.NewContext())

// The V1 sentinel shares the SQL workflow ID limit with the schedule ID
// prefix, so this is the largest schedule ID supported by every SQL backend.
const workflowIDColumnLimit = 255
scheduleIDLength := workflowIDColumnLimit - len(scheduler.WorkflowIDPrefix)
sid := strings.Repeat("a", scheduleIDLength)
wid := testcore.RandomizeStr("sched-large-id-wf")
wt := testcore.RandomizeStr("sched-large-id-wt")

var runs atomic.Int32
registerCountingWorkflow(s, wt, &runs)

createSchedule(ctx, t, s, sid, &schedulepb.Schedule{
Spec: intervalSpec(fastInterval),
Action: startWorkflowAction(s, wid, wt),
})

await.RequireTruef(t, func() bool { return runs.Load() > 0 }, awaitTimeout, pollInterval,
"schedule ID of length %d should start a workflow", scheduleIDLength)
}

func testUpdateScheduleBlobSizeLimit(t *testing.T, newContext contextFactory) {
s := newScheduleEnv(t,
append(scheduleCommonOpts(t),
Expand Down
Loading