-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add CHASM Visibility component to CHASM Workflow #10434
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: rodrigozhou/chasm-vis-text
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,8 @@ import ( | |
| callbackspb "go.temporal.io/server/chasm/lib/callback/gen/callbackpb/v1" | ||
| "go.temporal.io/server/chasm/lib/nexusoperation" | ||
| chasmworkflowpb "go.temporal.io/server/chasm/lib/workflow/gen/workflowpb/v1" | ||
| "go.temporal.io/server/common/log/tag" | ||
| "go.temporal.io/server/common/softassert" | ||
| "go.temporal.io/server/service/history/historybuilder" | ||
| "google.golang.org/protobuf/types/known/emptypb" | ||
| "google.golang.org/protobuf/types/known/timestamppb" | ||
|
|
@@ -27,6 +29,9 @@ type Workflow struct { | |
| // MSPointer is a special in-memory field for accessing the underlying mutable state. | ||
| chasm.MSPointer | ||
|
|
||
| // Visibility to store custom search attributes and memo. | ||
| Visibility chasm.Field[*chasm.Visibility] | ||
|
|
||
| // Callbacks map is used to store the callbacks for the workflow. | ||
| Callbacks chasm.Map[string, *callback.Callback] | ||
|
|
||
|
|
@@ -72,6 +77,73 @@ func (w *Workflow) Terminate( | |
| return chasm.TerminateComponentResponse{}, serviceerror.NewInternal("workflow root Terminate should not be called") | ||
| } | ||
|
|
||
| // SearchAttributes returns the predefined search attributes set in the underlying mutable state. | ||
| func (w *Workflow) SearchAttributes(ctx chasm.Context) []chasm.SearchAttributeKeyValue { | ||
| searchAttributes, err := w.GetPredefinedSearchAttributes() | ||
| softassert.That( | ||
| ctx.Logger(), | ||
| err == nil, | ||
| "failed to retrieve search attributes from mutable state execution info", | ||
| tag.Error(err), | ||
| ) | ||
|
|
||
| var res []chasm.SearchAttributeKeyValue | ||
| for saName, value := range searchAttributes { | ||
| res = append(res, chasm.SearchAttributeKeyValue{ | ||
| Alias: saName, | ||
| Field: saName, | ||
| Value: value, | ||
| }) | ||
| } | ||
| return res | ||
| } | ||
|
|
||
| // CustomSearchAttributes returns the custom search attributes. | ||
| func (w *Workflow) CustomSearchAttributes(ctx chasm.Context) map[string]*commonpb.Payload { | ||
|
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. where/how will the following four methods be used?
Contributor
Author
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. It will be used in the mutable state. I'll have a PR using those soon.
Contributor
Author
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. PR using those functions: #10498 |
||
| if vis, ok := w.Visibility.TryGet(ctx); ok { | ||
| return vis.CustomSearchAttributes(ctx) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // CustomMemo returns the custom memo. | ||
| func (w *Workflow) CustomMemo(ctx chasm.Context) map[string]*commonpb.Payload { | ||
| if vis, ok := w.Visibility.TryGet(ctx); ok { | ||
| return vis.CustomMemo(ctx) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // UpsertCustomSearchAttributes merges the provided custom search attributes into the existing one. | ||
| // For details of the merge, see [chasm.Visibility.MergeCustomSearchAttributes]. | ||
| func (w *Workflow) UpsertCustomSearchAttributes( | ||
| ctx chasm.MutableContext, | ||
| customSearchAttributes map[string]*commonpb.Payload, | ||
| ) error { | ||
| if vis, ok := w.Visibility.TryGet(ctx); ok { | ||
| vis.MergeCustomSearchAttributes(ctx, customSearchAttributes) | ||
| } else { | ||
| vis := chasm.NewVisibilityWithData(ctx, customSearchAttributes, nil) | ||
| w.Visibility = chasm.NewComponentField(ctx, vis) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // UpsertCustomMemo merges the provided custom memo into the existing one. | ||
| // For details of the merge, see [chasm.Visibility.MergeCustomMemo]. | ||
| func (w *Workflow) UpsertCustomMemo( | ||
| ctx chasm.MutableContext, | ||
| customMemo map[string]*commonpb.Payload, | ||
| ) error { | ||
| if vis, ok := w.Visibility.TryGet(ctx); ok { | ||
| vis.MergeCustomMemo(ctx, customMemo) | ||
| } else { | ||
| vis := chasm.NewVisibilityWithData(ctx, nil, customMemo) | ||
| w.Visibility = chasm.NewComponentField(ctx, vis) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // ProcessCloseCallbacks triggers "WorkflowClosed" callbacks using the CHASM implementation. | ||
| // It schedules all workflow-level and update-level callbacks that are in STANDBY state. | ||
| func (w *Workflow) ProcessCloseCallbacks(ctx chasm.MutableContext) error { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,9 +3,12 @@ package chasm | |
| import ( | ||
| "time" | ||
|
|
||
| commonpb "go.temporal.io/api/common/v1" | ||
| enumspb "go.temporal.io/api/enums/v1" | ||
| historypb "go.temporal.io/api/history/v1" | ||
| "go.temporal.io/server/common/nexus/nexusrpc" | ||
| "go.temporal.io/server/common/searchattribute/sadefs" | ||
| "google.golang.org/protobuf/proto" | ||
| ) | ||
|
|
||
| // MSPointer is a special CHASM type which components can use to access their Node's underlying backend (i.e. mutable | ||
|
|
@@ -61,3 +64,24 @@ func (m MSPointer) GetWorkflowTypeName() string { | |
| func (m MSPointer) GetNexusUpdateCompletion(ctx Context, updateID string, requestID string) (nexusrpc.CompleteOperationOptions, error) { | ||
| return m.backend.GetNexusUpdateCompletion(ctx.goContext(), updateID, requestID) | ||
| } | ||
|
|
||
| // GetPredefinedSearchAttributes retrieves the predefined search attributes from the underlying mutable state. | ||
| func (m MSPointer) GetPredefinedSearchAttributes() (map[string]VisibilityValue, error) { | ||
|
Contributor
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. maybe I need more context for this, will predefined search attributes end up getting moved to the Workflow Component's state? I guess I am in general having the same question as Yichao; do visibility tasks end up getting duplicated (MS backed and CHASM backed) in this current implementation? I guess when I say duplicate here, I mean CHASM visibility tasks end up writing both custom and predefined search attributes, whereas the legacy visibility task only reads mutable state, and writes only predefined search attributes?
Contributor
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. also for naming purposes, these predefined search attributes include Predefined and System search attributes right?
Contributor
Author
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. Predefined here means really only the predefined search attributes that are stored together with custom search attributes. At this moment, the idea is that all predefined search attributes are stored in the mutable state, and CHASM Workflow extracts the values from there. Not stored twice. For custom search attributes, it would be duplicated. More details in a follow up PR. This PR is not really adding any tasks since it doesn't introduce any changes to the CHASM Visibility component. |
||
| msSearchAttributes := m.backend.GetExecutionInfo().GetSearchAttributes() | ||
| predefinedWithType := make(map[string]*commonpb.Payload) | ||
| for saName, saPayload := range msSearchAttributes { | ||
| if saType, ok := predefinedSearchAttributes[saName]; ok { | ||
| if sadefs.GetMetadataType(saPayload) == enumspb.INDEXED_VALUE_TYPE_UNSPECIFIED { | ||
| saPayload = proto.CloneOf(saPayload) | ||
| sadefs.SetMetadataType(saPayload, saType) | ||
| } | ||
| predefinedWithType[saName] = saPayload | ||
| } | ||
| } | ||
| saMap, err := newSearchAttributesMapFromProto( | ||
| &commonpb.SearchAttributes{ | ||
| IndexedFields: predefinedWithType, | ||
| }, | ||
| ) | ||
| return saMap.values, err | ||
| } | ||
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 |
|---|---|---|
|
|
@@ -1113,6 +1113,10 @@ func (ms *MutableStateImpl) GetExecutionState() *persistencespb.WorkflowExecutio | |
| return ms.executionState | ||
| } | ||
|
|
||
| func (ms *MutableStateImpl) ExecutionStateUpdated() bool { | ||
|
Contributor
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. what's the purpose of this method?
Contributor
Author
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. Execution state is managed in the mutable state currently, and the CHASM Workflow needs to know if the execution state changed to generate CHASM visibility task. |
||
| return ms.stateInDB != ms.executionState.GetState() | ||
| } | ||
|
|
||
| func (ms *MutableStateImpl) FlushBufferedEvents() { | ||
| if ms.HasStartedWorkflowTask() { | ||
| return | ||
|
|
||
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.
do we have to deep copy/clone the SAs here?
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.
We need for backwards compatibility if we need to read search attributes from the mutable state that doesn't contain the metadata type. It's only making a deep copy when necessary, ie., when it doesn't contain the metadata type. Otherwise, it makes a shallow copy.