G'Day Niels, sorry it has taken me until the new year to return to this conversation.
We discussed this pull request in the last geoserver meeting: * Andrea is creating a draft pull request review guide <https://github.com/geoserver/geoserver/wiki/Pull-request-review-guide> page to try and prevent us tripping up this way in the future. * And I have created a proposal GSIP-136 <https://github.com/geoserver/geoserver/wiki/GSIP-136> for the api change you outline in your pull request. I also had a chance to talk to Gabriel about an event notification strategy. He echoed your words that doing this consistently was an known unsolved problem - so I have creating a technical debt page <https://github.com/geoserver/geoserver/wiki/Consistent-Event-Notification> with some feedback from Kevin on GWC. -- Jody Garnett On 23 December 2015 at 05:12, Niels Charlier <ni...@scitus.be> wrote: > On 22-12-15 23:00, Jody Garnett wrote: > >> >> Think you are missing the idea - in most cases these resources are not >> unpacked on disk and are retrieved by JDBCResourceStore. I do not know what >> that will do to performance - but that is why JDBCResourceStore is a >> community module. If we need to cache the bytes for small resources then >> those nodes would indeed need to listen for events to keep their cache in >> sync. >> >> So what does have to listen for events? Only a few things (templates, >> security files, ...) if you would like to search the code we can have a >> better conversation. >> > I think we need to be more on the same page on that which we want to > accomplish. What Gabriel and I wanted to accomplish in this project, is to > make Resource.addListener() work for jdbcstore as expected, under all > circumstances. Not to keep anything particular in sync: that is the job of > the module developers. > The developers must be sure that if they call Resource.addListener, they > will be notified of any changes in the resource: code that works for the > default store MUST also work for the jdbc store, that is the aim. When they > use the file system store, they know they will be notified if someone edits > the file on disk. When using the jdbcstore, they must also be notified of > any change. We decided to assume nobody will edit the files in the db > directly, so that we only have to deal with the issue of multiple instances. > > I had started with a single place to register for events, and refactored >> as you see based on making the code examples easier to use. But I guess you >> are correct - this approach is my own recommendation - as an implementor >> you can choose to implement heavyweight resource objects (but we do not >> have any dispose() methods since that was not the plan). >> > Heavyweight resource objects would be nearly impossible with the existing > API, but my point was only that the single place to register events is > encapsulated by the ResourceStore API and thus inaccessible to other > modules. The clustering modules do not have access to it, if limited to > using the ResourceStore API. So this fact remains irrelevant as far as the > implementation of these modules is concerned. > > I do not see how that matters, you are defining new API which could be >> injected into the ResourceStore implementation if needed. >> > > Indeed, and I was explaining why because you questioned the fact I am > doing that :) > > I am trying to avoid duplicating event notification across a cluster three >> times. >> > That is absolutely impossible in my implementation. And in fact, that > would only have been a risk if we went for a listening-and-redispatching > strategy. That is one reason I called it error-prone. > > > Regards > Niels >
------------------------------------------------------------------------------
_______________________________________________ Geoserver-devel mailing list Geoserver-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/geoserver-devel
