[ochaloup]

tomjenkinson yes, it works for me, I can change the PoC and document appropriately as you suggest

jhalliday I have a doubt. Have I decoded it right that 1PC optimization does not need splitting the the xa_end and xa_prepare calls? I mean the critical part is the ordering hint given by the getOrderingHint method here?

Spliting of the xa_end and xa_prepare functionality (which is baked together in the current implementation) is need for solution of the JTA-4 (btw. I have a trouble to find the original JTA-4 bug, where is filled, please)?

[ochaloup]

jhalliday : hi Jonathan, I'm playing here with changing of the Narayana functionality. I took the approach to work only with order hint while not splitting XAResource#end and XAResource#prepare functionality for not making abrupt changes to the  Narayana core functionality.

What I ended after the time is not proposing any ordering functionality to the RecordList itself but works only with the pendingList in the critical synchronized section just before we know the XAResource#end will be called (and no more business logic will be added to the transaction, at least this should be true for the synchronous  approach).

Thus the inserted code reorders the list depending on the ordering hint. See my changes

[JBTM-3020] adding ordering for prepare phase in BasicAction to impro… · ochaloup/narayana@fa4c899 · GitHub

[JBTM-3020] subordinate XAResource to implement Narayana 1PC optimiza… · ochaloup/wildfly-transaction-client@aa831dd

[JBTM-3020] XAResource hinting the order · ochaloup/jboss-transaction-spi@bb58d25 · GitHub

• What do you think about this kind of approach?
• Do you rather consider the ordering logic should be put to the RecordList itself?
• Has you consider API of the getOrderHint to be different from what I proposed?

Thank you, anybody, for you input.

Ondra

[jhalliday]

In general I like this, but there are still issues.

It is critical that the list be reordered only in a limited way. Some invariants must be respected. Record types cannot be reordered with respect to each other. In a mixed transaction, not all AbstractRecords are XAResourceRecords and Bad Things may happen if the XAResourceRecords are reordered with respect to other types. Futhermore, some XAResourceRecords, most importantly the special case of LastResourceRecord, must not be reordered relative even to other XAResourceRecords. Thus we are really dealing with nested comparators for the ordering. Order first by the existing rules and then, ONLY within the scope of XAResourceRecords OTHER THAN LastResourceRecord, reorder using the hints.  This is a pain to implement, especially when useAlternativeOrdering enters the picture. Perhaps altering XAResourceRecord.order() so that it uses hand-crafted Uid values that munge Uid.other may be one approach, though that's really just moving the complexity around rather than addressing it.

Also, I've belatedly come to realise we're actually dealing with two orders here: prepare phase order and commit phase order. I'm unclear if we're better off with one shared hint for both purposes, or two separate hint methods. Prepare phase order, which I'll call PreparePriority to distinguish it from AbstractRecord's notion of order, is used for David's RDONLY/OK optimization, but also for LRCO. The CommitPriority, OTOH, is used for the JMS vs. DB lock-release problem. So it's possible we'll wind up with something that reorders the list into PreparePriority order, runs the prepare phase, then reorders the list into CommitPriority order before running the commit phase. Though AFAICT the common cases have the same order, so maybe it's not a big deal. I also wonder how best to communicate the nuance of partial order. LRCO has a strict exclusivity requirement: the LastResource must be last. But the other N-1 resources can be in parallel with eachother. Hinting RDONLY for PreparePriority is really a partial order indication, whereas LastResource is an absolute. It's valid, and indeed maybe preferable, to prepare all RDONLY hinted resources concurrently. i.e. are we perhaps really dealing with a List<Set<AbstractRecord>> rather than a List<AbstractRecord> ? I'm probably over-engineering here, since the average number of resources in a tx is less than 2.

[tomjenkinson]

Good points, I think we should ensure the API can be extended in the future but not necessarily implement all the functionality right now.

For example, if we have getDefaultOrderHint() as the method in XAResourceWithOrderingHint, in the future then we can then add separate getPrepareOrderHint() and getCommitOrderHint() methods if the necessity arises.

[dmlloyd]

jhalliday  wrote:

I also wonder how best to communicate the nuance of partial order. LRCO has a strict exclusivity requirement: the LastResource must be last. But the other N-1 resources can be in parallel with eachother. Hinting RDONLY for PreparePriority is really a partial order indication, whereas LastResource is an absolute. It's valid, and indeed maybe preferable, to prepare all RDONLY hinted resources concurrently. i.e. are we perhaps really dealing with a List<Set<AbstractRecord>> rather than a List<AbstractRecord> ? I'm probably over-engineering here, since the average number of resources in a tx is less than 2.

This is just a suggestion, but it is how we tend to solve similar problems in other code bases: what about using a dependency graph instead of a list for resources?  The rationale is that some things may be strictly ahead of or behind other things, but in other cases, order perhaps does not matter so much.  Graph sorting is a different sort of problem domain than list sorting but it's still pretty well-understood AFAICT, at least for the purposes described here.  You could maintain two graphs (one for prepare, one for commit) without too much complication I would think.

[ochaloup]

hi jhalliday I've got back to this issue and I made the simple change based on your last comment - aka. taking into account ordering with the record type

[JBTM-3020] adding ordering for prepare phase in BasicAction to impro… · ochaloup/narayana@c009b14 · GitHub

[JBTM-3020] subordinate XAResource to implement Narayana 1PC optimiza… · ochaloup/wildfly-transaction-client@1d1d543 · G…

[JBTM-3020] XAResource hinting the order · ochaloup/jboss-transaction-spi@2127d6d · GitHub

I have few more points.

• as you mentioned I expect there are two places where the priority hint makes sense to be used. It's during the prepare and during the commit. As the extension point I consider I can have an interface with the method returning the prepare hint and later we can add a method witch returns value for commit hint when it's needed (maybe it's better to consider two interfaces, one for prepare hint other for commit hint, you probably suggested so). Would we need it more extensible from API point of view?
• I thought the asynchronous prepare where multiple resources could be processed at once are covered when asyncPrepare is permitted. From that point I think having a set which gathers all types with the same type with the same priority sounds me a bit double work at that part. In fact maybe processing the RDONLY hinted resources concurrently does not give much benefit as it's preferably only a local call in the same JVM.
• Then I had a thought - what about making priority setup as an extension point where user could write a strategy to influence the record priority in some way?
• Do you consider this simple approach where the prepare list is prioritized but the original order and type is feasible or do you think some deep refactoring of the "order" functionality should be considered? For example I think what David suggests with the graph ordering sounds me as a new ordering strategy for the abstract record, maybe.

Thanks for you ideas

Ondra