Repository navigation
Conversation
| return "", false, err | ||
| } | ||
|
|
||
| hookVersion := syncMatchQueue.QueueKey().Version().WorkerDeploymentVersionS() |
There was a problem hiding this comment.
used an existing helper here but seems to be a typo....capital S at the very end (shall change it in a future commit)
| // Stamp value from when the workflow task was scheduled. Used to validate the task is still relevant. | ||
| int32 stamp = 13; | ||
| // Identifies the bound single-session destination for this task, if any. | ||
| temporal.api.session.v1.SingleSessionOptions single_session_options = 14; |
There was a problem hiding this comment.
it's kinda weird to me that the workflow (i.e. the session) deal with SingleSessionOptions and has to pass it as part of tasks to matching.
The mutable state and history->protocol should not use SingleSessionOptions IMO, because that proto is a worker options proto.
When we look from session/workflow POV they would have info/options about their "exclusive worker". When we look from the sticky TQ it has info about it's single assigned session. When looked from worker it has options to state single session preferences.
There was a problem hiding this comment.
was thinking of the SingleSessionOptions being a proto that can be used in places where deemed necessary; i look at the worker options as the ones that are specifically defined on the worker, under WorkerSessionOptions.
There was a problem hiding this comment.
but this is the same SingleSessionOptions that is used in WorkerSessionOptions. I'm not sure the options wf wants to have for single-session mode will be always the same as the info that workflow wants to remember.
| // embedded activities. | ||
| bytes component_ref = 14; | ||
| // Identifies the bound single-session destination for this task, if any. | ||
| temporal.api.session.v1.SingleSessionOptions single_session_options = 15; |
There was a problem hiding this comment.
obviously, the more we put in history the better. Ideally, these APIs don't have to change.
Is there any reason matching needs to get this info from history? why not use the info stored in TQ metadata to know about single-session mode?
There was a problem hiding this comment.
information stored on the task queue metadata can't be used here because that bit is set once you have the task being assigned to a worker. For example, think about the first workflow task of a workflow. On being assigned to a session S1, we set the bit in the task queue metadata. However, we only do this for the 'workflow' task queue partition and not the activity one.
Thus, if the next thing to do for the workflow is to schedule an activity task, then the corresponding activity task queue partition would not know from matching solely that it has been assigned to the session -- which is why i was thinking that we pass this information from history itself.
There was a problem hiding this comment.
the activity case makes sense. but should the proto be something more generic? thinking something like:
message TaskSessionOptions {
WorkerAddress worker_address;
bool exclusive_worker_bound;
}There was a problem hiding this comment.
or maybe TaskSessionDirective consistent with TaskVersionDirective
There was a problem hiding this comment.
as discussed in our call, i agree.
Merge shivam/agentcore-single-session and derive hook context from SessionState.ExclusiveWorker. Preserve the updated timeout and fallback behavior, and omit the redundant standby workflow snapshot.
Summary
Stacked on #12770, which builds on #12630.
History now sends the bound session's stable identity with workflow tasks and opted-in activity tasks. Matching passes it to task-add hooks with the task's actual deployment version and existing match outcome.
Things to do