Fallback behaviour for Queries #684

Shivs11 · 2025-12-05T22:03:42Z

READ BEFORE MERGING: All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted.

changes so that we can now support Pinned Queries on Poller-less Drained versions to bounce to another Pinned version that has pollers.

Why?

The reasoning for this is so that folks who want to query drained versions can still do so, so a certain extent.

Breaking changes

These changes are additive. No breaking changes.

It's in the process! These changes being additive makes me wanna believe that it may not be required.

Shivs11 · 2025-12-05T22:10:00Z

temporal/api/enums/v1/deployment.proto

+    PINNED_QUERY_FALLBACK_UNSPECIFIED = 0;
+    // The Query will be sent to the next Pinned Version that has pollers. 
+    // If no such version exists, the Query will return an error.
+    PINNED_QUERY_FALLBACK_NEXT_PINNED_WITH_POLLERS = 1;


The idea is that Queries coming in from the UI, when you click the Send Query button, will be sent with this behaviour enabled by default. Maybe I should mention this here, but not sure if it's recommended to do so in the API repo. Open to suggestions

UI querying should not be special. We have several interfaces for common things like querying and it would be confusing for users if they behaved differently. And if we expect something to be default, then the behavior when the field is unset should represent that and any field setting should be to change the default behavior.

Talked w/ the crew in Slack and I think the missing bit of context here is when cloud folks started using internally, they immediately ran into confusion with the UI metadata queries breaking.

But I agree having different defaults in different places is concerning.

My proposal would be to make UI metadata work, via this, but to add a disclaimer text somewhere that says it ran on a different version.

Even if we had 100 (hypothetical number) different callers using the same underlying interface, I was assuming that the call that takes place to build and process the UI metadata query sets the value of this enum to PINNED_QUERY_FALLBACK_NEXT_PINNED_WITH_POLLERS_. In all other cases, this value is PINNED_QUERY_FALLBACK_UNSPECIFIED which is used by the other 99 call sites.

This was the idea that I had, but maybe I didn't do a good job explaining it. @Sushisource / @cretz are y'all saying that this design is not possible due to some other constraints?

If I am wrong, I apologize since I have a low understanding of how code is built and present in the SDK rn

The concerning statement for me was this:

Queries coming in from the UI, when you click the Send Query button, will be sent with this behaviour enabled by default

Even if we had 100 (hypothetical number) different callers using the same underlying interface, I was assuming that the call that takes place to build and process the UI metadata query sets the value of this enum to PINNED_QUERY_FALLBACK_NEXT_PINNED_WITH_POLLERS_. In all other cases, this value is PINNED_QUERY_FALLBACK_UNSPECIFIED which is used by the other 99 call sites.

I'm a bit confused. One statement says the "send query button will be sent with this behavior" and another says "UI metadata query". Those seem contradictory. Can you clarify when you expect callers to set this?

Queries using "send query" or CLI or SDK or API or whatever, should all have the same default, and it should be the same thing as having never set this field.

Also, ideally the metadata query should not be treated differently than any other query. Many users have their own form of this they query the exact same way to power their UIs. Are we expecting Temporal users to now have to understand these separate query forms/options depending on how they use this query?

cretz · 2025-12-05T22:11:34Z

temporal/api/workflowservice/v1/request_response.proto

    temporal.api.enums.v1.QueryRejectCondition query_reject_condition = 4;
+
+    // Specifies the fallback behavior for Queries sent to Pinned Workflow executions. 
+    temporal.api.enums.v1.PinnedQueryFallback pinned_query_fallback = 5;


Can we qualify the field/enum name a bit here? May not otherwise be clear this is only for people leveraging deployments/versioning. Also, the current field/enum names makes it sound like Temporal has a concept of "pinned queries". Also, "fallback" is a bit generic, maybe that's not a big deal, maybe it's clear that the only meaning of "pinned fallbacks" is this drained/no-poller situation.

Maybe the field name can be pinned_versioning_behavior_fallback and the enum can be QueryPinnedVersioningBehaviorFallback or something

Agreed - I modified the enum, message and the fields. Let me know what you make of them.

Looks great, thanks

cretz · 2025-12-05T22:12:35Z

temporal/api/workflowservice/v1/request_response.proto

Is there a linked server PR for this? I know we add a lot of versioning stuff these days and some of it is deprecated soon thereafter, so just want to confirm we're sure this is what is needed (I admit I don't know much about this versioning subject).

I still have not brought it to a stage where I can open a PR - it's very roughly built rn with a bunch of TODO's

I can do my best to wrap it up soon before the API PR closes out so that you can have a peek at it before you approve of this.

👍 IMO no need to merge this until we have code ready to use it (in case we learn something)

cretz · 2025-12-05T22:21:48Z

temporal/api/workflowservice/v1/request_response.proto

    temporal.api.enums.v1.QueryRejectCondition query_reject_condition = 4;
+
+    // Specifies the fallback behavior for Queries sent to Pinned Workflow executions. 
+    temporal.api.enums.v1.PinnedQueryFallback pinned_query_fallback = 5;


Is it expected for all interfaces (UI, CLI, and SDKs) to expose this option to query callers? In general trying to understand how this advanced field is used.

My understanding was that the purpose was to resolve complaints that we have received where closed pinned workflows can't show their metadata in the UI after the workers for those versions have been killed.

So the first place to expose it would be UI queries.

But what about users that have their own queries for the exact same type of purpose for use in their UIs?

IMO all queries should work the same way. If you have set a workflow that cannot service queries anymore because the worker is gone, then it can't service any query anymore IMO and user metadata is no different than any other post-close query I would think. How the current details is set may be incompatible and it goes through full replay to run this query.

I also wonder, if a user wants to override versioning behavior for one interaction (not just query, but maybe other interactions), is that specific to one interface or one interaction type?

cretz · 2025-12-05T22:25:14Z

temporal/api/enums/v1/deployment.proto

+enum PinnedQueryFallback {
+    // This is the default behavior. Queries will not fallback to any other version by default.
+    PINNED_QUERY_FALLBACK_UNSPECIFIED = 0;
+    // The Query will be sent to the next Pinned Version that has pollers. 


Pardon my lack of knowledge here, but versions are ordered in the sense there is such thing as an obvious "next" version? And is there no case where you'd want to signal, update, cancel, etc a "next" version in this fallback situation, only query?

They're ordered by creation time

Sushisource

Did we decide for sure that we even want this to be configurable?

My biggest concern is simply that we have too many knobs. Safe Deploys overall are already just absolutely riddled with various settings. I don't want to add more unless we've got a very strong signal that this is needed.

yiminc

I like this approach.

Shivs11 · 2025-12-08T18:23:29Z

Synced with the crew and we have decided to close this for now. With safe-transitions coming around, we anticipate more folks to use AutoUpgrade workflows rather than Pinned ones. Moreover, we also decided to wait until we have more feedback coming in from users since right now, it's only been the internal team who has requested this (we can also fix their problem by just increasing the amount of time they have their worker pods up). Additionally, there's already a lot of churn in the API and punting this ahead is a step in the right direction.

add some protos

3d1f097

Shivs11 requested review from a team as code owners December 5, 2025 22:03

Shivs11 added 3 commits December 5, 2025 17:07

changes

ee6f30b

Merge branch 'master' into ss/pinned-query-fallback

c734aae

protos

373226a

Shivs11 commented Dec 5, 2025

View reviewed changes

cretz reviewed Dec 5, 2025

View reviewed changes

Sushisource reviewed Dec 5, 2025

View reviewed changes

yiminc approved these changes Dec 6, 2025

View reviewed changes

change naming to be clearer

e583158

Shivs11 closed this Dec 8, 2025

Fallback behaviour for Queries #684

Fallback behaviour for Queries #684

Uh oh!

Conversation

Shivs11 commented Dec 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shivs11 Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sushisource left a comment

Choose a reason for hiding this comment

Uh oh!

yiminc left a comment

Choose a reason for hiding this comment

Uh oh!

Shivs11 commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cretz Dec 5, 2025 •

edited

Loading

Shivs11 Dec 8, 2025 •

edited

Loading

cretz Dec 5, 2025 •

edited

Loading

cretz Dec 8, 2025 •

edited

Loading

Shivs11 commented Dec 8, 2025 •

edited

Loading