This PR is intended to merge 4 months of work in the `nexus` feature
branch into `main`.
The functionality it brings is:
- Dispatching Nexus Tasks by namespace and task queue
- Internal implementation of the Incoming Service Registry - not yet
exposed
- Attaching workflow close callbacks on `StartWorkflowExecutionRequest`
and processing of those callbacks
There's more to come and some of the callback code will be refactored
into a new plugin architecture
(https://github.com/temporalio/temporal/pull/5446) soon.
---------
Co-authored-by: PJ Doerner <pj.doerner@temporal.io>
## What changed?
Our shard info update logic now monitors how many tasks have been
completed across all queues. If enough changes have occurred it will
persist changes to our database even if `ShardUpdateMinInterval` time
hasn't elapsed since the last change. The time between updates and the
number of tasks completed per update can be monitored using
`tasks_per_shardinfo_update` and `time_between_shardinfo_updates`
metrics.
While here I extracted the metric calculation from the updateShardInfo function.
In the previous code we'd stop updating metrics if we stopped updating shard info
which isn't the behavior we want. Now it's handled by a separate goroutine that
grabs the shard's read lock as needed.
Follow-up work will handle updating shard info based on replication task
completion if it makes sense to.
## Why?
When we have a large number of shards updating shard info every five
minutes (the default) can be costly. If we persist after enough changes
have occurred we can increase this interval and reduce the load on our
DB.
## How did you test it?
I added a handful of new unit tests to verify the behavior
## Potential risks
None
## Is hotfix candidate?
No
## What changed?
Moving history events cache to host level.
## Why?
To improve cache utilization
## How did you test it?
Unit tests and integration tests
## Potential risks
## Is hotfix candidate?
No
## What changed?
Keep history cache dynamic config backward compatible
## Why?
Reuse the old dynamic config for shard level cache.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
## What changed?
Internal errors encountered during task processing will be dropped (or
sent to the DLQ) when this new config is enabled.
## Why?
These errors represent unprocessable tasks, so should not block our task
queues.
## How did you test it?
## Potential risks
We're not 100% certain that we only return internal errors when a task
is unprocessable, so this will be enabled by dynamicconfig for now.
## Is hotfix candidate?
## What changed?
Adding code to send a task to DLQ after a number of attempts.
This number can be configured through dynamic config.
## Why?
Repeatedly failing task can be moved to DLQ and inspected.
## How did you test it?
Unit tests
## Potential risks
None
## Is hotfix candidate?
No
## What changed?
Remove the `history.queueMaxReaderCount` dynamic config in favor of a
per queue config in the form of `history.{queue}QueueMaxReaderCount`
## Why?
Nexus Callback and Operation multi-destination queues need to increase
the default limit without applying the increased limit to other queues.
## How did you test it?
Ran existing tests.
## Potential risks
Users may have altered the existing configuration in their production
setup but that's highly unlikely and it should be fine to add a warning
in the release notes to mention that the old value is no longer applied.
## What changed?
<!-- Describe what has changed in this PR -->
Dynamic config to allow list of values for single value search attribute types.
## Why?
<!-- Tell your future self why have you made these changes -->
Able to allow per namespace, ease migration.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? -->
Existing unit tests.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this change to production? -->
No.
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
No.
## What changed?
<!-- Describe what has changed in this PR -->
Removing deprecated Visiblity dynamic configs.
## Why?
<!-- Tell your future self why have you made these changes -->
They have been replaced with new ones.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit test? Checked in staging env? -->
Existing tests.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this change to production? -->
People need to update their configs if they are using the old keys.
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to be sent to the broader community? (Yes/No) -->
No.
## What changed?
<!-- Describe what has changed in this PR -->
Add replicate local generated history events feature and gated with
dynamic config
## Why?
<!-- Tell your future self why have you made these changes -->
To support replicate events that generated by current cluster
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Unit test and new integration test added
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
N/a
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
no
## What changed?
<!-- Describe what has changed in this PR -->
Don't drop query results on heartbeat workflow task.
I also renamed confusing `workflowTaskHeartbeating` local var and added
comments to explain WT heartbeat logic.
## Why?
<!-- Tell your future self why have you made these changes -->
Currently, if SDKs completes WT with `ForceNewWorkflowTask` set to
`true` and w/o specifying any commands or messages but query results,
these query results are dropped by server and query is sent to worker
again. After bunch internal discussion we don't see why server does it
for queries while normally accepting commands and messages at the same
time. So I decided to remove this "drop" for the parity with messages
and commands. After all, currently, if all what workflow does is a local
activity, there is no way to query it.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Removed existing UT.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
User code which relays on the fact that local activity must be completed
before worker serves the query will see these queries completed first.
But this is not a guarantee which Temporal provides.
## Is hotfix candidate?
<!-- Is this PR a hotfix candidate or does it require a notification to
be sent to the broader community? (Yes/No) -->
No.
---------
Co-authored-by: Spencer Judge <sjudge@hey.com>
<!-- Describe what has changed in this PR -->
**What changed?**
🚀 This PR turns on the new history task DLQ by default.
<!-- Tell your future self why have you made these changes -->
**Why?**
Previously, if history tasks, e.g. transfer, timer, etc., failed with
terminal errors (i.e. data corruption issues), we would drop the tasks
completely 💀 . Now, we guarantee that they will be sent to the history
task DLQ. We retry sends forever until they succeed. The history task
DLQ supports operation via `tdbg` (under `cmd/tools/tdbg/main.go`).
There is also a previous DLQ which only worked for replication and
namespace replication tasks, but that one never worked for history
tasks. As a result, there are two version of the dlq in tdbg. To use the
new one, which is the only way to interact with the history task DLQ,
you can run the following command: `tdbg dlq --dlq-version v2
<subcommand>`. The supported subcommands are:
- 📖 `read` view messages in DLQs
- 🗑️ `purge` drop messages from a DLQ
- ♻️ `merge` re-enqueue (AKA retry) DLQ'd messages
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
🧪 **How did you test it?**
There are end-to-end tests in the `tests` package which create workflows
with data corruption issues and retry them using `tdbg`, verifying that
they eventually succeed.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
I think the biggest risk is a bug in the ability to write tasks to the
DLQ because we retry such attempts infinitely.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No, we plan on releasing this as part of the next minor release because
it has many dependencies.
<!-- Describe what has changed in this PR -->
**What changed?**
Removed direct calls to persistence for `PollWorkflowTaskQueue` out of
frontend service
The logic to get histories was moved to matching service
Matching service now includes history as part of its
`PollWorkflowTaskQueueResponse`
Actual behavior should be unchanged. Existing logic is kept as part of
`workflow_handler_deprecated` since it will be needed if frontend gets a
response from an older matching instance without a history.
Uses updated protos from
https://github.com/temporalio/temporal/pull/4968
<!-- Tell your future self why have you made these changes -->
**Why?**
Frontend shouldn't be responsible for doing database queries.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Existing tests and new test for error returned to matching from
persistence
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
Inconsistent handling of errors between old logic and new
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
<!-- Describe what has changed in this PR -->
**What changed?**
This PR adds a dynamic config flag which controls the rollout of the
persistence queue V2 backend for writing history replication tasks to
the DLQ.
<!-- Tell your future self why have you made these changes -->
**Why?**
Our first priority of the queue v2 migration is to make it work for
history replication tasks, so this PR adds the ability for us to roll
that out.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
I added branches to our history replciation DLQ integration test that
turn on this dynamic config flag.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
<!-- Describe what has changed in this PR -->
**What changed?**
Implement the batching functionality on history event replication task
<!-- Tell your future self why have you made these changes -->
**Why?**
Improve performance of history event replication task
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
manually tested locally. Unit tests.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
N/a. Feature is behind a feature flag.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
no
<!-- Describe what has changed in this PR -->
**What changed?**
This is the final PR to actually wire up the history task DLQ.
_Note about the PR structure: The merge base is dlq-5-base, but it will
be changed to dlq-feature after
https://github.com/temporalio/temporal/pull/4875 is merged. I did it
this way because 4875 is a PR against main (because it's independent of
the DLQ project), but this PR depends on both that and another PR in the
dlq-feature branch._
<!-- Tell your future self why have you made these changes -->
**Why?**
I made this so that we can actually use the DLQ. All previous PRs were
basically library code--this hooks it up to our main entry point.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
There's an fx test which verifies that any executable we create will use
the DLQ iff the dynamic config flag is on.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
The dynamic config defaults to false, and I added a comment to not turn
it on if you aren't using Cassandra.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
Added a new dynamic config option,
`history.taskSchedulerRateLimiterStartupDelay`, to control a delay for
task scheduler rate limiting on service startup. Defaults to 10s
<!-- Tell your future self why have you made these changes -->
**Why?**
On service start, task processing has to catch up and replays a large
number of previously completed tasks and we do not want to rate limit
during this period.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Existing tests
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
None
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
<!-- Describe what has changed in this PR -->
**What changed?**
Add feature flag for host level workflow cache
<!-- Tell your future self why have you made these changes -->
**Why?**
Add feature flag for host level workflow cache to protect the new
component.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
only config changes. Existing tests covers the case.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
<!-- Describe what has changed in this PR -->
**What changed?**
Move mutable state cache to host level.
<!-- Tell your future self why have you made these changes -->
**Why?**
For better utilization. This will be better to handle hot shard problem
with full pinned items in cache.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Local server + unit tests
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
N/A
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
<!-- Describe what has changed in this PR -->
**What changed?**
This PR deletes all the code for the old workflow-based workflow
archival feature.
<!-- Tell your future self why have you made these changes -->
**Why?**
Originally, the tool to archive workflows was itself written as a
workflow. There were some complications that arose from this, so we
decided to build the archival system closer to the metal as its own
queue similar to the timer and transfer queues. In a previous release,
we disabled the archival workflow, and enabled the new archival queue.
Now that this process is done, we can delete all the dead code for the
archival workflow.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
I just searched for "archiv" in our codebase and looked at every
occurrence.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
Maybe there are people still using the old archival workflow, and
they'll now need to migrate.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
I don't think so.
<!-- Describe what has changed in this PR -->
All new code is gated behind the frontend.accessHistoryFraction dynamic
config. The new code is meant to be a port of the original FE code, but
primarily executed by the History service moving forward. Because this
change affects both FE and History, it requires a prolonged deprecation
process where:
- upon the 1st minor release (upcoming), the dynamic config must remain
disabled by default
- by the 2nd minor release, the dynamic config to be switched to be
enabled by default
- starting from the 3rd minor release, the deprecated code will be
removed
This change is staged into the following 4 major steps:
- DEPRECATE - refactor FE logic/tests into deprecated files (no logic
change)
- DUPLICATE - dup deprecated logic to history service (verbatim
copy/paste)
- PLUMBING - introduce rpc and handlers along with basic necessities
(hooking up)
- IMPORTANT - call-by-call replacement (requires careful review)
<!-- Tell your future self why have you made these changes -->
This is a big refactor to move business logic from FE to History service.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
- Ported FE unit tests to the History service layer
- Spot checked by running in dev environment
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
<!-- Describe what has changed in this PR -->
**What changed?**
Wire EagerNamespaceRefresher into the replication task processing logic
<!-- Tell your future self why have you made these changes -->
**Why?**
Eagerly sync namespace from remote cluster when getting an unknown
namespace replication task
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit test. Plan to do local test before merging.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
Replication stack may fail.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
remove the cache initial size from lru cache and related configs. also
refactor putInternal to better support different scenarios.
and added more unittests for cache size.
<!-- Tell your future self why have you made these changes -->
**Why?**
in https://github.com/temporalio/temporal/pull/4621 I change the history
cache to use item bytes instead item count. However it accidentally
touched default number of key when cache initialized
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
unittest
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
Memory usage increase and pined mutable state will never leave the cache, which cause block workflow.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
<!-- Describe what has changed in this PR -->
**What changed?**
This adds support to the shard controller to delay closing a shard due
to membership changes, until either a shard ownership lost error, or up
to a max delay time, intended to be a few seconds. This is the server
side dual of https://github.com/temporalio/temporal/pull/4652 .
<!-- Tell your future self why have you made these changes -->
**Why?**
The intent is to allow an existing shard owner to keep processing
requests until the new shard owner has acquired the shard. In testing,
this has reduced request latencies during shard transitions, like
history service upgrades and restarts.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit tests, and has been tested in a staging environment.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
The behavior is off by default.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
This adds configurable behavior for the history client to cache shard
ownership information, instead of checking membership for each request.
Cache entries are evicted either for shard ownership lost errors, or for
any error that might indicate the history instance is no longer
available, including timeouts.
The new behavior is not enabled by default.
<!-- Tell your future self why have you made these changes -->
**Why?**
History requests could timeout awaiting shard acquisition at a new
owner, even though the old owner still has ownership. By continuing to
send requests to owners until they signal ownership lost, we can avoid
the unnecessary timeouts and retries.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
New unit tests are added & verified. These changes were also tested in a
staging environment.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
The behavior is not enabled by default, so there's no risk with just the
code in place.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
change history cache key from history.eventsCache*Size to
history.eventsCache*SizeBytes
<!-- Tell your future self why have you made these changes -->
It will remove the key name confusion since cache size definition
changed in: https://github.com/temporalio/temporal/pull/4621
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
unittests
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
<!-- Describe what has changed in this PR -->
**What changed?**
* Added a new possible value for the `caller-type` header: `operator`
* This header should be set by web UI and tctl to indicate that requests
should be given higher precedence
* Rate limiters for persistence, history, matching, and visibility were
updated with a new priority corresponding to `operator` requests
* These new priorities have a lower RPS limit to prevent abusing the
header to get high priority for all requests. Currently the operator
priority is allowed 20% of the rate of the existing rate limiters
<!-- Tell your future self why have you made these changes -->
**Why?**
When the system is being throttled, users would like to be able to debug
using the web UI and/or tctl without running into rate limit errors
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
New unit tests
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
The operator RPS is set too low leading to new rate limit errors
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
<!-- Describe what has changed in this PR -->
**What changed?**
I removed the durable archival flag which switches between the archival
workflow and the archival queue.
<!-- Tell your future self why have you made these changes -->
**Why?**
Because we just use the archival queue unconditionally now, and the old
code path is something like 5K lines, so being able to delete it will
slim us down a lot.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Durable archival was [released in
February](https://github.com/temporalio/temporal/releases/tag/v1.20.0)
as part of 1.20.0. There was one bug that caused schedules-based
workflows to not be archived, but that was fixed. Other than that, there
have been no issues, and so I think it's been long enough, and it's been
battle-tested enough that we can now remove its predecessor.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
If there's a bug in the archival queue, we won't have a way to switch
back to the archival workflow any more.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
When a history instance starts, support a configurable (defaulting to
zero) delay before joining membership.
<!-- Tell your future self why have you made these changes -->
**Why?**
In environments where the history service is running via a Kubernetes
Deployment, rolling restarts or image upgrades cause considerable shard
movement, because the Deployment will simultaneously terminate one pod &
create a new one. By configuring a non-zero delay on the order of
seconds, the shard movement due to the terminating pod can be separated
from the shard movement of the newly created pod. Overall, this reduces
the impact to user api calls during the change.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
This has been tested in a staging environment.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
With the default setting of zero, no risk.
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
* calculate ms size on db load
* enforce mutable state size limit
* comments
* track size on ms update
* update size on hbuilder change
* cleanup
* cleanup
* only check buffered events size when requested
* refactor size checks
* tests
* feedback
* check for existence before subtracting size
* tests
* change existence check
* feedback
* tests
* include buffered events if they will not be flushed
* include keys in size
* math
* update size in retry activity
* cleanup
