## What changed?
Remove pinned dependency on modernc.org/sqlite.
## Why?
We added this version override when we experienced some table not found
errors. This happens when sqlite db connections are closed when
transactions fail.
We have TestSQLiteTransactionContextCancellation which passes
consistently for this new version. So we believe this issue should not
happen again.
## How did you test it?
- [x] built
- [ ] run locally and tested manually
- [x] covered by existing tests
- [ ] added new unit test(s)
- [ ] added new functional test(s)
## What changed?
Move converting `time.Time` values for `Datetime` type search attributes
to the DB plugin code.
## Why?
This gives more flexibility for the plugin implementation to deal with
it as needed.
## How did you test it?
- [x] built
- [ ] run locally and tested manually
- [x] covered by existing tests
- [ ] added new unit test(s)
- [ ] added new functional test(s)
## Potential risks
## What changed?
Add last message id in the DLQ ListQueues response. Use this last
message id in merge DLQ command if it is not provided by the user.
## Why?
tdbg dlq merge command has an optional argument --last-message-id. If
this is not provided, the system workflow created for this task will
merge/purge messages until the DLQ is empty.
This can create a loop that will not end. If the underlying issue that
pushed the messages to DLQ is still around, the newly merged tasks will
again get added to DLQ. The system workflow will proceed to merge this
task again. This can go in an infinite loop.
## How did you test it?
- [x] built
- [ ] run locally and tested manually
- [x] covered by existing tests
- [x] added new unit test(s)
- [ ] added new functional test(s)
## What changed?
Adding sql connection pool metrics from stdlib `Stats`
## Why?
Expose more metrics for config fine tuning
## How did you test it?
- [x] built
- [x] run locally and tested manually
- [x] covered by existing tests
- [ ] added new unit test(s)
- [ ] added new functional test(s)
---------
Co-authored-by: Roey Berman <[email protected]>
Co-authored-by: Roey Berman <[email protected]>
## What changed?
This PR aims to avoid usage of
[`strings.Split`](https://pkg.go.dev/strings#Split) where possible in
favor of better string splitting techniques, speficially:
[`strings.SplitN`](https://pkg.go.dev/strings#SplitN) and
[`strings.SplitSeq`](https://pkg.go.dev/strings#SplitSeq) where
appropriate.
There was also a [`strings.Fields`](https://pkg.go.dev/strings#Fields)
change I made to use
[`strings.FieldsSeq`](https://pkg.go.dev/strings#FieldsSeq) instead, and
another for S3 to use the [`path`](https://pkg.go.dev/path) package
instead of [`strings.Split`](https://pkg.go.dev/strings#Split).
## Why?
[`strings.SplitN`](https://pkg.go.dev/strings#SplitN) and
[`strings.SplitSeq`](https://pkg.go.dev/strings#SplitSeq) are often
better options in many cases, and can be _partially_ detected using
[`modernize`](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/modernize):
> `stringsseq`: replace Split in "for range strings.Split(...)" by
go1.24's more efficient `SplitSeq`, or `Fields` with `FieldSeq`.
## How did you test it?
- [X] built
- [X] run locally and tested manually
- [x] covered by existing tests
- [x] added new unit test(s)
- [ ] added new functional test(s)
## Potential risks
There are lots of potentially subtle behaviors from the `strings.Split`
(and `strings.Fields`) usage that should be accounted for. If our
existing tests don't cover those subtleties, there's risk for
introducing an unintended bug. More intricate handling/parsing
previously using the `strings` package should get extra attention from
reviewers. I've attempted to break up my changes into logical commit
chunks to aid in review / help spot potentially concerning changes.
## What changed?
Remove some code duplication for cassandra matching task queue
persistence by rewriting v2 to v1 queries.
Switch same scheme for SQL queries to use the v2 table name in the base
query and rewriting v1.
Use panic instead of returning an invalid query.
## Why?
Remove code duplication.
## How did you test it?
- [x] covered by existing tests
## What changed?
Add SQL support for matching task fairness.
Based on #8043 with some changes to reduce code duplication (can be
applied to cassandra later).
## Why?
Support all persistence implementations.
## How did you test it?
- [x] covered by existing tests
- [x] added new unit test(s)
---------
Co-authored-by: sivagirish81 <[email protected]>
## What changed?
Refactor: use new `serviceerror.New...f` constructors.
## Why?
Most of service errors now supports `fmt.Sprintf` format and they should
be used instead of `fmt.Sprintf`.
## How did you test it?
- [ ] built
- [ ] run locally and tested manually
- [x] covered by existing tests
- [ ] added new unit test(s)
- [ ] added new functional test(s)
## What changed?
<!-- Describe what has changed in this PR -->
Remove license header from every file. Because it is really hard to
follow in this PR here is the summary:
1. License header is removed from all `.go` and `.proto` files
:fireworks::fireworks:🎆.
2. `LICENSE` file in the root directory has only Temporal and Uber
copyrights.
3. 5 other `LICENSE` files added to the packages which have copyrights
different from Temporal and Uber: Datadog, Xargin, "Mat Ryer, Tyler
Bunnell and contributors".
4. `license_file` flag is removed from all code generation tools.
5. `copyright_file` flag is removed from `go:generate mockgen`
directive.
6. All copyright related targets are removed from `Makefile`.
7. Updated Temporal copyright year to 2025 everywhere.
## Why?
<!-- Tell your future self why have you made these changes -->
I double checked with legal department that it is not needed to have
license header in every file. One file per repo is enough. I put all
copyrights to the root `LICENSE` file and removed header from all other
files. Also updated tools and `Makefile`.
## What changed?
<!-- Describe what has changed in this PR -->
Add event ID to `RequestIDInfo`.
Also added `RequestIdInfos` to `DescribeWorkflowExecutionResponse`.
## Why?
<!-- Tell your future self why have you made these changes -->
Be to able to find which event was added by the give request ID.
## 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? -->
## 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) -->
## What changed?
<!-- Describe what has changed in this PR -->
Remove duplicate code: three implementations of `CreateAdminDB` were
exactly identical to the respective `CreateDB`.
## Why?
<!-- Tell your future self why have you made these changes -->
DRY.
## 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? -->
## 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) -->
## What changed?
- CHASM nodes are now internally represented as split distinctly between
Metadata and Data parts.
- Additional columns were added to SQL plugins, bumping the SQL schema
- No change was made to the Cassandra columns; instead, ExecutionManager
ensures that when running on Cassandra, the entire node continues to be
represented in a single blob, to avoid lowering Cass performance
post-change.
## Why?
Before, a ChasmNode contained both its metadata fields, and the user
data blob. Decoding a ChasmNode would cause an additional (often large,
relative to the metadata) allocation to store the inner data blob, as
well as additional memcpys while encoding/decoding. With this change,
the data field can be passed along as-is, opaquely, while the metadata
field can be decoded without additional penalty from a large data blob.
## How did you test it?
- Updated tests
---------
Co-authored-by: Yichao Yang <[email protected]>
## What changed?
SQL persistence support has been added for CHASM nodes.
- A new `chasm_node_maps` table has been added to SQL schemas
- SQL test suites updated
- SQL execution manager/util updated
- SQL plugins for `chasm_node_map` table
## Why?
- We've landed Cassandra persistence support for CHASM nodes, this
brings SQL support up to parity.
## How did you test it?
- New tests ran on all SQL plugins
- `go test -v ./...`
## Potential risks
- No application paths write chasm_node_maps yet, but change does
involve a schema change
---------
Co-authored-by: Yichao Yang <[email protected]>
## What changed?
Add version column to SQLlite, MySQL, and Postgress VisibilityStores
## Why?
There is no built in support for versioning in SQLlite, MySQL, and
Postgress, which leaves open the possibility for out-of-order updates.
We need to explicitly have a column for version to prevent this.
## How did you test it?
Manually tested that Server can run normally (still some bugs). There
will be more tests with nightly pipelines and mid pipeline version
changes to test backwards compatibility.
## Potential risks
High. A failure during table alteration or in compatibility guarantees
would entirely halt all visibility updates.
## Documentation
Not that I am aware of. We will have to close several github issues
though!
## Is hotfix candidate?
Not for cloud. Aiming for upcoming OSS release.
---------
Co-authored-by: rodrigozhou <[email protected]>
## What changed?
<!-- Describe what has changed in this PR -->
Add `OnConflictOptions` to `StartWorkflowExecution`.
- if the workflow id conflict policy is not `USE_EXISTING`, then no-op;
- if the field is nil, then no-op;
- otherwise, then it will add a `WorkflowExecutionOptionsUpdatedEvent`
to the existing running workflow.
## Why?
<!-- Tell your future self why have you made these changes -->
Ability to attach completion callbacks to existing running workflows.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
WIP: writing tests
## 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) -->
## What changed?
Use the latest release of modernc.org/sqlite library which has our fix
for the connection cancellation issue:
https://gitlab.com/cznic/sqlite/-/merge_requests/74.
We had a temporary fix merged in #6815. Reverting that as well.
## Why?
Sqlite library did not implement functions ResetSession() and IsValid()
in the database connection. sql library was discarding a database
connection when a context was canceled during a transaction. The new
version of modernc.org/sqlite has these two methods implemented.
## How did you test it?
Existing unit test to verify the connection is not lost.
## Potential risks
None
## Documentation
## Is hotfix candidate?
No
## What changed?
Use an sqlite connection that has ResetSession() and IsValid()
implemented.
## Why?
If these methods are not implemented, sql library will close the
connection when the context is cancelled during a transaction. We will
lose the database if this happens.
IsValid() will always return true in this case. We should always have a
valid database connection, otherwise we would have already lost the
database. Similarly ResetSession() does nothing.
## How did you test it?
Tried to reproduce the issue by cancelling a transaction context.
## Potential risks
None
## Documentation
## Is hotfix candidate?
No
CI is failing for the original PR #5278.
I think that's due to that PR being open from the `main` branch in
@serprex's fork.
I'll give credit to the original author when I merge.
---------
Co-authored-by: Philip Dubé <[email protected]>
## What changed?
A critical bug was fixed in PR #6538. These are the associated tests to
prevent regression.
## Why?
Adding tests for a bug fix.
## How did you test it?
Reverted the PR locally and ensured the tests catch it.
## Potential risks
N/A
Fixes#6514
## What changed?
I fixed a bug in our sql `DBHandle` that could cause us to never
reconnect to the database when an issue occurs.
## Why?
We should only update the last refresh time when we actually refresh. As
written we always update the last refresh time even when we don't
refresh.
If _every_ attempt to reconnect happens within
`sessionRefreshMinInterval` of the last attempt, the server will _never_
actually reconnect as it will just keep bumping the last refresh...
## How did you test it?
Existing tests
## Potential risks
None
## Documentation
No changes
## Is hotfix candidate?
Absolutely (cc @yiminc)
## What changed?
<!-- Describe what has changed in this PR -->
- Switch to uber gomock
- Generate workflowservice and operatorservice mock inside server repo,
instead of using imports from api-go to avoid making breaking changes
there.
## Why?
<!-- Tell your future self why have you made these changes -->
- google gomock is no longer maintained and doesn't support generics
## 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? -->
## 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) -->
## What changed?
Make import formatting a part of the build, and test validity with
"ensure-no-changes" target
## Why?
So we never discuss import order again (hopefully)
## How did you test it?
its a build change, so the build will test it!
## Potential risks
As long as build works, the only thing I risk is the ire of coworker who
want imports to be different ;)
## Documentation
N/A
## Is hotfix candidate?
No
## What changed?
<!-- Describe what has changed in this PR -->
Replace `golang.org/x/exp` with stdlib where possible. Unfortunately,
`maps` doesn't provide all methods that `expmaps` does, so I still have
`golang.org/x/exp` in `go.mod`.
## Why?
<!-- Tell your future self why have you made these changes -->
Follow up to #6018.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Build.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
No risks.
## 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/`? -->
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?
Add "start_time" nullable column to the current_executions table.
Some refactoring of the existing code - send row as a parameter rather
then every single field of that row.
## Why?
Adding column - to allow fast checking workflow creation time to avoid
noisy workflow restarts. Without it we need to get workflow start time
from a mutable state, which can affect latency.
## Is hotfix candidate?
No
## What changed?
SQL config now supports passing a connect function.
## Why?
It's currently not possible to embed a Temporal server to a host
application where the database connection is managed by the host
application and is provided as a database connector. This allows the
host application to pass a db connector rather than the the individual
sql config strings.
## How did you test it?
I have embedded this code into a custom application and verified that
the connector is working properly.
## Potential risks
This is a non-breaking change with very minimal risks.
## Is hotfix candidate?
No.
## What changed?
Both our PostgreSQL and MySQL database backends will now automatically
reconnect to the database when certain errors occur: all errors chosen
have been experienced when testing this behavior through an AWS Aurora
RDS failover of either MySQL or PostgreSQL.
For both backends we will reconnect when we see:
- `ECONNRESET`
- `ECONNABORTED`
- `ECONNREFUSED`
- `io.EOF`
- `io.ErrUnexpectedEOF`
- `database/sql/driver.ErrBadConn`
for postgres we will also reconnect on the following SQLStates:
- `25006` read-only transaction
- `57P03` cannot connect now
- `0A000` feature not supported, but ONLY when the message is `cannot
set transaction read-write mode during recovery`
for mysql we will also reconnect when we see the following error codes:
- `1040` too many connections
- `1792` read-only transaction (SQLstate `25006`)
- `1836` running in read-only mode
This logic is easily extensible should we discover more failure modes
over time
## Why?
We've had multiple community reports of Temporal problems during RDS
failover. One part of this is the fact that we wouldn't necessarily
reconnect; we were at the whims of our chosen SQL abstraction's
connection pooling logic.
## How did you test it?
I manually tested this functionality in the presence of repeated RDS
failovers:
- [x] postgres12 plugin with pq driver
- [x] postgres12 plugin with pgx driver
- [x] mysql plugin
Automated testing will be added to our regular testing pipelines once
our infrastructure friends have added the support I need (it's in
progress)
## Potential risks
We're concerned there's a correctness issue in our PostgreSQL backend
that's related to our behavior during an RDS failover. If we merge this
before I figure out what's going on, we could hide the issue and make it
harder to reproduce.
## Documentation
N/A
## Is hotfix candidate?
Yes?
## What changed?
- Refactored Nexus Registry code to merge incoming and outgoing into one
registry
- Added separate concept of "endpoints" - which is roughly analogous to
what was previously called "services"
- Upgraded the Nexus Go SDK and API dependencies accordingly
- Command handler interface was modified to accept a context given new
requirements
- Note that invoking a worker endpoint is not yet supported, there will
be some minor restructuring required to enable that
- Requires this unmerged API PR
https://github.com/temporalio/api/pull/403. The two PRs should be merged
together.
## Why?
Part of late redesign. See also https://github.com/nexus-rpc/api/pull/5
## How did you test it?
Modified existing and added new tests.
## What changed?
<!-- Describe what has changed in this PR -->
Add root workflow execution to Visibility
## Why?
<!-- Tell your future self why have you made these changes -->
Follow up of https://github.com/temporalio/temporal/pull/5520.
Be able to list all workflows based on the root execution.
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Modified existing tests to check the root workflow execution.
## Potential risks
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
No.
## 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) -->
No.
## What changed?
Noticed that `db.CreateTask` and `persistence.CreateTask` were
relatively unused and could be removed.
## How did you test it?
Current CI/CD tests
## Potential risks
Could break things if an unseen component were to use these methods;
have checked it to the best of my abilities.
## Is hotfix candidate?
No
## What changed?
We now specify `interpolateParams` by default when using MySQL for our
main.
While we don't currently support specifying `interpolateParams` for
MySQL visibility databases, I changed the type returned when serializing
`VisibilitySearchAttributes` as a first step towards doing so (MySQL 8
won't allow binary data within JSON columns, which makes sense).
While we could also have explicitly prepared and reused our statements
that requires far more work and it may not be the better approach
anyways.
## Why?
@mattrobenolt did a good job of explaining why in #5425 but I'll
summarize it here anyways.
When `interpolateParams` is false (the default) the driver will prepare
parameterized statements before executing them, meaning we need two
round-trips to the database for each query. By setting
`interpolateParams` to true the DB driver will handle interpolation and
send the query just once to the database, halving the number of round
trips necessary. This should improve the performance of all clusters
using MySQL.
## How did you test it?
Existing tests.
## Potential risks
No
## Is hotfix candidate?
This is up for discussion
## What changed?
<!-- Describe what has changed in this PR -->
Added a new persistence layer API to get a Nexus incoming service by
service ID.
## Why?
<!-- Tell your future self why have you made these changes -->
To align with new API requirements
## How did you test it?
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
Updated existing tests
---------
Co-authored-by: Roey Berman <[email protected]>
## What changed?
Sort sqlite connect attributes to ensure a deterministic string so the
sqlite pool logic works correctly.
## Why?
I noticed a lot of `database is locked (5) (SQLITE_BUSY)` errors when
using a local dev server, and tracked it down to multiple services
having separate connection to the db. The sqlite connection pool is
supposed to prevent this, but it depends on the mapping from
`ConnectAttributes` to a string being deterministic. The code that
constructed the string was iterating over a `map[string]string` without
sorting the keys making it nondeterministic.
## How did you test it?
local testing, saw no more SQLITE_BUSY errors
## What changed?
Attempting to configure a MySQL 8 Visibility database with
go-sql-driver/mysql's `interpolateParams` option will now fail.
## Why?
There are a number of reasons why this doesn't work right now so we're
going to prevent it from happening while we discuss how much we should
invest in fixing it. The current work to fix this is in
https://github.com/temporalio/temporal/pull/5428
## How did you test it?
Added a new test
## Potential risks
There should be none. Any user who has currently set up their database
this way will already have encountered exciting errors like `Cannot
create a JSON value from a string with CHARACTER SET 'binary'` and
`Unable to parse ExecutionStatus value from DB`.
## Is hotfix candidate?
That's up for discussion
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 <[email protected]>
## What changed?
<!-- Describe what has changed in this PR -->
Rename vars related to visibility.
## Why?
<!-- Tell your future self why have you made these changes -->
Ease to read code.
## 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.
## 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) -->
No
## What changed?
<!-- Describe what has changed in this PR -->
Remove support for standard visibility
## Why?
<!-- Tell your future self why have you made these changes -->
Standard visibility is deprecated.
## 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?
Add generic function to copy a value and return a pointer.
Also restore `build-tests` Makefile target.
## Why?
Simplify code.
## How did you test it?
existing tests
**What changed?**
gogo/protobuf has been replaced with Google's official go compiler.
**Why?**
gogo/protobuf has been deprecated for some time and the community is
moving on, building new tools (like vtproto) atop google's v2 compiler.
**How did you test it?**
`make test`
**Potential risks**
1. The change from embedded gogo-generated-structs to
google-generated-pointers-to-structs created a risk of nil pointer
exceptions. I've fixed all the ones our tests found but it's possible
there are more lurking in the new code.
2. This change may cause our performance to decrease. Certainly
encoding/deconding of proto objects will become slower, but the overuse
of pointers by the google compiler may negatively affect our overall
performance. We'll need to keep an eye on the GC stats
3. This breaks the HTTP API. We will not support [shortand payload
encoding](https://github.com/temporalio/proposals/blob/master/api/http-api.md#payload-formatting)
in this first pass; that will come once this initial work is in testing.
**Breaking changes for developers**
- `*time.Time` in proto structs will now be
[timestamppb.Timestamp](https://pkg.go.dev/google.golang.org/[email protected]/types/known/timestamppb#section-documentation)
- `*time.Duration` will now be
[durationpb.Duration](https://pkg.go.dev/google.golang.org/protobuf/types/known/durationpb)
- V2-generated structs embed locks, so you cannot dereference them. `go
vet` will scream at you about this. If you need a copy, use
`proto.Clone`.
- If the performance of this sucks then I will either update our code
generator to add shallow-clone methods or hand-roll the ones we need
- Proto enums will, when formatted to JSON, now be in
`SCREAMING_SNAKE_CASE` rather than `PascalCase`. We decided (in
discussion with the SDK team) that now was as good a time as any to rip
the bandage off.
- Proto objects, or objects embedding protos, cannot be compared using
`reflect.DeepEqual` or _anything_ that uses it. This includes `testify`
and `mock` equality testers!
- You will need to use the `common/testing/protorequire`,
`common/testing/protoassert`, or `common/testing/protomock` packages
instead. I've implemented proto-compatible matchers and assertions there
for all cases I've encountered
- If you need `reflect.DeepEqual` for any reason you can use
`go.temporal.io/api/temporalproto.DeepEqual` instead
Note that history loading will not be impacted by the JSON changes: I
rewrote history loading to dynamically fix incoming history JSON data
(like all our other sdks); you can find this code in [my fork of our go
API](https://github.com/tdeebswihart/temporal-api-go/blob/master/internal/temporalhistoryv1/load.go)
alongside its tests.
**🚨Sharp Edges Introduced🚨**
Beware `*timestamppb.Timestamp.AsTime()`. If you need to extract a time
value from a proto time (timestamppb) **always** make sure to check
whether it's nil first. When the proto object is `nil` `AsTime()` will
return a non-zero time at the proto epoch: UTC midnight on January 1,
1970.
I've made this mistake multiple times during this transition and each
time it's been a pain to debug
**Is hotfix candidate?**
No.
<!-- Describe what has changed in this PR -->
**What changed?**
Adding a new field MessageCount to ListQueues response.
<!-- Tell your future self why have you made these changes -->
**Why?**
This field will be useful for operators who wants to get details about
DLQs
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit tests with 100% test coverage.
<!-- 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?**
Store parent workflow execution in visibility
<!-- Tell your future self why have you made these changes -->
**Why?**
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Added integration test
<!-- 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?**
Adding ListQueues API to QueueV2 and implementing it for Cassandra and
SQL
<!-- Tell your future self why have you made these changes -->
**Why?**
This API can be used by operators to list different types of queues
including DLQs.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit tests with 100% coverage.
<!-- 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?**
Adding DescribeDLQJob and CancelDLQJob APIs to admin_handler.
<!-- Tell your future self why have you made these changes -->
**Why?**
DescribeDLQJob API will be used by operators to find the status of a DLQ
job such as MergeDLQTasks or PurgeDLQTasks.
CancelDLQJob API will be used by operators to delete a Merge or Purge
DLQ task.
a command will be added to tdbg to call these API. End users will be
using tdbg to invoke these APIs.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit tests
<!-- 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?**
Adding FOR UPDATE clause when fetching queueV2 metadata and removing
unused field version from queue metadata.
<!-- Tell your future self why have you made these changes -->
**Why?**
If a queueV2 metadata row is fetched in a transaction, we have to place
a lock on that row
to avoid another transaction reading from that line. This can happen
when two threads are trying to
delete messages from the queue. Two of them will update queue metadata
and write it back. This will lead
to a race condition. By adding 'FOR UPDATE' SQL clause, the first
transaction will get an exclusive lock
on the row. The second transaction will wait until the first transaction
is complete or rollback.
Also, removing the version field from QueueV2 metadata table. We will
not be using this field since we are using sql transactions.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit test. Executed through the debugger to make sure
SelectFromQueueV2MetadataForUpdate() is called instead of
SelectFromQueueV2Metadata() when executed inside a trancation.
<!-- 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?**
Add ExecutionDuration and StateTransitionCount to SQL visibility
schemas.
Add unit test to validate all system search attributes are mapped to SQL
DB column name.
Moved StateTransitionCount to visibility Close request only.
<!-- Tell your future self why have you made these changes -->
**Why?**
Match features with Elasticsearch.
https://github.com/temporalio/temporal/issues/4942
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Run unit tests.
<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
No.
<!-- 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?**
Adding RangeDeleteMessages() and CreateQueue() methods for SQL version
of persistence.QueueV2, which supersedes the persistence.Queue
interface.
<!-- Tell your future self why have you made these changes -->
**Why?**
We plan on using this for the upcoming history task DLQ project.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit tests. Code has 100% unit test coverage.
<!-- 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?**
---------
Co-authored-by: Michael Snowden <[email protected]>
<!-- Describe what has changed in this PR -->
**What changed?**
Adding EnqueueMessage() and ReadMessage() methods for SQL version of
persistence.QueueV2, which supersedes the persistence.Queue interface.
The implementations for delete and merge are left out for now and will
be implemented later.
<!-- Tell your future self why have you made these changes -->
**Why?**
We plan on using this for the upcoming history task DLQ project.
<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Unit tests.
<!-- 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?**
`Driver` interface (with two implementations) was added to the
postgresql persistence package in order to support other drivers than
`lib/pq`. Fixes https://github.com/temporalio/temporal/issues/1775
Essentially, this PR does
https://github.com/temporalio/temporal/pull/4914 in a
backward-compatible and extendable way
<!-- Tell your future self why have you made these changes -->
**Why?**
Since `lib/pq` does not support multi-node clusters
<!-- 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**
No, since you should explicitly change the driver in your config file.
However, `connectAttributes` depend on the driver (e.g.,
`binary_parameters` is supported only by `lib/pq` and is not a part of
postresql standard), so one should be careful changing drivers in
production.
<!-- 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?**
Removed the custom utility functions `Min` & `Max` and replaced them
with the new builtin functions. See
https://tip.golang.org/ref/spec#Min_and_max
<!-- Tell your future self why have you made these changes -->
**Why?**
Less code!
<!-- 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**
A subtle implementation detail I missed?
<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
