{{Quickfixn}} Report of an issue / Proposal for change

[Andreas Meisner]

Hi @all,

this is my first email on this list and I am not yet familiar with the 
rules here.

I have found the following issue, because I have implemented a 
IMessageStore for MSSQL server (using EF) having indices on the database 
tables:

In class Session we have the method Persist(Message, string):

         protected void Persist(Message message, string messageString)
         {
             if (PersistMessages)
             {
                 SeqNumType msgSeqNum = 
message.Header.GetULong(Fields.Tags.MsgSeqNum);
                 _state.Set(msgSeqNum, messageString);
             }
             _state.IncrNextSenderMsgSeqNum();
         }

The call of Set() and IncrNextSenderMsgSeqNum() should be in one 
transaction, but currently I have a straight forward implementation with 
no dependencies between these methods. In my case a call to Set() was 
successful, but the call to IncrNextSenderMsgSeqNum() failed, leaving 
the database in an inconsitant state. I think the same could happen when 
using FileStore, but here we do not have an index that could cause an 
exception.

I can catch this in my DbMessageStore implementation, but it would be 
nicer to have only one method in this case.

So my proposal for a change is:


1. Add a new method in interface IMessageStore with a default 
implementation:

public interface IMessageStore : IDisposable
{

     // ... existing members

     public bool SetAndIncrNextSenderMsgSeqNum(SeqNumType msgSeqNum, 
string msg)
     {
         bool result = Set(msgSeqNum, msg);
         IncrNextSenderMsgSeqNum();
         return result;
     }
}

2. Add the method with the same signature to class SessionState:

         public bool SetAndIncrNextSenderMsgSeqNum(SeqNumType msgSeqNum, 
string msg)
         {
             lock (_sync) { return 
MessageStore.SetAndIncrNextSenderMsgSeqNum(msgSeqNum, msg); }
         }

3. Call the new method in class Session:

         protected void Persist(Message message, string messageString)
         {
             if (PersistMessages)
             {
                 SeqNumType msgSeqNum = 
message.Header.GetULong(Fields.Tags.MsgSeqNum);
                 _state.SetAndIncrNextSenderMsgSeqNum(msgSeqNum, 
messageString);
             }
             else
             {
                 _state.IncrNextSenderMsgSeqNum();
             }
         }

This change should be 99.9% backwards compatible and should not affect 
any existing implementation. The only difference (the 0.1%) is that the 
call to Set() and IncrNextSenderMsgSeqNum() is in one lock(_sync) in 
class SessionState, but this should be also an improvement.

Please let me know, if I can add a new pull request with these changes.


Thanks,
Andreas


-- 
Andreas Meisner
Software-Development

C/Poeto Alonso Quesada 30
35310 Santa Brígida
Las Palmas Gran Canaria
Spain

Tel. +49 176 42026763
Email: software at meisner.de
Web: http://software.meisner.de

N.I.E. Y-9624964-F