feat: classify retryable transaction exceptions#10162
Open
memleakd wants to merge 1 commit intocodeigniter4:4.8from
Open
feat: classify retryable transaction exceptions#10162memleakd wants to merge 1 commit intocodeigniter4:4.8from
memleakd wants to merge 1 commit intocodeigniter4:4.8from
Conversation
Signed-off-by: memleakd <121398829+memleakd@users.noreply.github.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
This PR proposes adding
isRetryableTransactionException()to database connections.The goal is to give CodeIgniter a small, driver-aware way to recognize transaction failures that are reasonable candidates for a whole-transaction retry, such as deadlocks and serialization failures.
This is useful on its own for applications that already want to implement their own retry policy:
It is also intended as groundwork for a follow-up proposal to add opt-in retry support to the closure-based
transaction()helper. Establishing this driver classification policy first keeps that later retry proposal smaller: which database errors CodeIgniter considers retryable can be discussed separately from retry control flow, attempt counts, delay/backoff behavior, and callback side-effect concerns.The classifier is intentionally conservative. It only includes errors that represent transaction-level concurrency failures where retrying the whole transaction is a reasonable default.
This PR does not retry anything automatically. It only gives applications and future framework APIs a common way to ask: "is this database exception one of the transaction failures the framework considers retryable?"
API Shape Feedback
One intentional question in this PR is whether this classifier should be public now.
I proposed it as
ConnectionInterface::isRetryableTransactionException()because it has standalone value for applications and packages that already want to implement their own retry policy, and it gives future framework retry behavior a shared driver-aware classification point.That said, if the team would rather avoid exposing this as public API before automatic transaction retry support is discussed, I am happy to adjust the PR so the driver-specific classification stays internal/protected for now, and leave the public surface to a later
transaction()retry proposal.Feedback on this API shape would be very welcome.
Changes
ConnectionInterface::isRetryableTransactionException().BaseConnection.Driver Policy
Included by default:
121340001,40P0151205,396060,8177Not included by default: lock timeouts, unique constraint violations, resource-busy errors, or SQLite extended busy codes. Those can be valid retry cases in some applications, but they are more policy-dependent.
References: MySQL, PostgreSQL, SQLite, SQL Server deadlocks, SQL Server 3960, Oracle ORA-00060, Oracle ORA-08177.
Checklist: