Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Acceptor Dynamic Session Templates (2nd version) #650

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pvkv1799
Copy link
Contributor

This PR enables "*" wildcards in Acceptor's session configuration (for SenderCompID, SenderSubID, SenderLocationID, TargetCompID, TargetSubID, and TargetLocationID).
Such session templates can be defined either at start-up or as an ad-hoc operation, just like "normal" sessions.
When processing a new incoming connection, if the incoming SessionID is not equal to any of the created sessions, it is then matched vs. templates with wildcards. Upon the first match, the new Session is created with the same settings as the template ones, just changing wildcards to the actual values from the incoming SessionID.

This PR is an alternative for #607 to address architectural concerns.
Here, instead of changing the Session class, I added the new SessionProvider class, implementing in it most of the logic for creating Sessions on-the-fly. Change in SocketReader is to call SessionProvider.GetSession instead of Session.LookupSession.
In ThreadedSocketAcceptor, the main change is in CreateSession, to check for wildcards in session settings and do not create such a Session immediately, but record it as a template while still creating AcceptorSocketDescriptor for it. I separated the block of code to do the actual session creation into internal CreateAcceptorSession, so SessionProvider can call it when it needs to create a new Session on-the-fly. Placing SetSessionSettings also into ThreadedSocketAcceptor maybe not the best option; still, it seems to fit OK there to change settings for the entire Acceptor.
I tried replacing static Session.LookupSession with also static SessionProvider.GetSession, but that seems to be logically incorrect, as ThreadedSocketAcceptor is not static, so theoretically, several of its instances may be created, and then they all will share the same SessionTemplate, which does not look correct. So ThreadedSocketAcceptor has to create a SessionProvider object, and pass it through AcceptorSocketDescriptor -> ThreadedSocketReactor -> ClientHandlerThread to SocketReader, causing minor changes in all those classes. If you think those changes are not necessary, and there can be only one ThreadedSocketAcceptor, then SessionProvider can be made static. In this case, ThreadedSocketAcceptor will need to clean up the SessionProvider on its Stop (as static SessionProvider will keep templates between acceptor stop/start, causing duplication of the templates).

Your remarks would be highly appreciated!

@SenyaMur
Copy link

SenyaMur commented Sep 1, 2020

It's very nice PR.
Can you create interface for SessionProvider aka ISessionProvider

public inteface ISessionProvider {
  Session GetSession(SessionID sessionID);
}

Use SessionProvider instead of SessionProvider every where
Add ISessionProvider as paremeter to ThreadedSocketAcceptor
SessionProvider change visibility to public class
Create SessionProviderDefault where base behaivour and use it if sessionProvider parameter in ThreadedSocketAcceptor is not set

public Session GetSession(SessionID sessionID)
        {
            Session session = Session.LookupSession(sessionID);
            return session;
        }

@pvkv1799
Copy link
Contributor Author

pvkv1799 commented Sep 2, 2020

@SenyaMur - thank you for the ideas, however there is an issue with changing it to an interface.
Apart from this one method:

public inteface ISessionProvider {
  Session GetSession(SessionID sessionID);
}

the interface has also to provide

void AddTemplate(SessionID sessionID, Dictionary dict, ThreadedSocketAcceptor acceptor, AcceptorSocketDescriptor descriptor)

to be used in ThreadedSocketAcceptor.CreateSession(SessionID sessionID, Dictionary dict), and making AddTemplate a public method causes an error, because AcceptorSocketDescriptor is an internal class, which leads to Error CS0051 Inconsistent accessibility: parameter type 'AcceptorSocketDescriptor' is less accessible than method 'SessionProvider.AddTemplate(SessionID, Dictionary, ThreadedSocketAcceptor, AcceptorSocketDescriptor)'

I do not feel right to change AcceptorSocketDescriptor's accessibility...

@gbirchmeier, @mgatny : could you please advise regarding the proposed change? I would highly appreciate your feedback on this PR. Thank you!

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.

2 participants