Skip to content

C++ Local transactions#442

Merged
asf-gitbox-commits merged 7 commits intoapache:mainfrom
astitcher:cpp/local-txn/precommit
Mar 13, 2026
Merged

C++ Local transactions#442
asf-gitbox-commits merged 7 commits intoapache:mainfrom
astitcher:cpp/local-txn/precommit

Conversation

@astitcher
Copy link
Copy Markdown
Member

This is the work in #437 just about ready to be merged.

@astitcher
Copy link
Copy Markdown
Member Author

I've spent today carefully considering the behaviour of the code and API design with respect to message settlement and disposition under various conditions and I've concluded that the API and implentation currently does not offer the required support for a transactioned API.

Specifically:
Most importantly: In the case that the broker fails a commit the C++ code has no way to know for sure which messages are concerned - either because they are now released back to the client in the case of a sent message or because the disposition update sent for them by the client is now undone. In either case the client has to do work unrolling or redoing work.

  • I think to make this easier we need a separate commit failed callback whilst still keeping the existing transaction failed callback

Settlements aren't handled correctly - the basic reason you'd want transactions in the first place implies that you need to delay settlement until after transaction commit, but our code does not do this - it doesn't even acknowledge you'd want to do this.

  • The practical way to achieve this would be to only allow a transactioned session to contain transactioned messages and then to settle every message in the session after a successful commit. The simplest way to do this in this API is to fail to transition to a transacted session if there are any unsettled messages on the session -- I think we need to implement this.
  • This also means that for transaction controlled messages/disposition updates we should not support immediate settlement but always defer until transaction discharge, but then in the happy path the API needs to handle the settlements itself.

@astitcher
Copy link
Copy Markdown
Member Author

We also need some way of testing the error/unhappy cases - so the transaction tester needs to simulate a broker failing commit and also doing some things that are illegal in the AMQP transaction protocol.

@gemmellr gemmellr marked this pull request as draft December 9, 2025 15:36
@astitcher astitcher force-pushed the cpp/local-txn/precommit branch 2 times, most recently from 184806f to a3e4fd4 Compare December 11, 2025 01:11
@astitcher
Copy link
Copy Markdown
Member Author

I've done a lot more work on this now, and I'm getting a lot happier about the correctness of its behaviour for outgoing messsages. I think the settlement behaviour for these is now correct under successful commit and also rollback.

I've enhanced the python broker some more and I think it now actually represents most of the transactional behaviour of a spec correct broker. It would be good to add a way to make it fail to commit - perhaps adding a transaction timeout timer would be a realistic way to do this.

@astitcher astitcher force-pushed the cpp/local-txn/precommit branch from f8e9918 to 9db81aa Compare December 12, 2025 05:11
@astitcher astitcher force-pushed the cpp/local-txn/precommit branch from 8bbaa5b to 4848d87 Compare March 6, 2026 22:24
@astitcher astitcher force-pushed the cpp/local-txn/precommit branch from 4848d87 to c3f154b Compare March 13, 2026 09:25
astitcher and others added 6 commits March 13, 2026 13:38
This code was written with the assistance of Cursor.
This returns an iterator for all unsettled transfers on a link.

Used to access transfers within a transaction or left unsettled after
one has failed.

This code was written with the assistance of Cursor.
Implement handling for declaring and discharging transactions on AMQP
sessions.

Added new callbacks which allow the application to respond to
transaction events: declaring, committing and aborting transactions;
provisionally accepting, rejecting & releasing deliveries in a
transaction.

This code was written with the assistance of Cursor.
These examples are also useful for manually testing transactions against
a broker that supports them.
Added callback to note aborted/failed transaction
You can interactively declare, commit, abort transactions;
fetch/send messages;
list pending unsettled messages;
release unsettled messages

This needs to be run against a broker that supports transactions

Probably shouldn't be in examples as it's really a tester

This code was written with the assistance of Cursor.
@astitcher astitcher force-pushed the cpp/local-txn/precommit branch from c3f154b to d90c706 Compare March 13, 2026 17:40
@astitcher astitcher marked this pull request as ready for review March 13, 2026 18:53
@astitcher
Copy link
Copy Markdown
Member Author

I'm now happy with the behaviour for incoming messages as well. I enhanced the python broker so that it can have transactions time out and this allowed me to test the behaviour under transaction commit failure and abort scenarios.

@astitcher
Copy link
Copy Markdown
Member Author

I think this work is now good enough for a tech preview style release. There are still areas that I would like to see some further work though, and I will raise extra issues for these:

  • There should be better testing for the various transaction scenarios:
    • Ideally this would include an in process broker for ease of use
  • It would be more convenient if we could configure the settlement and release behaviour for messages:
    • It would probably be more convenient for most users to automatically release any messages acquired under a transaction if the transaction is aborted or the commit fails.
    • There are some (probably rare) cases where we might want to automatically settle messages sent or acknowledged under a transaction
    • Probably the way to do this would be to add a transaction_option object to the the session - this is consistent with the current API and would be extensible.

@asf-gitbox-commits asf-gitbox-commits merged commit d90c706 into apache:main Mar 13, 2026
3 checks passed
@astitcher astitcher removed the draft label Apr 7, 2026
@gemmellr
Copy link
Copy Markdown
Member

Some feedback after looking over the changes.

Examples:

  • If explicitly releasing consumed messages after rollback, using modified(delivery-failed=true) would be the more typical than 'released' like the example does currently, in order to also increment delivery count.
  • That said, not needing to explicitly release/other the rolled back messages at all would be nicer and simpler, see settlement discussion below.
  • There is no example of doing both receiving and sending, which is one of the main transaction use cases for reliable processing of request and sending response.

The use of settlement in the client:

  • For the non-tx case, when consuming messages the client also settles when accepting/other (either by application accepting/other, or by 'auto-accept' occurring). The application doesnt need to worry about settlement at all.
  • For the tx case, the transactional acceptance is being done unsettled, which means having to deal with releasing[/modifying(delivery-failed=true)] the messages after a rollback, and [auto?] settling after commit.
  • Alternatively, sending settled transactional acceptance/other on a link with default-outcome of modified(delivery-failed=true) would mean the transactional application similarly wouldn't need to deal with settlement after discharge, or explicitly releasing (if using batch-sized credit grants, as the example does). The transaction commit would either succeed, or fail if the prior acceptances couldnt be applied and the deliveries would disappear with delivery count bumped and the messages be resent. Similarly with explicit rollback. No need for applications or client to handle releasing or settlement as it would already be done. Less bandwidth used. This settled-transactional-acceptance is what the other clients do. Matching them would seem nice, its certainly the exercised path for brokers also.
  • https://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-transactions-v1.0-os.html#doc-idp145616

Tests:
No actual-tests in tree. Seems likely to cause issues.

API:
The API essentially bakes-in lower peak tx throughput. I dont really consider this an issue needing changed, just noting it as something to consider now before the API is in the wild, in case it wasnt considered already. I think it actually makes sense as is for the simplicity.

The separate explicit declare and discharge APIs, and subsequent need to wait for discharge completion before declaring again, means the use of transactions currently mandates waiting 2 full round trips in order to begin a subsequent transaction, which could up to half the peak message rate possible for cases with prefetched messages.
This is something users ran into in qpid-jms about 10 years ago. For transacted sessions JMS only has 'commit' and 'rollback', i.e there is always a transaction from the API perspective, so we switched to pipelining the discharges and declares to effectively remove a round trip and ~double peak throughput, returning to levels like what [our] other JMS clients could do. Of course, commit and rollback are synchronous APIs in JMS which does make that easier to do than here (but to be clear, it was still a massive pain to do).

Notably though, ProtonJ2 made the same choice of separate declare and discharge operations, and forcing the 2 round trips, despite the operations being synchronous and despite the prior knowledge from qpid-jms, because it is far far simpler and transactional use cases are seemingly less typical with the non-JMS clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants