Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Starting server + adding search attributes doesn't make the attributes immediately available #6195

Closed
cretz opened this issue Jun 26, 2024 · 0 comments
Closed

Starting server + adding search attributes doesn't make the attributes immediately available #6195

cretz opened this issue Jun 26, 2024 · 0 comments
Assignees
@rodrigozhou
Labels
potential-bug

Comments

@cretz
Copy link
Member

cretz commented Jun 26, 2024

Expected Behavior

When starting server (i.e. with go.temporal.io/server/temporal.Server.Start) with system.forceSearchAttributesCacheRefreshOnRead dyn config as true and then using operator service immediately register search attributes, I should be able to start a workflow right away.

Actual Behavior

Sometimes the workflow fails with "Namespace default has no mapping defined for search attribute ". And when it does succeed, the logs have warnings like:

time=2024-06-26T10:51:28.528 level=WARN msg="Search attribute DotNetTemporalTestText already exists." service=frontend namespace=default visibility-search-attribute=DotNetTemporalTestText
time=2024-06-26T10:51:28.528 level=WARN msg="Search attribute DotNetTemporalTestBool already exists." service=frontend namespace=default visibility-search-attribute=DotNetTemporalTestBool
time=2024-06-26T10:51:28.534 level=WARN msg="Search attribute DotNetTemporalTestDateTime already exists." service=frontend namespace=default visibility-search-attribute=DotNetTemporalTestDateTime
time=2024-06-26T10:51:28.534 level=WARN msg="Search attribute DotNetTemporalTestDouble already exists." service=frontend namespace=default visibility-search-attribute=DotNetTemporalTestDouble
time=2024-06-26T10:51:28.534 level=WARN msg="Search attribute DotNetTemporalTestKeyword already exists." service=frontend namespace=default visibility-search-attribute=DotNetTemporalTestKeyword
time=2024-06-26T10:51:28.534 level=WARN msg="Search attribute DotNetTemporalTestKeywordList already exists." service=frontend namespace=default visibility-search-attribute=DotNetTemporalTestKeywordList
time=2024-06-26T10:51:28.534 level=WARN msg="Search attribute DotNetTemporalTestLong already exists." service=frontend namespace=default visibility-search-attribute=DotNetTemporalTestLong

Steps to Reproduce the Problem

Start server programmatically with system.forceSearchAttributesCacheRefreshOnRead dyn config as true, then use OperatorService.AddSearchAttributes immediately to add attributes, then use client immediately to start workflow. Note, it works sometimes and fails others.
This was referenced Jun 26, 2024
Draft
Draft
@rodrigozhou rodrigozhou self-assigned this Jun 26, 2024
This was referenced Sep 4, 2024
Merged
Merged
rodrigozhou added a commit that referenced this issue Sep 16, 2024
@rodrigozhou
Add search attributes param to sqlite.NewNamespaceConfig (#6476)
8752a90 
## What changed?
<!-- Describe what has changed in this PR -->
Add search attributes param to sqlite.NewNamespaceConfig.

## Why?
<!-- Tell your future self why have you made these changes -->
Ability to start Temporal from the CLI with custom search attributes.
Address #6195

## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Modified the CLI to not call `registerSearchAttributes`, pass it
directly to `NewNamespaceConfig`.

## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->

## Documentation
<!-- Have you made sure this change doesn't falsify anything currently
stated in `docs/`? If significant
new behavior is added, have you described that in `docs/`? -->

## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
rodrigozhou added a commit to temporalio/cli that referenced this issue Nov 18, 2024
@rodrigozhou
Fix register search attributes when starting server (#653)
17aa5b6 
<!--- Note to EXTERNAL Contributors -->
<!-- Thanks for opening a PR! 
If it is a significant code change, please **make sure there is an open
issue** for this.
We work best with you when we have accepted the idea first before you
code. -->

<!--- For ALL Contributors 👇 -->

## What was changed
<!-- Describe what has changed in this PR -->
Fix register search attributes when starting server

## Why?
<!-- Tell your future self why have you made these changes -->
Currently, search attributes are registered after the server is ready,
which can cause race condition with other processes waiting for the
server to become ready too. This PR fixes this by registering the search
attributes together with the namespaces.

Address temporalio/temporal#6195

## Checklist
<!--- add/delete as needed --->

1. Closes <!-- add issue number here -->

2. How was this tested:
<!--- Please describe how you tested your changes/how we can test them
-->

3. Any docs updates needed?
<!--- update README if applicable
      or point out where to update docs.temporal.io -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rodrigozhou rodrigozhou

Labels
potential-bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@cretz @rodrigozhou