Ok, I have adapted your code in the following ways:
1) 2) There is a configurable limit to the number of pages per page map and page maps are stored separately, this is to combat the problem I found in point 2. 3) I have removed the pagemap from PageKey class. 4) Adapted getPage to fit the api doc. 5) Moved serialization / de-serialization higher up so that dont need to store transient TerracottaPageStore What do you think? I'm add some debug output code and test it in a clustered environment and will report back. http://www.nabble.com/file/p18314496/OurTerracottaPageStore.java OurTerracottaPageStore.java im still not entirly sure about the containsPage method either, it might require iteration through the map because I'm not sure the DEFAULT_AJAX_VERSION_NUMBER approach will work. Richard richardwilko wrote: > > Hi Stefan, > > Looking through your code I see a couple of issues: > > 1) There is no limit on the number of pages stored in the pagemap, pages > could get added forever. I feel there needs to be a way to limit the > number of pages stored, with oldest ones discarded first. This is how > DiskPageStore works. > > 2) Following on from point 1, a HashMap does not keep insertion order so > it is not possible to remove the oldest ones easily. A simple change to > LinkedHashMap would solve this and make point 1 easy to implement. > However storing all the pagemaps together does mean that the most recent > pages from one pagemap could get removed due to high use of another > pagemap. In this case when the user goes back to the other pagemap he/she > will encounter an exception. > > 3) Your getPage code is not general enough; from the javadocs for getPage > in IPageStore: > * If ajaxVersionNumber is -1 and versionNumber is specified, the page > store must return the page with highest ajax version. > * If both versionNumber and ajaxVersioNumber are -1, the pagestore must > return last touched (saved) page version with given id. > Your method of constructing a key object wouldn't work in these > situations, as it would only find exact matches, and so getPage would > require iterating through the entire HashMap and looking at every entry. > > This issue is the reason why I went for the nested structure I used. I do > agree that a single storage map would ideally be better, especially as > this make it easier to better manage the number of pages stored, but i'm > not sure if it is the most efficient method of storage for the complex > getPage requirements. By efficient i mean execution time rather than > memory usage. > > Thoughts? > > Richard > > > Stefan Fußenegger wrote: >> >> Hi Richard, >> >> I had a thorough look on your code. I have the following remarks: >> >> - yes, SerializedPage must be clustered and should therefore implement >> IClusterable (it is already Serializable, it should therefore be okay to >> change) >> - I found two problems with your implementation: >> 1) unbind() is called during invalidation of a session. getPageStore() >> will therefore result in a NPE as there is no WebRequest >> 2) according to the JavaDoc of DiskPageStore#removePage(SessionEntry, >> String, int) ("page id to remove or -1 if the whole pagemap should be >> removed") calling removePage(String, String, int) with an id of -1 should >> delete all pages of a pageMap (however, that's not documented in the >> JavaDoc of IPageStore!) >> - I feel that all pages could be in a single HashMap (rather than using 3 >> levels of nested HashMaps and HashSets). I therefore implemented my own >> PageStore based on your ideas to confirm my feelings (using a single >> HashMap per sesison, using less Hash(Map|Set) iterations; access >> synchronized using a ReentrantReadWriteLock which I think has quite good >> performance with TC). Please have a look. We can probably >> http://www.nabble.com/file/p18312624/MyTerracottaPageStore.java >> MyTerracottaPageStore.java merge our ideas for best results! >> >> Regards >> Stefan >> >> >> http://www.nabble.com/file/p18312624/MyTerracottaPageStore.java >> MyTerracottaPageStore.java >> >> > > -- View this message in context: http://www.nabble.com/Terracotta-integration-tp18168616p18314496.html Sent from the Wicket - Dev mailing list archive at Nabble.com.
