-
Notifications
You must be signed in to change notification settings - Fork 138
server: support distributed sessions #232
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
base: main
Are you sure you want to change the base?
Conversation
This looks right to me -- session state is the correct thing to store rather than a transport. Should this be implemented for the SSE transport as well? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Session change LGTM, just one minor question about NewStreamableServerTransport
- SessionState holds the state of a ServerSession. - SessionStore allows arbitrary storage for SessionState. - ServerSession.InitSession provides a session ID, store and state to a ServerSession.
Support stateless streamable sessions by adding a GetSessionID function to StreamableHTTPOptions. If GetSessionID returns "", the session is stateless, and no validation is performed. This is implemented by providing the session a trivial initialization state. To implement this, some parts of modelcontextprotocol#232 (distributed sessions) are copied over, since they add an API for creating an already-initialized session. In total, the following new API is added: - StreamableHTTPOptions.GetSessionID - ServerSessionOptions (a new parameter to Server.Connect) - ServerSessionState - ClientSessionOptions (a new parameter to Client.Connect, for symmetry) For modelcontextprotocol#10
Support stateless streamable sessions by adding a GetSessionID function to StreamableHTTPOptions. If GetSessionID returns "", the session is stateless, and no validation is performed. This is implemented by providing the session a trivial initialization state. To implement this, some parts of modelcontextprotocol#232 (distributed sessions) are copied over, since they add an API for creating an already-initialized session. In total, the following new API is added: - StreamableHTTPOptions.GetSessionID - ServerSessionOptions (a new parameter to Server.Connect) - ServerSessionState - ClientSessionOptions (a new parameter to Client.Connect, for symmetry) For modelcontextprotocol#10 Co-Authored-By: 36760115+ln-12@users.noreply.github.com
Support stateless streamable sessions by adding a GetSessionID function to StreamableHTTPOptions. If GetSessionID returns "", the session is stateless, and no validation is performed. This is implemented by providing the session a trivial initialization state. To implement this, some parts of modelcontextprotocol#232 (distributed sessions) are copied over, since they add an API for creating an already-initialized session. In total, the following new API is added: - StreamableHTTPOptions.GetSessionID - ServerSessionOptions (a new parameter to Server.Connect) - ServerSessionState - ClientSessionOptions (a new parameter to Client.Connect, for symmetry) For modelcontextprotocol#10 Co-Authored-By: 36760115+ln-12@users.noreply.github.com
I've converted this to draft status. I don't think we fully understand what it means to have a distributed server. For example, if a server receives a request and then sends a nested request back, how can we be sure that the same server process will receive the reply? I don't think having a session store helps with that problem. So we're going to wait until this gets resolved in the specification. |
@jba what do you mean by sends nested request back? my understanding of the spec is that it is potentially possible to operate in both streamable and simple request/response modes, as long as session is distributed and transport does not assume streams only that should be compatible, right? |
What if the server in step 1 is in a different process from the server in step 3? |
I think these are essentially two different modes, stremableHTTP and stremableHTTP + SSE, SSE is optional according to the spec, hence, the concrete servers using SDK which dont utilise SSE may omit that part completely. when it comes to the stremableHTTP + SSE and the provided use-case, regardless of session storage there is also resumability aspect, hence the server needs to have the ability to replay messages based on "Last-event-id" when resume request from the client arrives, as the result in multi container server setup (where is more than one instance of MCP server in a cluster), I think we will need to provide an interface or a fallback so it is possible to replay these messages, something like RMQ. The protocol itself omits the details on mechanism for message replay, but on the interface level they assume resumability to be functional if I read it correctly, expecting the request to land on the same server limits the implemtation options to sticky session on the edge. |
Support stateless streamable sessions by adding a GetSessionID function to StreamableHTTPOptions. If GetSessionID returns "", the session is stateless, and no validation is performed. This is implemented by providing the session a trivial initialization state. To implement this, some parts of modelcontextprotocol#232 (distributed sessions) are copied over, since they add an API for creating an already-initialized session. In total, the following new API is added: - StreamableHTTPOptions.GetSessionID - ServerSessionOptions (a new parameter to Server.Connect) - ServerSessionState - ClientSessionOptions (a new parameter to Client.Connect, for symmetry) For modelcontextprotocol#10 Co-Authored-By: 36760115+ln-12@users.noreply.github.com
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI I checked most recent updates on the branch - it looks fully functional now 👍
I'd like to include a small flow diagram as well, and I'm curious if you agree with its representation 🙂 ![]() cc: @jba |
You could say that distributed servers must not initiate SSE streams, but then they couldn't even send things like progress notifications and log messages. So you'd have to say that they don't send nested requests. That seems like a very strange restriction, and I'm not sure how you'd enforce it. All the discussion about resumability is irrelevant to this point. There may be separate issues there, but I think this one is enough of a concern that I'd like to wait for clarity in the specification before pursuing this. |
See also my comment here: #148 (comment) (Sorry that the conversation is split between that issue and this PR). |
Support stateless streamable sessions by adding a GetSessionID function to StreamableHTTPOptions. If GetSessionID returns "", the session is stateless, and no validation is performed. This is implemented by providing the session a trivial initialization state. To implement this, some parts of #232 (distributed sessions) are copied over, since they add an API for creating an already-initialized session. In total, the following new API is added: - StreamableHTTPOptions.GetSessionID - ServerSessionOptions (a new parameter to Server.Connect) - ServerSessionState - ClientSessionOptions (a new parameter to Client.Connect, for symmetry) For #10
SessionState holds the state of a ServerSession.
SessionStore allows arbitrary storage for SessionState.
ServerSession.InitSession provides a session ID, store and state to a ServerSession.
For #148