On Wed, Aug 10, 2016 at 10:14 AM, Fabian Pichler <[email protected]> wrote: > Hi Thomas > > Thanks for responding and thanks for supporting the suggested solution. > > I understand the doc.Key-issue that you mentioned. Yet, I think this is not > a > XWikiCacheStore#loadXWikDoc concern. In my understanding the expected > document > to load is defined by the document-reference and the language. Thus the > Hibernate-Store must be concerned in setting the context database according > to the Wiki-Reference before loading the document. This is acctually already > done in the XWiki#getDocument call. I agree that this might be the wrong > place to do it. > Thus, I would suggest to fix this issue in the XWikiHibernateStore on a > seperate jira-issue.
In an ideal world it would be fixed in XWikiHibernateStore I agree but this would break retro compatibility (yeah always a pain). That's why I talked about providing a new API. So right now in practice using a combination of context wiki and document reference is part of the API definition and can be applied at XWikiCacheStore too. Note that in practice this is actually a pretty rare case since anyone is supposed to use XWiki#getDocument which make sure the context contain the document wiki. But this can still happen since this is a very old and public API. > Maybe this could be combined with a significant performance improvment in > loading > the documents with fewer sql-queries per loading action. > > With XWIKI-13623 I try to tackle the issue about randomly loosing BaseObject > data because of an not thread safe construction and publication of > XWikiDocument > java objects in XWikiCacheStore. Because this issue gave us a big headache > in > production multiple times, I solved it in our project in the way mentioned. > > If the xwiki-dev team supports the suggested solution, I would be willing > to proceed > and prepare a pull request for an adapted XWikiCacheStore fix. > > Kind regards, > Fabian > > > 2016-08-08 12:07 GMT+02:00 Thomas Mortagne <[email protected]>: > >> XWikiCacheStore#loadXWikiDoc is actually a bit tricker than that right >> now for retro compatibility and laziness (really need to rewrite this >> whole store API) reasons and before applying what you propose the way >> it deals with document uid should be modified a bit. >> >> Hibernate store is always loading the document from the wiki indicated >> in the context completely ignoring the wiki in the XWikiDocument >> object. That means that doc.getKey() may be different than the key of >> the document that will actually come from the hibernate store. So >> before adding DocumentLoader jingling a first thing to do is generate >> the initial cache key with a mix of the wiki id located in the context >> and the XWikiDocument reference. >> >> Then what you propose sounds good. >> >> On Mon, Aug 8, 2016 at 8:53 AM, Fabian Pichler >> <[email protected]> wrote: >> > Hi devs >> > >> > http://jira.xwiki.org/browse/XWIKI-13623 >> > I would like to discuss the suggested solution, before I start >> implementing >> > it: >> > >> > *I suggest the following solution:* >> > Introducing a nested DocumentLoader class which loads the document >> calling >> > the basestore loadXWikiDoc in a synchronized getDocOrLoad method. The >> > DocumentLoader objects gets instantiated in the cacheStore on a cache >> miss. >> > It then gets stored in a ConcurrentHashMap using the same key which is >> used >> > for the document cache (DocumentReference including language). Using >> > putIfAbsent to insert the DocumentLoader and calling the synchronized >> load >> > method on the object in the map solves the memory visibility issue and >> > moreover ensures that the same XWikiDocument is not loaded multiple times >> > concurrently from the database. It is still possible to load different >> > documents in parallel. A DocumentLoader will load the XWikiDocument only >> > once and the returns the identical document object on any following call >> of >> > the synchronized getDocOrLoad method. >> > >> > WDYT? >> > >> > Kind Regards, >> > Fabian >> > _______________________________________________ >> > devs mailing list >> > [email protected] >> > http://lists.xwiki.org/mailman/listinfo/devs >> >> >> >> -- >> Thomas Mortagne >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs >> > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs -- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
