[
https://issues.apache.org/jira/browse/QPID-2128?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12790496#action_12790496
]
Xin Chen commented on QPID-2128:
--------------------------------
Some comments/questions
Dtcplugin.cpp
188: should host be checked for empty string?
172: "i < len" should be "i < len-1" since the next line is referencing dsn[i+1]
324: the qpid SQL storage provider does not log indoubt transactions, so the
recover will always get an empty list. If the qpid broker crashes after
prepare, the transaction is rolled back by SQL. This may cause data
inconsistency because other RMs may commit that transaction already. Need to
work with Stephen to figure out if this is an issue in the storage provider
implementation.
454: if the new operator throws due to low memory condition and the exception
is handled in some other place, there could be deadlock because the critical
section is not released.
454 - 464: if you call open first and add the new resource manager to the map
when it succeeds, there is no need to erase. Using an auto pointer here can
make it easy to release the object on failures.
461: what exception can be thrown by delete? The destructor of ResourceManager
is empty.
490: if close fails, is xa_close going to retried?
Overall the implementation in this file does not follow the qpid style very
well. I would wrap the global variables and functions (line 91 to 190) into a
DtcPlugin class to avoid global variables and functions as much as possible.
At least for me, the use of the critical section rmLock is not easy to follow.
Ideally thread synchronization should be encapsulated into classes (i.e. making
the class thread-safe). The ResourceManager class can hold one critical section
to synchronize communication with the qpid broker. The DtcPlugin class can use
one critical section to synchronize access to all resource manager instances.
The DtcPlugin class can implement the XA interface and the xa_switch entry
methods (xa_open, xa_close, etc) can just call the DtcPlugin methods. Qpid
implements a scoped lock/unlock (qpid.sys.ScopedLock) but I am not sure if you
want to use it in the DTC plugin.
AmqpConnection.cpp
110 - 118: this can be refactored into DtxResourceManager. The connection
should not care what needs to be done for pending transactions on connection
closing. It should notify the resource manager that the connection is closing,
like what it does for sessions.
XaTransaction.cpp
66: use InterlockedIncrement/InterlockedDecrement so you don't need this lock
204: You can set the ref count to 1 in the constructor so you don't have to
call AddRef() after creating each object.
AmqpSession.cpp
109: why not make this a method of XaTransaction?
322: remove
In ConnectionClosed and NotifyClosed, why is it important to flush phase 0
(sending dtxEnd)? The broker is going to rollback all pending transactions of a
session when it is closed. If a dtx involves multiple sessions, what is the
behavior of closing one session before commit?
DtxResourceManager.cpp
220-227: this method is named GetXaTransaction but internally we create a new
resource manager for that connection. I would refactor this method into two
methods: GetResourceMananger and then GetXaTransaction from that resource
manager.
233: this method can be combined with the previous one for the refactoring
mentioned above.
214: InternalGetXaTransaction returns nullptr only during TmDown. In that case
do we need to create the RM?
270: ideally the resource manager should manage the state of all transactions
in response to events, e.g. connection close. It seems a bit weird for a
connection to get the pending transactions and perform actions on each
transaction.
Inputlink.cpp
141: does Subscription automatically call cancel() in its destructor if the
subscription is active?
> Transaction support for the WCF channel
> ---------------------------------------
>
> Key: QPID-2128
> URL: https://issues.apache.org/jira/browse/QPID-2128
> Project: Qpid
> Issue Type: New Feature
> Components: WCF/C++ Client
> Affects Versions: 0.6
> Environment: Windows
> Reporter: Cliff Jansen
> Attachments: cppbld.patch, qpid2128_1.patch, wcfbld.patch
>
>
> Transaction support is currently lacking from the WCF channel.
> As an initial implementation, full distributed transaction support from
> within .NET is proposed using the dtx AMQP classes and creating a plugin to
> the MSDTC infrastructure to drive recovery.
> Added performance from promotable transaction enlistment is deferred to a
> later date.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
