Oracle: run the durability agent through the shared batching mechanics - #3659
Merged
Conversation
…hanics The durability agent batches its whole recovery operation set into one command builder and executes it. Oracle's message store handed back the generic DbCommandBuilder, which emits `@` bind markers and concatenates every statement into a single command. ODP.NET rejects both -- it has no DbBatch support at all (CanCreateBatch is false, CreateBatch throws) and will not execute several statements from one command -- so the agent threw ORA-00933 / ORA-00936 / ORA-03405 on every sweep and nothing persisted in the inbox or outbox was ever recovered. Rather than give Oracle a bespoke execution path, this teaches the shared batching mechanics about statement boundaries and lets the provider decide what they mean. DatabaseOperationBatch now marks a boundary before each operation and executes whatever CompileCommands() hands back. On every provider whose driver can execute several statements from one command, StartNewCommand() is a no-op, CompileCommands() returns a single command, and the behaviour is byte for byte what it was. Oracle returns Weasel.Oracle's OracleDbCommandBuilder, which emits `:` markers, types parameters through OracleProvider, and splits. Three things the semicolon-splitting approach would have missed: - Four operations write more than one statement each (both ReleaseOrphaned variants, MoveReplayableErrorMessagesToIncoming, and PersistNodeRecord's insert per event). Splitting per operation is not enough, so those now mark their internal boundaries explicitly. - MoveReplayableErrorMessagesToIncoming binds :replayable from two different statements. AddNamedParameter finds-or-adds, so it exists once and has to be bound to both split commands. - The same operation hard-coded `@replayable` in its SQL text, which no provider-neutral consumer should do. It reads the marker off the builder now. Also documents the real reason OracleMessageStore.EnqueueAsync is a no-op: it implements IMessageDatabase directly rather than deriving from MessageDatabase, so it has no DatabaseBatcher. The durability agent does not use that path. Adds Pedro Andrade's coverage from #3615, retargeted at the new design and extended with an end-to-end assertion that the real recovery batch runs against a real Oracle database -- red-verified as ORA-03405 before this change. Fixes #3614. Co-Authored-By: Pedro Henrique Andrade Siqueira <pedroandrade03@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Brings in JasperFx/weasel#390 -- Weasel.Oracle's OracleDbCommandBuilder and the StartNewCommand()/CompileCommands() statement-boundary hooks on CommandBuilderBase that the Oracle durability fix is built on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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.
Supersedes #3615. Fixes #3614.
Builds on @pedroandrade03's diagnosis and carries their test coverage forward — retargeted at this design and extended with end-to-end assertions. Thank you for the careful root-cause work on this one.
Why not the approach in #3615
#3615 gave Oracle a provider-specific
IDatabaseOperationBatchExecutorhook, then repaired the already-compiled SQL withRegex.Replace(sql, "@(?=[A-Za-z_])", ":")andsql.Split(';'). That works, but it leaves Oracle on its own execution path forever and does string surgery on SQL that should never have been generated in the wrong shape to begin with.Two of the three things it fixed were already solved in Weasel and simply not wired up:
@markers came fromOracleMessageStore.ToCommandBuilder()returningnew Weasel.Core.DbCommandBuilder(...), whose constructor hardcodes'@'.Weasel.Oracle.CommandBuilderhas used':'all along — but it's a sibling ofDbCommandBuilder, not a subclass, so it couldn't be returned whereIMessageDatabaseexpects one.Guid→RAW(16)conversion already existed inWeasel.Oracle.CommandBuilder.The third thing is real and unavoidable: ODP.NET cannot execute several statements from one command. Verified directly against 23.7.0:
What this does instead
Teaches the shared batching mechanics about statement boundaries, and lets each provider decide what a boundary means. Paired with JasperFx/weasel#390 (Weasel 9.19.0).
DatabaseOperationBatchnow marks a boundary before each operation and executes whateverCompileCommands()returns:StartNewCommand()is a no-op,CompileCommands()returns a single command holding every statement, and behaviour is byte-for-byte what it was.OracleDbCommandBuilder, which emits:markers, setsBindByName, types parameters throughOracleProvider, and splits into oneOracleCommandper statement.No public API change —
IMessageDatabase.ToCommandBuilder()still returnsDbCommandBuilder; Oracle just returns an Oracle-shaped one.Three things per-operation splitting would have missed
ReleaseOrphanedMessages*variants,MoveReplayableErrorMessagesToIncomingOperation(insert + delete), andPersistNodeRecord(one insert per event). Splitting per operation isn't enough, so those now mark their internal boundaries explicitly.:replayableis bound by two different statements.AddNamedParameterfinds-or-adds, so the parameter exists exactly once; any index-range slicing binds it to one command and the other fails at execution. Weasel now also matches parameters by name reference (whole-token, so:p1isn't mistaken for:p11).MoveReplayableErrorMessagesToIncomingOperationhardcoded@replayablein its SQL text, so the marker abstraction leaked regardless of provider. It readsbuilder.ParameterPrefixnow.Also documented
OracleMessageStore.EnqueueAsyncis a no-op that silently drops its operation. The comment claimed it was about@parameter syntax; the real reason is thatOracleMessageStoreimplementsIMessageDatabasedirectly rather than deriving fromMessageDatabase, so it has noDatabaseBatcher. The durability agent doesn't use that path — it builds its ownDatabaseOperationBatch— and the one caller that does (OracleNodePersistence.LogRecordsAsync) works around it with direct inserts. Comment corrected, behaviour unchanged; worth its own issue.Validation
Red-baselined: reverting only
ToCommandBuilder()makes all three new end-to-end recovery tests fail withORA-03405, then pass with the fix.OracleTests(live Oracle, full suite)PersistenceTestsPostgresqlMessageStoreTestsSqliteMessageStoreTestsSqlServerTests(store + durability)dotnet build wolverine.slnx -c ReleaseNew coverage:
oracle_durability_command_translation— Pedro's cases, retargeted at the real durability operations rather than internal string helpers: bind markers, per-statement splitting, per-command parameter ownership,Guid/bool/DateTimeOffsettyping.oracle_durability_agent_recovery— runs the realDurabilityAgent.buildOperationBatch()set against a real Oracle database, and asserts orphaned messages are released and replayable dead letters move back to the inbox.🤖 Generated with Claude Code