Skip to content

Conversation

@neil-xie
Copy link
Member

@neil-xie neil-xie commented Nov 19, 2025

Detailed Description
Add cron schedule to visibility customized search attribute

WORKFLOW TYPE      |                   WORKFLOW ID                   |                RUN ID                |    TASK LIST    | IS CRON | START TIME | EXECUTION TIME | END TIME |   CLOSE STATUS   | HISTORY LENGTH | UPDATE TIME |                 SEARCH ATTRIBUTES
  helloWorldWorkflow      | helloworld_0261b29c-5163-4e9d-ab36-fe3449e340c3 | 98dd5f8b-d506-4400-b7d5-12aa672a875e | helloWorldGroup | false   | 13:31:29   | 13:31:29       | 13:31:29 | COMPLETED        |             11 | 16:00:00    | BinaryChecksums:[2172e88e8a2485381bab46576c92773e]
  main.sampleCronWorkflow | cron_e81476f5-ebeb-453d-b2c3-de941c19d46c       | d17f7043-2836-4d22-bbc4-8659c7cbeba0 | cronGroup       | true    | 13:28:00   | 13:30:00       | 13:30:00 | CONTINUED_AS_NEW |             11 | 16:00:00    | CronSchedule:*/2 * * * *

Impact Analysis

  • Backward Compatibility: Yes, it might not be backward compatible for users with same name of attribute already being used. It will get override by this value
  • Forward Compatibility: Yes

Testing Plan

  • Unit Tests: Yes, and manual tested
  • Persistence Tests: [If the change is related to a data type which is persisted, do we have persistence tests covering the change?]
  • Integration Tests: [Do we have integration test covering the change?]
  • Compatibility Tests: [Have we done tests to test the backward and forward compatibility?]

Rollout Plan

  • What is the rollout plan? Normal rollout
  • Does the order of deployment matter? No
  • Is it safe to rollback? Does the order of rollback matter? Yes
  • Is there a kill switch to mitigate the impact immediately? NA

@neil-xie neil-xie changed the title Add cron schedule to customized search attribute feat: Add cron schedule to customized search attribute Nov 19, 2025
"BinaryChecksums": { "type": "keyword"},
"Passed": { "type": "boolean" }
"Passed": { "type": "boolean" },
"CronSchedule": { "type": "keyword" }
Copy link
Member

Choose a reason for hiding this comment

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

What is the difference between adding it here and adding it outside Attr property?

Copy link
Member Author

Choose a reason for hiding this comment

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

Inside attr is the customized search attributes, outside is the system columns

Copy link
Member

Choose a reason for hiding this comment

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

Should we make it a system column?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking not all workflows need this field, but it should be ok since the field will not be present in visibility store if we don't send them. And we will no longer need to worry about if oss user has used them in search attribute

Copy link
Member

@timl3136 timl3136 left a comment

Choose a reason for hiding this comment

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

Let's add a comment in the description saying if user has their CronSchedule field defined in the customized search attribute, their field's value will be override by our value.
And it could leave compatibility issues as two version of CronSchedule might differ.

@timl3136
Copy link
Member

  • Backward Compatibility: Yes
    It might not be backward compatible for users with same naming of attribute

@neil-xie
Copy link
Member Author

Let's add a comment in the description saying if user has their CronSchedule field defined in the customized search attribute, their field's value will be override by our value. And it could leave compatibility issues as two version of CronSchedule might differ.

That is a good idea, I will also mention this in the release note

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants