-
Notifications
You must be signed in to change notification settings - Fork 37
Db retries #146
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.
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
Db retries #146
Conversation
|
||
go 1.22.0 | ||
|
||
toolchain go1.25.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed for template
type DBOSError struct { | ||
Message string // Human-readable error message | ||
Code DBOSErrorCode // Error type code for programmatic handling | ||
IsBase bool // Internal errors that shouldn't be caught by user code |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused
This reverts commit 9127d2c.
AutoRemove: true, | ||
Binds: []string{ | ||
fmt.Sprintf("%s:%s", hostPgDataVolumeName, pgData), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out that an explicit volume binding is required. On MacOS, exporting PGDATA
is enough to instruct Docker desktop to mount the right thing. Docker desktop uses a light VM to run postgres, and persist this volume across container restarts.
On the github action runner -- and likely on other environments too? The mount is not persisted across containers restarts.
wfID, err := ctx.GetWorkflowID() | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to get workflow ID: %w", err) | ||
} | ||
err = ctx.(*dbosContext).systemDB.updateWorkflowOutcome(WithoutCancel(ctx), updateWorkflowOutcomeDBInput{ | ||
workflowID: wfID, | ||
status: WorkflowStatusError, | ||
err: newWorkflowUnexpectedInputType(fqn, fmt.Sprintf("%T", typedInput), fmt.Sprintf("%T", input)), | ||
}) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to record unexpected input type error: %w", err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not required because typedErasedWorkflow
is always call within RunWorkflow
, in a goroutine that update the workflow outcome.
mockCtx.AssertExpectations(t) | ||
mockChildHandle.AssertExpectations(t) | ||
mockGenericHandle.AssertExpectations(t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A mock built with mocks.NewMoc...
already checks expectations before destroying the object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, important tests and fixes!
Address #93
This PR adds a
retry
method that can take in another function for retry until a specific condition is met.retry
can be configured with functional options and has aretryWithResult
generic counterpart for functions returning a value.All system database methods are now called within
retry
orretryWithResult
.For example
retry performs exponential backoff with jitter. It defaults to infinite retries.
The default condition for retrying is
isRetryablePGError
, which matches the function returned error with a set of connection errors (postgres codes, pgx connection errors,net.Error
)Note: in the particular case of RunWorkflow, which does manage transactions, the entire function has to be retried. This is because the transaction object becomes invalid as soon as Commit/Rollback have been called, regardless of whether the operation was successful. See https://github.com/jackc/pgx/blob/61d3c965ad442cc14d6b0e39e0ab3821f3684c03/tx.go#L180
This PR also improves
DBOSError
with the ability to unwrap the underlying error, if any.