## 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?
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?
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?
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?**
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?**
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?**
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
- Range for matching GetTasks is now [inclusive, exclusive), instead of (exclusive, inclusive]
- Range for matching CompleteTasksLessThan is now < max TaskID, instead of <=
- Range for admin ListTaskQueueTasks is now [inclusive, exclusive)
- support pagination for sql GetTransfer/VisibilityTasks
- improve pagination for sql GetTimer/ReplicationTasks
- new ExecutionMutableStateTaskSuite suite
* Add new public functions to the `schema/sqlite` package which help with initial schema setup and pre-registering namespaces to enable fast startup time (workaround for https://github.com/temporalio/temporal/issues/1336).
NOTE: SQLite support is still in development stage
* Import SQLite driver from Temporalite: these files originated from https://github.com/DataDog/temporalite
* Add SQLite driver layer & UT
* Use mattn SQLite driver
NOTE: SQLite support is still in development stage
* SQL: Rename SelectFromHistoryNode to RangeSelectFromHistoryNode
* SQL: Rename DeleteFromHistoryNode to RangeDeleteFromHistoryNode
* SQL: New DeleteFromHistoryNode aiming to delete one record from history node table
* New InternalHistoryNode struct containing history node info
* Modify SQL persistence layer to use key set pagination, instead of existing hacky solution
* ReadHistoryBranch in addition will allow returning history node metadata only
* Move history node filtering logic from persistence layer to upper history store layer
* New last_history_node_txn_id within execution info tracking the last valid history node
* New prev_txn_id within history node table tracking previous valid history node record
* Add db version attribute to executions table for better CAS guarantee
* 1.9.x will by default use existing next event ID for CAS, but will also support the new DB version CAS
* 1.10.x will by default use DB version CAS
Last event ID is not enough for CAS operation protection: e.g. activity heartbeat / buffer events.
The newly introduced DB version will be increased by 1 each time execution table is updated (update to mutable state record).
* Change test code to execute one SQL / CQL statement at a time.
Some database does not handle well if seeing a batch of sql statements.
This PR breaks the SQL / CQL statements used by test setup and execute
one statement at a time.
* Add version check method to SQL interface
* Remove tools version check method in favor of functions defined in persistence package
* Refactor tests to not depend on tools package
* Remove forced namespace cache refresh
* Make SQL insert into history tree idempotent
NOTE: Previously, if a namespace is newly created and not loaded
in cache, the namespace cache can do forced cache refresh reducing
the wait time for caller. This forced cache refreshment may able
to trigger a new callback for failover. Now, if a task with deleted
namespace is processed, namespace cache will trigger the above
cache refreshment and causing a deadlock. The only solution now is
to remove the namespace cache forced refreshment logic.
* Optimize SQL layer supporting batch delete
* Split DeleteFromActivityInfoMaps into DeleteFromActivityInfoMaps and DeleteAllFromActivityInfoMaps
* Split DeleteFromTimerInfoMaps into DeleteFromTimerInfoMaps and DeleteAllFromTimerInfoMaps
* Split DeleteFromChildExecutionInfoMaps into DeleteFromChildExecutionInfoMaps and DeleteAllFromChildExecutionInfoMaps
* Split DeleteFromRequestCancelInfoMaps into DeleteFromRequestCancelInfoMaps and DeleteAllFromRequestCancelInfoMaps
* Split DeleteFromSignalInfoMaps into DeleteFromSignalInfoMaps and DeleteAllFromSignalInfoMaps
* Split DeleteFromSignalsRequestedSets into DeleteFromSignalsRequestedSets and DeleteAllFromSignalsRequestedSets
