Robert Godfrey wrote:
Thanks for your hard work on this Martin,
much appreciated
+1
Arnaud is out for the week, but when he gets back I'll ask him to have a
look at the DTX issues and figure out what the deal is with store vs
messageStore.
--Rafael
-- Rob
On 30/08/2007, Martin Ritchie <[EMAIL PROTECTED]> wrote:
There were several issues with the trunk code that has delayed my merging.
The biggest of which was getting all the tests passing. As I didn't
want an excuse for the changes to be rolled out.
I recall being asked earlier in the month to take time out of
finishing up M2 to look at why the trunk code was failing a couple of
python tests. I didn't have time then and hoped it had been done. It
had not so I had to go back _5 months_ to locate the problem.
The tests have been failing since revision 534541. I don't have time
right now to investigate the problem with the code but here is a not
exactly short list of things I had to adjust on the broker:
Commit in question added a package messageStore which duplicated most
of package store. This of course caused class issue where merging M2.
The use DistributedTransactionalContext appears to be causing the two
Python tx tests to fail. I reverted to using the
LocalTransactionalContext.
This change however required a lot of ancillary changes as the LTC
uses the store.MessageStore interface not messageStore.MessageStore.
I have commented in the code with '//DTX MessageStore' the changes I
had to make to this change.
In addition there are few dubious changes that I don't have time to
verify right now. I shall hopefully have time to study the changes in
more detail in the coming weeks.
in NonContextTransactional - deliver()
the deliverFirst boolean, that controls the if the message is added to
the front or rear of the Queue is being used to determine if the
message is enqueued. It is conceivable that this could be executed
twice, I don't know if that is a bad thing it just doesn't seem 100%
correct to me.
Comparing where enqueue is done shows that it is done at different
locations in the Local & Distributed TransactionalContexts. Again this
may be correct but I haven't had time to look. I just know that having
it in the deliver method really can't be right as this method is
called multiple times. We previously had the
message.incrementReference() here which really messed up the counts.
IMO all storage/referencing of the message for the Queue should be
done before calling deliver.
in AMQQueue - constructor
the new static queueID starts at 0 on each broker startup but what
about the queues that are recovered and have a previous ID? Do these
IDs need to be unique?
The final thing that has delayed me no end is the part porting of M2
to trunk of the testing work.
Systests are inconsistent Some use TestableMemoryMessageStore some use
MemoryMessageStore
There was a lot of file movements in the integrationtest/systest
packages. I believe I have all the changes but if those more intimate
with these tests could verify then that would be great.
Lets not roll back this change if issues are discovered. Lets work
together to go forward and address any issues. I have spent a lot of
time ensuring the tests work for me so I hope that they work for you.
--
Martin Ritchie
