I believe you can remove the sessionId check. It was just meant to prove my expectation that a passed sessionId is always equal to the current session's id (which I think it did).
The extra synchronization still makes sense as it avoids that two concurrent threads add a PageStore for the same pageMapName. This could cause strange (and hard to reproduce/debug) errors. good luck with your testing! richardwilko wrote: > > Ok, cool, > > Initial testing shows that no extra synchronisation is required. It works > :) > > However I had to modify the getHttpSession method, the test to make sure > the sessionid is the same, that doesn't work clustered as the session id > contains jvm route (ie machine identifier) so changes from machine to > machine. The first part of the id is the same (the part before the dot) > so we could either implement some text splitting or remove it completely. > > I also forgot about the other stuff that gets stored in httpsession, ie > the actual session object, so my simple 4 class config didnt work. I just > copied the original wicket one into mine, there are some extra classes > that are now distributed that dont need to be, but this was the quickest > way to get it working. IClusterable is used everywhere so will be pain if > we want to sort that out. > > After more testing i will post back a merged version of code, but all is > looking good, > > cheers, > > Richard > > > > Stefan Fußenegger wrote: >> >> As I said, it's a matter of taste and not worth a discussion. I think I >> can handle it, if you want to stick with your solution ;) >> >> With "complain" I meant: "All changes to clustered objects must happen >> within the context of a Terracotta transaction. This means that a thread >> must acquire a clustered lock prior to modifying the state of any >> clustered objects. If a thread attempts to modify a clustered object >> outside the context of a terracotta transaction, a runtime exception will >> be thrown." ( >> http://www.terracotta.org/confluence/display/docs1/Concept+and+Architecture+Guide#ConceptandArchitectureGuide-Transactions >> Terracotta Product Documentation ). PageMapStore is clustered after being >> stored in the session. Therefore, modifications to PageMapStore._pageMaps >> (as in getPageMap(String)) will cause an exception outside of a >> transaction. I can't see where the transaction starts, but testing this >> in a clustered environment will quickly give an answer whether we need >> further synchronization or not (maybe you already did this, while I only >> tested without actually using TC clustering). I added the further >> synchronization if needed >> >> btw: getPageStore(...) should probably be renamed as it in fact returns a >> PageMapStore, not a PageStore, same for getPageMap(...) that returns a >> PageStore. >> >> i think a nice side effect of this new implementation is that wicket >> components need not be IClusterable anymore. we always test our app in >> "pure wicket mode" and test it clustered prior to deployment. we always >> find some objects that are Serializable but not IClusterable, hence >> causing exceptions with TC. Using serialization of pages, both modes >> would need Serializable objects only. >> >> regards >> stefan >> >> http://www.nabble.com/file/p18335661/OurTerracottaPageStore.java >> OurTerracottaPageStore.java >> >> >> >> >> richardwilko wrote: >>> >>> Looks good, >>> >>> >>> Stefan Fußenegger wrote: >>>> >>>> - I wouldn't use an extra class just to wrap a HashMap of PageStore. I >>>> would just put them into the plain session. But finally, this is just a >>>> matter of taste. I even think that this class lacks proper >>>> synchronization. Doesn't Terracotta complain about modifying an >>>> instance outside of a transaction?? >>>> >>> >>> I disagree, I think it makes the code cleaner as all the stuff to do >>> with creating PageStores (and debug information) is encapsulated in the >>> class. I don't think that the synchronisation is an issue, im not sure >>> what you mean about terracotta complaining, if Ari is still watching >>> this thread then he can probably answer the question. >>> >>> It is a pain that you cant get the last element of the list, but your >>> solution works well, I tweaked it slightly to remove a not needed if >>> statement (you dont need to check the page id, the subset stuff takes >>> care of it). >>> >>> I'm gonna put this slightly modified class through our test environment >>> here where i work and throw loads of simulated users at it to see how it >>> works. >>> >>> I have removed the wicket module from my terracotta config as this >>> currently forces the use of httpsessionstore. Also with our solution we >>> only need to instrument 4 classes so i'm not using the IClusterable >>> thing in the config, i'm just declaring the classes manually. >>> I would be good to find out how we go about modifying the wicket tim >>> (probably another question for Ari there). I downloaded the terracotta >>> source but couldn't get it to build. >>> >>> Richard >>> >>> >>> >>> >>> >>> >> >> > > ----- ------- Stefan Fußenegger http://talk-on-tech.blogspot.com // looking for a nicer domain ;) -- View this message in context: http://www.nabble.com/Terracotta-integration-tp18168616p18337608.html Sent from the Wicket - Dev mailing list archive at Nabble.com.
