AArhin / memcached-session-manager

Automatically exported from code.google.com/p/memcached-session-manager
0 stars 0 forks source link

Tomcat7: check/implement support of async/comet requests #75

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Servlet spec 3 / tomcat 7 bring async request processing. It should be checked 
if there are changes needed to support this.

Original issue reported on code.google.com by martin.grotzke on 5 Jan 2011 at 10:27

GoogleCodeExporter commented 9 years ago
Hi Martin,
the least thing you need to do is to adapt your Valves:
RequestTrackingHostValve
RequestTrackingContextValve

they need to have a default constructor that calls super(true).
if you dont do that the default super constructor is called with false.
That boolean is there since tomcat 7.0, so it should be safe to move the valves 
to the tc6/7 jars and have a async supporting version in the tc7 package.

I hope you can work on this :)

Original comment by fabian.l...@codecentric.de on 28 Oct 2012 at 9:56

GoogleCodeExporter commented 9 years ago
Setting the async property is really easy. What I wonder is how request 
processing then is different, especially regarding the mentioned valves. They 
are used to track start/end of a request, e.g. to set/reset some thread local 
and to backup the session at the end of the request.

Are those valves still wrapping the request processing as in sync mode?

Original comment by martin.grotzke on 28 Oct 2012 at 9:12

GoogleCodeExporter commented 9 years ago
I created the branch issue_75_async_request_processing 
(https://github.com/magro/memcached-session-manager/tree/issue_75_async_request_
processing) in which the tc7 msm creates the valves with asyncSupported(true).
Tests are all green, which is a good start.

Though, I'm wondering what are the consequences of this change.

I attach jars so you can test directly if you want.

Original comment by martin.grotzke on 28 Oct 2012 at 9:34

Attachments:

GoogleCodeExporter commented 9 years ago
Sorry Martin,
Google did not notify me of your reply. I am going to test the snapshot builds.
As you are just providing access to the session, I think that an async request 
is not different than a normal request for you. You dont care if its a 
continued or suspended call.
As I do have an async application, I am willing to look for problems you have 
in mind. Feel free to contact me directly

Fabian

Original comment by fabian.l...@codecentric.de on 2 Nov 2012 at 12:56

GoogleCodeExporter commented 9 years ago
The Dummy Manager did not work :-( it needs to set async as well :)

Original comment by fabian.l...@codecentric.de on 2 Nov 2012 at 1:14

GoogleCodeExporter commented 9 years ago
I'll change the dummy manager, too.

To produce an error probably the following has to be done:
- configure msm to use nonsticky sessions
- start an async task, and from within the task write s.th. to the session

-> this should either fail or at least the added/modified session data should 
be lost

I'm assuming that valves are bound to the original request thread and don't 
wrap the whole async task.

Original comment by martin.grotzke on 2 Nov 2012 at 7:17

GoogleCodeExporter commented 9 years ago
Increasing priority, as there's another request on the mailing list.

Original comment by martin.grotzke on 12 Jul 2013 at 9:52

GoogleCodeExporter commented 9 years ago
Attached jars with changes for async based on current master.

Original comment by martin.grotzke on 24 Sep 2013 at 9:07

Attachments:

GoogleCodeExporter commented 9 years ago
First of all, thanks for your work, msm has helped me a great deal!

What's the current status on this? I'd need this very much, but only to read 
from the session during async call. How hazardous would you consider just 
patching the one commit on top of 1.6.5?

Br,
Santeri

Original comment by santeri....@gmail.com on 20 Nov 2013 at 3:32

GoogleCodeExporter commented 9 years ago
I just got reminded that I am still subscribed to this ticket. As Santeri, we 
have figured out that writing to Session in async is not a requirement for us. 
We only need to access (a more or less recent) state of the session for reading 
values (like authentication).

Original comment by fabian.l...@codecentric.de on 20 Nov 2013 at 3:35

GoogleCodeExporter commented 9 years ago
Can you test the attached 1.6.6-SNAPSHOT jars? If they're working I can merge 
this change and make a release.

Original comment by martin.grotzke on 20 Nov 2013 at 4:38

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Martin,

Thanks for the quick reply!

I tested the jars, and at least the async part works for me, and I didn't 
notice any other regressions either, although with this timeframe testing is of 
course rather superficial.

-Santeri

Original comment by santeri....@gmail.com on 20 Nov 2013 at 7:25

GoogleCodeExporter commented 9 years ago
Ok, I merged it into master, will make into the next release (coming soon).

Original comment by martin.grotzke on 26 Nov 2013 at 9:00

GoogleCodeExporter commented 9 years ago
Issue 184 has been merged into this issue.

Original comment by martin.grotzke on 2 Dec 2013 at 9:10

GoogleCodeExporter commented 9 years ago
Issue 188 has been merged into this issue.

Original comment by martin.grotzke on 20 Dec 2013 at 9:26

GoogleCodeExporter commented 9 years ago
Finally available with msm 1.7.0 I released today.

Original comment by martin.grotzke on 20 Dec 2013 at 10:27