[Seaside] Confused about WASession cleanup [proposed fix]

[Sven Van Caekenberghe]

Julian, Lukas,

I extended my tests and kept on testing this morning and I think I finally found why the behavior that I was looking for (seeing sessions being cleaned up in some reliable, comprehensible way) did not manifest itself. I also think I have found a solution and I would propose that as a future out of the box default. This is what I did (full code attached or accessible at http://www.squeaksource.com/ADayAtTheBeach/ADayAtTheBeach-svc.6.mcz):

I added a simple WASessionCleanupComponent that is configured as follows in SessionCleanupTest#setUp:

| application |
application := WAAdmin register: WASessionCleanupComponent asApplicationAt: self class handlerName.
application preferenceAt: #sessionClass put: WASessionCleanupSession.
application cache expiryPolicy configuration at: #cacheTimeout put: self class cacheTimeout.
application cache reapingStrategy configuration at: #cacheReapInterval put: self class cacheReapInterval

I am using WASessionCleanupSession to better track instances and to print something on the Transcript when it is #unregistered.

As far as I understood the code in WARetrievalIntervalReapingStrategy, the reaping happens every 10 accesses.
That is why I expected these tests to succeed:

testSessionCleanupMany
	| startCount highCount delay |
	startCount := self numberOfSessions.
	self log: 'Start session count is ', startCount.
	100 timesRepeat: [ self log1: $.. self hitWebApp ].
	self assert: (self numberOfSessions > startCount).
	highCount := self numberOfSessions.
	self log: 'High session count is ', highCount.
	delay := self class cacheTimeout + 10.
	self log: 'Waiting for ', delay, ' seconds'.
	(Delay forSeconds: delay) wait.
	"By now the previous 100 sessions are older than cacheTimeout, 
	create cacheReapInterval more sessions to activate the reaper"
	(self class cacheReapInterval + 2) timesRepeat: [ self log1: $.. self hitWebApp ].
	3 timesRepeat: [ Smalltalk garbageCollect ].
	self log: 'Current session count is ', self numberOfSessions.
	self assert: (self numberOfSessions < highCount)

or

testSessionCleanupContinuous
	| startCount highCount delay |
	startCount := self numberOfSessions.
	self log: 'Start session count is ', startCount.
	(self class cacheReapInterval + 2) timesRepeat: [ 
		self hitWebApp.
		self log: 'Session count is ', self numberOfSessions, ' waiting 5 seconds'.  
		(Delay forSeconds: 5) wait ].
	self assert: (self numberOfSessions > startCount).
	highCount := self numberOfSessions.
	self log: 'High session count is ', highCount.
	"By now the some of the previous sessions are older than cacheTimeout,
	because (cacheReapInterval + 2) * 5 seconds > our cacheTimeout,
	create more sessions to activate the reaper"
	(self class cacheReapInterval + 2) timesRepeat: [ 
		self hitWebApp.
		self log: 'Session count is ', self numberOfSessions, ' waiting 5 seconds with GC'.  
		(Delay forSeconds: 5) wait.
		Smalltalk garbageCollect ].
	self log: 'Current session count is ', self numberOfSessions.
	self assert: (self numberOfSessions < highCount)

They kept on failing however, although now and then some sessions did get reaped somehow, but not in a reliable comprehensible way.

FIX

The reason this didn't work was both in the nature of the test as well as in the implementation of WARetrievalIntervalReapingStrategy. Like the name says (and as its implementation of #retrieved:key shows), it only counts _retrievals_ and not the other cache operations. So I created a subclass, WAAccessIntervalReapingStrategy, with these methods:

stored: anObject key: aString
	super retrieved: anObject key: aString

removed: anObject key: aString
	super retrieved: anObject key: aString

Doing

	application cache setReapingStrategy: WAAccessIntervalReapingStrategy new

in the setUp now produces the behavior that I expected. 

(Although the tests do not always succeed, doing a GC and expecting instances to reliably disappear does not alwys work, but they do get reaped as the transcript log shows. The test is too dependent on elements that it cannot control.). 

The test accesses (like most benchmarks) create new sessions for each request and never access the sessions afterwards. Therefore, the reaping never took place with the default WARetrievalIntervalReapingStrategy !

Especially for internet accessible servers that get hit by robots, this behavior seems necessary. 

Don't you think that counting retrieved/stored/removed by default would be better ?

Sven

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ADayAtTheBeach.st
Type: application/octet-stream
Size: 20062 bytes
Desc: not available
Url : http://lists.squeakfoundation.org/pipermail/seaside/attachments/20100610/2a6df43a/ADayAtTheBeach-0001.obj
-------------- next part --------------


--
Sven Van Caekenberghe - http://homepage.mac.com/svc
Beta Nine - software engineering - http://www.beta9.be