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

Vaadin throws exception Unexpected message id from the client when Spring Session Serialization is used #4746

Open
ilya-ershov opened this issue Oct 30, 2018 · 11 comments

Comments

@ilya-ershov
Copy link

ilya-ershov commented Oct 30, 2018

When 'org.springframework.session:spring-session-jdbc' is used to support Session serialization with intent to use Vaadin with multiinstances (in a cloud for example), then subsequent request after the first one causes an Exception

Exception is thrown with message 'Unexpected message id from the client. Expected sync id: 0, got 1'
It happens here:

at com.vaadin.flow.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:303) ~[flow-server-1.2-SNAPSHOT.jar:1.2-SNAPSHOT]

The reason is that expected request id is stored inside UIInternals class and never gets stored/restored to/from session.
Each time when new UIInternals is created, it receives com.vaadin.flow.component.internal.UIInternals#lastProcessedClientToServerId == -1

int expectedId = ui.getInternals().getLastProcessedClientToServerId() + 1;

And each request after the first one gets refused with the exception

Even if I hack internals of ServerRpcHandler.handleRpc method, and do expectedId = requestId to surpass the validation, page just loads its initial state each button click. Not sure if it a sign of a next deeper bug or just impact of my dirty hack/workaround

Bug can't be reproduced with karibu test, it works as expected either it has 'org.springframework.session:spring-session-jdbc' dependency or not. But navigation to http://localhost:8080/ and manual test throws an exception.

I attach demo project with all setup done and karibu test that shows the scenario. It uses 12.0-SNAPSHOT version. Current released version 11.0.1 can't be used instead, because it has BeanStore not serializable issue

bug-demo.zip

@Artur-
Copy link
Member

Artur- commented Oct 31, 2018

Vaadin Framework and Vaadin Flow both rely on a request having exclusive (locked) access to the HTTP session. To support any Spring Session module, locking would have to be implemented in a different way than it is today (assumes sticky sessions so that locking can be done inside the JVM).

vaadin/framework#9189 (comment) has some more information

@elmot
Copy link
Contributor

elmot commented Oct 31, 2018

Probably we need to mention Spring-Session (and other similar frameworks) incompatibility somewhere in documentation.

@mvysny
Copy link
Member

mvysny commented Oct 31, 2018

The reason that Karibu-Testing library works as expected, is probably because Karibu-Testing acquires the Vaadin Lock automatically for all testing code. Therefore, Karibu-Testing may cause even the Spring Session code being executed with Vaadin lock held, which causes the setup to work. In a sense, this is caused by Karibu-Testing imperfect mock of the runtime environment.

@ilya-ershov
Copy link
Author

So I assume that if in method com.vaadin.flow.server.VaadinService#loadSession I can manually deserialize session from the distributed store it 'should' be working okay in clustered environment?
If so, is there a way to extend VaadinService with own implementation and make vaadin use it?
And is there an option to pass into com.vaadin.flow.server.VaadinService#loadSession request as parameter, because we need to get understand which exact session to restore?

@Legioth
Copy link
Member

Legioth commented Nov 2, 2018

So I assume that if in method com.vaadin.flow.server.VaadinService#loadSession I can manually deserialize session from the distributed store it 'should' be working okay in clustered environment?

That is not enough. You also need to ensure that there's only one concurrent request using one session at any given time. Otherwise you will have problems when serializing back the session to the distributed store, because you might accidentally overwrite changes made for a different request.

@ilya-ershov
Copy link
Author

So additional effort should be to make com.vaadin.flow.server.VaadinService#loadSession know about lock in a distributed manner? I mean, now it checks for lock as such:
assert VaadinSession.hasLock(this, wrappedSession);
But I should make this lock distributed across instances (Im thinking about IMDG-related solution)?

@Legioth
Copy link
Member

Legioth commented Nov 7, 2018

Furthermore, it would be necessary to change the logic so that the lock is acquired before the session is deserialized (this giving a nice chicken and egg problem). Otherwise, it cannot be guaranteed that requests are always operating against the most recently updated session contents.

Or as an alternative, use sticky sessions instead so that the default in-JVM locking works as expected. You can still serialize sessions to distributed storage after each request so that sessions can be deserialized on a new node when one node goes down, but you'd need some additional logic that ensures that a session is fully migrated to a new node before accepting any requests for that session.

@mimarko
Copy link

mimarko commented Mar 26, 2020

This topic is still open for a long time? Now-days a lot of apps are deployed on some cloud scalable environment (AWS, Azure, Google, Oracle, IBM Cloud, etc) and this feature regarding session replication, sticky session with replication with all major shared storage should be very simple for setup for Vaadin apps with Spring integration (not just with Spring).

At least Vaadin team should create best practice and complete examples with few major Cloud environment (AWS, Azure, Google) !

@Legioth
Copy link
Member

Legioth commented Mar 26, 2020

You are right that it would be useful to provide more guidance on how to deploy Vaadin applications in those environments. This is an area where we are currently considering a focused investment.

Depending on the complexity of the application and e.g. whether it uses setItems or setDataProvider (with something else than ListDataProvider) for data, the size of a session is typically somewhere between 100 kb and 10 mb. Pulling that amount of data through a session replication scheme for every single user interaction sounds like some quite heavy overhead that would only be justified in situations when the occasional lost session because of unexpected server shutdowns cannot be tolerated.

All other cases would be better served by simple sticky sessions in combination with reserved instances. If it's important to quickly evacuate excess capacity when scaling down after a peak, then it would be more appropriate to serialize and deserialize each session only for the purpose of migrating it to another node instead of doing full replication for every single request.

@nemanjakmno
Copy link

@Legioth "that would only be justified in situations when the occasional lost session because of unexpected server shutdowns cannot be tolerated." and "it would be more appropriate to serialize and deserialize each session only for the purpose of migrating it to another node instead of doing full replication for every single request."

Yes, this would be enough and please provide some example on how to do this. Very important topic and still nothing concrete from Vaadin team on this.

All your suggestions in this topic: https://vaadin.com/blog/session-replication-in-the-world-of-vaadin are very helpful but unfortunately do not resolve any of this in concrete problem.

@mshabarov
Copy link
Contributor

mshabarov commented Feb 14, 2025

The changes to how Vaadin handles the unique access is not gonna happen in the short/mid-term perspective, but we want to enhance our documentation to help users better understand the design, limitations and how to configure set-up with Sticky Sessions + Session Replication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

8 participants