-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Hook up CHASM Scheduler to Frontend handler #8694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,8 +19,10 @@ import ( | |||||||||||||
| "go.temporal.io/server/chasm/lib/scheduler/gen/schedulerpb/v1" | ||||||||||||||
| chasmnexus "go.temporal.io/server/chasm/nexus" | ||||||||||||||
| "go.temporal.io/server/common" | ||||||||||||||
| "go.temporal.io/server/common/searchattribute/sadefs" | ||||||||||||||
| "go.temporal.io/server/common/util" | ||||||||||||||
| "go.temporal.io/server/service/worker/scheduler" | ||||||||||||||
| "google.golang.org/protobuf/types/known/durationpb" | ||||||||||||||
| "google.golang.org/protobuf/types/known/timestamppb" | ||||||||||||||
| ) | ||||||||||||||
|
|
||||||||||||||
|
|
@@ -92,8 +94,8 @@ func NewScheduler( | |||||||||||||
| Backfillers: make(chasm.Map[string, *Backfiller]), | ||||||||||||||
| LastCompletionResult: chasm.NewDataField(ctx, &schedulerpb.LastCompletionResult{}), | ||||||||||||||
| } | ||||||||||||||
| sched.setNullableFields() | ||||||||||||||
| sched.Info.CreateTime = timestamppb.New(ctx.Now(sched)) | ||||||||||||||
| sched.Schedule.State = &schedulepb.ScheduleState{} | ||||||||||||||
|
|
||||||||||||||
| invoker := NewInvoker(ctx, sched) | ||||||||||||||
| sched.Invoker = chasm.NewComponentField(ctx, invoker) | ||||||||||||||
|
|
@@ -109,6 +111,16 @@ func NewScheduler( | |||||||||||||
| return sched | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // setNullableFields sets fields that are nullable in API requests. | ||||||||||||||
| func (s *Scheduler) setNullableFields() { | ||||||||||||||
| if s.Schedule.Policies == nil { | ||||||||||||||
| s.Schedule.Policies = &schedulepb.SchedulePolicies{} | ||||||||||||||
| } | ||||||||||||||
| if s.Schedule.State == nil { | ||||||||||||||
| s.Schedule.State = &schedulepb.ScheduleState{} | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // handlePatch creates backfillers to fulfill the given patch request. | ||||||||||||||
| func (s *Scheduler) handlePatch(ctx chasm.MutableContext, patch *schedulepb.SchedulePatch) { | ||||||||||||||
| if patch != nil { | ||||||||||||||
|
|
@@ -523,16 +535,72 @@ func (s *Scheduler) Describe( | |||||||||||||
| ctx chasm.Context, | ||||||||||||||
| req *schedulerpb.DescribeScheduleRequest, | ||||||||||||||
| ) (*schedulerpb.DescribeScheduleResponse, error) { | ||||||||||||||
| if s.Closed { | ||||||||||||||
| return nil, ErrClosed | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| visibility := s.Visibility.Get(ctx) | ||||||||||||||
| memo := visibility.GetMemo(ctx) | ||||||||||||||
| delete(memo, visibilityMemoFieldInfo) // We don't need to return a redundant info block. | ||||||||||||||
|
|
||||||||||||||
| attributes := visibility.GetSearchAttributes(ctx) | ||||||||||||||
| delete(attributes, sadefs.TemporalNamespaceDivision) | ||||||||||||||
| delete(attributes, sadefs.TemporalSchedulePaused) | ||||||||||||||
|
|
||||||||||||||
| if s.Schedule.Policies == nil { | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not call |
||||||||||||||
| s.Schedule.Policies = &schedulepb.SchedulePolicies{} | ||||||||||||||
| } | ||||||||||||||
| if s.Schedule.GetPolicies().GetOverlapPolicy() == enumspb.SCHEDULE_OVERLAP_POLICY_UNSPECIFIED { | ||||||||||||||
| s.Schedule.Policies.OverlapPolicy = s.overlapPolicy() | ||||||||||||||
| } | ||||||||||||||
| if !s.Schedule.GetPolicies().GetCatchupWindow().IsValid() { | ||||||||||||||
| // TODO - this should be set from Tweakables.DefaultCatchupWindow. | ||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not pass this into the function? |
||||||||||||||
| s.Schedule.Policies.CatchupWindow = durationpb.New(365 * 24 * time.Hour) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| schedule := common.CloneProto(s.Schedule) | ||||||||||||||
| cleanSpec(schedule.Spec) | ||||||||||||||
|
|
||||||||||||||
| return &schedulerpb.DescribeScheduleResponse{ | ||||||||||||||
| FrontendResponse: &workflowservice.DescribeScheduleResponse{ | ||||||||||||||
| Schedule: common.CloneProto(s.Schedule), | ||||||||||||||
| Info: common.CloneProto(s.Info), | ||||||||||||||
| ConflictToken: s.generateConflictToken(), | ||||||||||||||
| // TODO - memo and search_attributes are handled by visibility (separate PR) | ||||||||||||||
| Schedule: schedule, | ||||||||||||||
| Info: common.CloneProto(s.Info), | ||||||||||||||
| ConflictToken: s.generateConflictToken(), | ||||||||||||||
| Memo: &commonpb.Memo{Fields: memo}, | ||||||||||||||
| SearchAttributes: &commonpb.SearchAttributes{IndexedFields: attributes}, | ||||||||||||||
| }, | ||||||||||||||
| }, nil | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // cleanSpec sets default values in ranges for the DescribeSchedule response. | ||||||||||||||
| func cleanSpec(spec *schedulepb.ScheduleSpec) { | ||||||||||||||
| cleanRanges := func(ranges []*schedulepb.Range) { | ||||||||||||||
| for _, r := range ranges { | ||||||||||||||
| if r.End < r.Start { | ||||||||||||||
| r.End = r.Start | ||||||||||||||
| } | ||||||||||||||
| if r.Step == 0 { | ||||||||||||||
| r.Step = 1 | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| cleanCal := func(structured *schedulepb.StructuredCalendarSpec) { | ||||||||||||||
| cleanRanges(structured.Second) | ||||||||||||||
| cleanRanges(structured.Minute) | ||||||||||||||
| cleanRanges(structured.Hour) | ||||||||||||||
| cleanRanges(structured.DayOfMonth) | ||||||||||||||
| cleanRanges(structured.Month) | ||||||||||||||
| cleanRanges(structured.Year) | ||||||||||||||
| cleanRanges(structured.DayOfWeek) | ||||||||||||||
| } | ||||||||||||||
| for _, structured := range spec.StructuredCalendar { | ||||||||||||||
| cleanCal(structured) | ||||||||||||||
| } | ||||||||||||||
| for _, structured := range spec.ExcludeStructuredCalendar { | ||||||||||||||
| cleanCal(structured) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Delete marks the Scheduler as closed without an idle timer. | ||||||||||||||
| func (s *Scheduler) Delete( | ||||||||||||||
| ctx chasm.MutableContext, | ||||||||||||||
|
|
@@ -557,13 +625,32 @@ func (s *Scheduler) Update( | |||||||||||||
| // | ||||||||||||||
| // TODO - we could also easily support allowing the customer to update their | ||||||||||||||
| // memo here. | ||||||||||||||
| visibility := s.Visibility.Get(ctx) | ||||||||||||||
| visibility.SetSearchAttributes(ctx, req.FrontendRequest.GetSearchAttributes().GetIndexedFields()) | ||||||||||||||
| if req.FrontendRequest.GetSearchAttributes() != nil { | ||||||||||||||
| // To preserve compatibility with V1 scheduler, we do a full replacement | ||||||||||||||
| // of search attributes, dropping any that aren't a part of the update's | ||||||||||||||
| // `CustomSearchAttributes` map. Search attribute replacement is ignored entirely | ||||||||||||||
| // when that map is unset, however, an allocated yet empty map will clear all | ||||||||||||||
| // attributes. | ||||||||||||||
|
|
||||||||||||||
| // Preserve the old custom memo in the new Visibility component. | ||||||||||||||
| oldVisibility := s.Visibility.Get(ctx) | ||||||||||||||
| oldMemo := oldVisibility.GetMemo(ctx) | ||||||||||||||
|
|
||||||||||||||
| visibility := chasm.NewVisibility(ctx) | ||||||||||||||
| s.Visibility = chasm.NewComponentField(ctx, visibility) | ||||||||||||||
| visibility.SetSearchAttributes(ctx, req.FrontendRequest.GetSearchAttributes().GetIndexedFields()) | ||||||||||||||
| visibility.SetMemo(ctx, oldMemo) | ||||||||||||||
|
Comment on lines
+639
to
+642
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
|
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| s.Schedule = common.CloneProto(req.FrontendRequest.Schedule) | ||||||||||||||
| s.Schedule = req.FrontendRequest.Schedule | ||||||||||||||
| s.setNullableFields() | ||||||||||||||
| s.Info.UpdateTime = timestamppb.New(ctx.Now(s)) | ||||||||||||||
| s.updateConflictToken() | ||||||||||||||
|
|
||||||||||||||
| // Since the spec may have been updated, kick off the generator. | ||||||||||||||
| generator := s.Generator.Get(ctx) | ||||||||||||||
| generator.Generate(ctx) | ||||||||||||||
|
|
||||||||||||||
| return &schedulerpb.UpdateScheduleResponse{ | ||||||||||||||
| FrontendResponse: &workflowservice.UpdateScheduleResponse{}, | ||||||||||||||
| }, nil | ||||||||||||||
|
|
@@ -601,6 +688,11 @@ func (s *Scheduler) generateConflictToken() []byte { | |||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| func (s *Scheduler) validateConflictToken(token []byte) bool { | ||||||||||||||
| // When unset in mutate requests, the schedule should update unconditionally. | ||||||||||||||
| if token == nil { | ||||||||||||||
| return true | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| current := s.generateConflictToken() | ||||||||||||||
| return bytes.Equal(current, token) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package chasm | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kinda hate that we have to put this here but this in not blocking the PR. |
||
|
|
||
| // Scheduler's library and component name constants are exported here, as they | ||
| // are used as part of old VisibilityManager queries (e.g., visibility queries | ||
| // out-of-band of CHASM). Most components shouldn't have to define, or export, | ||
| // these names. | ||
| const ( | ||
| SchedulerLibraryName = "scheduler" | ||
| SchedulerComponentName = "scheduler" | ||
| ) | ||
|
|
||
| var ( | ||
| SchedulerArchetype = Archetype(fullyQualifiedName(SchedulerLibraryName, SchedulerComponentName)) | ||
| SchedulerArchetypeID = ArchetypeID(generateTypeID(SchedulerArchetype)) | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import ( | |
| "encoding/base64" | ||
| "fmt" | ||
| "io" | ||
| "strings" | ||
|
|
||
| "github.com/google/uuid" | ||
| commonpb "go.temporal.io/api/common/v1" | ||
|
|
@@ -44,7 +45,7 @@ func logInternalError(logger log.Logger, internalMsg string, internalErr error) | |
|
|
||
| func (c chasmInvocation) Invoke(ctx context.Context, ns *namespace.Namespace, e taskExecutor, task InvocationTask) invocationResult { | ||
| // Get back the base64-encoded ComponentRef from the header. | ||
| encodedRef, ok := c.nexus.GetHeader()[commonnexus.CallbackTokenHeader] | ||
| encodedRef, ok := c.nexus.GetHeader()[strings.ToLower(commonnexus.CallbackTokenHeader)] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am fixing this in #8692 FYI. |
||
| if !ok { | ||
| return invocationResultFail{logInternalError(e.Logger, "callback missing token", nil)} | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These shouldn't come back from visibility... they are system fields, not user fields.
Are you seeing those come back?