implement dual-write workflows with go-workflows #28
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
area/core
area/dependencies
vroldanbet
force-pushed
the
dual-write-go-workflow
branch
4 times, most recently
from
61e874c
to
03fced3
Compare
vroldanbet
commented
Sep 18, 2023
vroldanbet
force-pushed
the
dual-write-go-workflow
branch
4 times, most recently
from
598788b
to
b3091a1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
vroldanbet
force-pushed
the
dual-write-go-workflow
branch
3 times, most recently
from
332e60f
to
c8b02a0
Compare
ecordell
reviewed
Sep 18, 2023
ecordell
reviewed
Sep 18, 2023
vroldanbet
force-pushed
the
dual-write-go-workflow
branch
9 times, most recently
from
129b4bc
to
72f256f
Compare
vroldanbet
commented
Sep 19, 2023
vroldanbet
force-pushed
the
dual-write-go-workflow
branch
2 times, most recently
from
13ff697
to
4560b06
Compare
the main drivers to replace with go-workflows are: - we through the maintainer it's being used in production in a large scale system - type-safety with generics, less boilerplate - more approachable backend interface - redis support This commit conducted a refactoring of the code, added unit tests, and addressed a corner-case in CheckKubeResource method
vroldanbet
force-pushed
the
dual-write-go-workflow
branch
from
4560b06
to
9b559d2
Compare
ecordell
approved these changes
Sep 19, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement as alternative to MSFT's durable-task
Also undoes #27, because that was a bad idea ™️ 🙈 The amount of complexity it added did not justify it, and both implementations are different enough to become a mess to reconcile both
Main benefits:
BackendinterfaceDrawbacks:
Forked go-workflows
I forked
go-workflowsbecause I couldn't makekobuild a CGO-enabled image, so I replaced the CGO library with the Go native library in my fork. This will come in handy as I bring in the Postgres driver tooIt's also modified to set the maximum number of connections to 1. SQLite is designed to have multiple readers and a single writer, so whenever 2 processes attempt to write to the same database, a
database is locked (5) (SQLITE_BUSY)will show up. Followed the advice in mattn/go-sqlite3#274Changes to tests
Some E2E tasks were changed due to some subtle changes to how
go-workflowshandles errors:go-workflows: on panic, tasks are rescheduled and retried, client future blocks until task succeedsdurabletask-go: panic is returned as error to the client right awayFor that reason, when operations are idempotent,
go-workflowseventually succeeds, whiledurabletask-gohas the compensatory code invoked and the client needs to retry.This is the behaviour I also expected in
durabletask-go, as per my review in #16. Perhaps retries need to be explicitly configured? 🤷🏻Follow ups
CheckKubeResourceactivity fails