Hi,
me and Justin had a conversation on an issue with the in memory
catalog modification mechanism that we believe it's a good
idea to share with everybody else.
The topic is, how do we make sure a single thread
does not trump on its own feet trying to interact with the
catalog?
For those who don't know, the in memory catalog (almost)
never returns raw xxxInfo objects, but wraps them with a
ModificationProxy instead.
The proxy serves the purpose of allowing the code to perform
changes the xxxInfo objects, and avoid them being visible
to the other threads up until a Catalog.save/Catalog.update
call is performed. Basically they buffer whatever property
change is made in a local hashmap, and actually change the
bean contents only when save/update are called.
Without them the modifications would be against the beans
that everybody else sees.
-------------------------------------------------------------
aaime: getCatalog().getLayer() -> modificationProxy
aaime: getCatalog().getLayer().getResource() -> !modificationProxy
jdeolive: i know where you are going
jdeolive: and that is a bug currently what is on trunk
iwillig è ora conosciuto come iwillig|lunch
aaime: I just noticed I cannot save any existing layer configuration change
aaime: because of the above
jdeolive: i meant to actually bring this up to you but got side tracked
jdeolive: yup
jdeolive: we have a couple of options to fix
jdeolive: would like your feedback
***aaime had a couple of ideas too
jdeolive: who wants to go first?
aaime: (let's see if they match)
aaime: you
jdeolive: well my first idea is that if we add a root interface for all catalog interfaces then we can do a check in ModifcationProxy for it being returned
jdeolive: and if so, wrap it in a proxy
vheurteaux ha abbandonato la stanza (quit: ).
aaime: good, we can add the "id" concept there as well
jdeolive: yup
aaime: second option?
jdeolive: a pretty bad idea to wrap anything that is not immiutable in a proxy
jdeolive: but there is no really good way to tell if something is immutable
aaime: I prefer the first, but I still have an open question
jdeolive: the best i could come up with
jdeolive: is to check for any setter methods...
jdeolive: shoot
aaime: where is the wrapping for resource going to happen? In LayerINfoImpl?
aaime: or is it the job of whatever builds the catalog to set a modification proxied one in the layer?
jdeolive: i was thinking it would go right in ModifcationProxy
aaime: ah right
aaime: ok, got it
jdeolive: and leave the *INfoImpl classes ignorant to the fact they are being proxied
jdeolive: cool
aaime: whenevr the modification proxy needs to return something, it'll check if it's an uwrapped catalog object
aaime: question... won't we generate many modification proxies, one for each time getResource is called?
jdeolive: good point, it should do a check that what it is proxying is not already proxied
jdeolive: yes it would
aaime: ouch, what about cod
aaime: code
aaime: that calls getResource once and changes it
aaime: then does it again
aaime: and changes it again
aaime: then saves it
aaime: only the second modification would stick?
jdeolive: yeah, that is a problem
aaime: I mean, it's not uncommon in java not to store the return value in a local variable
jdeolive: not sure i have a great solutation for that one...
***jdeolive notes again his horrible spelling
aaime: well, the modification proxy could have a local cache of the returned proxies
jdeolive: any ideas on that? the problem that is, not my spelling 
aaime: but I guess that moves the problem up to the catalog
aaime: if I call getLayer(xxx) multiple times, I'll be in the same situation, no?
jdeolive: yeah
aaime: so I guess the catalog should keep some cache as well?
jdeolive: it is a nasty one
aaime: in hibernate based apps there is no wrapping because you're playing with a view of the database
jdeolive: we could introduce a cache... but
aaime: the rest of the code does not see it
jdeolive: you beat me to my next question
jdeolive: how do things change with hibernate?
aaime: and hibernate has his own per session cache
aaime: well, in hibernate the beans are pure views
aaime: changing them won't change the database state
aaime: but you have a session concept
jdeolive: i guess hibernate handles this transparently?
aaime: if you ask the same resource 10 times in a row from the session, you'll get back the same object if my memory serves me right
SandGorgon [n=user@anonymised.com] è entrato nella stanza.
aaime: whilst if you close it, not, it will be different
aaime: the session is controlled usually by a servlet filter
jdeolive: ok.. so that is an argument for adding a cache / being a bit smarter about how we proxy
aaime: so it lasts as long as it takes to serve a request
aaime: so yeah, the cache should be per thread and live only as long as the current request is alive I guess
jdeolive: makes sense
aaime: that would pretty much mimick the "open session in view" approach most hibernate users are following
jdeolive: so basically
jdeolive: ModificationProxy could hold on to some thread local cache of object to its proxy
jdeolive: clearing it when an object is saved
aaime: hmmm... I would really do it linked to a servlet filter
cabraham [n=cabraham@anonymised.com] è entrato nella stanza.
aaime: and have the cache cleared by the filter when the current request is over, saved or not
aaime: the thread will be reused, you cannot afford to keep the thread local lingering around
cabraham ha abbandonato la stanza (quit: Client Quit).
aaime: it could be a dispatcher callback also, so that it can be easily plugged off if we roll in another persistence mechanism
***jdeolive is thinking
aaime: restconfig could enjoy that as well
aaime: and WPS might expose catalog config related processes as well
dwins_ [n=dwins@anonymised.com] è entrato nella stanza.
aaime: think buffer and store in the global config kind of interaction
jdeolive: what about things not executing as part of a servlet request?
aaime: what would that be?
jdeolive: ie a test case?
jdeolive: that is not a mock test
aaime: how would that be an issue?
jdeolive: maybe i am not quite understanding
jdeolive: lets back up for a moment
aaime: consider that I don't fully understand the current catalog either so we're in the same boat 
jdeolive: fair enough
jdeolive: so
jdeolive: do we agree that adding the idea of a thread local proxy cache is needed?
aaime: sort of
aaime: that is, if spawning new thread inhertis the same cache, yes
***aaime is thinking about groldan job control thing
aaime: but apart from that issue, yeah, thread local cache sounds good
cabraham [n=cabraham@anonymised.com] è entrato nella stanza.
jdeolive: ok... but does that not mean that we have to start worrying about making catalog objects thread safe?
dwins ha abbandonato la stanza (quit: No route to host).
aaime: hmmm... good question, I don't have a good answer
aaime: afaik the modification proxies should isolate the various threads no?
dwins_ è ora conosciuto come dwins
aaime: There are two diffrent cases I guess
aaime: two requests made by separate users, different threads, we want to have them transactional
aaime: each request has its own view of the catalog
aaime: which the modification proxies provide to some extent
Bojan ha abbandonato la stanza (quit: Remote closed the connection).
jdeolive: right
aaime: the different case is one request, but two or more threads carrying it on
cabraham ha abbandonato la stanza (quit: Client Quit).
aaime: which could conceivably happen with WPS
jdeolive: which comes up when we start thinking about job quueing
aaime: yeah, well, as long as the jobs are service oriented no issue
aaime: they don't change the catalog
aaime: but issues start popping up when they do
aaime: which is something only WPS could be interested in doing so far?
jdeolive ha abbandonato la stanza (quit: ).
aaime: and even in that case, each process in a chain would execute in isolation
jdeolive [n=jdeolive@anonymised.com] è entrato nella stanza.
aaime: so no, I don't see it becoming a problem, was thinking too much
jdeolive: sorry, got disconnected
aaime: to sum up, I believe we're safe with a thread local cache
jdeolive: ok, i think that makes sense
aaime: http://pastebin.com/m4c64c8
sigq: Title: pastebin - collaborative debugging tool (at pastebin.com)
aaime: (pasted what you might have lost)
aaime: jdeolive, the issue with thread local only
jdeolive: thanks
aaime: is that threads are reused by the container
aaime: so you build your cache
aaime: the request completes
jdeolive: right, so a servlet filter that clears the cache is needed
aaime: yeah, or a dispatcher callback of some kind
jdeolive: i think a servlet filter makes more sense... for instance with rest which does not use the dispatcher
jdeolive: or were you talking spring dispatcher? not ows dispatcher?
aaime: I don't know
aaime: what I had in mind is
aaime: the day we plug another kind of catalog, we might not need modification proxies around
aaime: think a hibernate based one
aaime: so I guess whatever we do should be something that we can turn off with a configuration?
jdeolive: right, and that has its own servlet filter for doing the same thing
jdeolive: agreed
oterral123 ha abbandonato la stanza.
aaime: ok, so to sum up, to solve the issue cleanly we need
aaime: - a new catalog object interface
aaime: - a per thread ModificationProxy cache
aaime: - a mechanism to clean it up when the current request ends
aaime: correct?
jdeolive: yup, makes sense
aaime: you open the jira? (and work on it?)
jdeolive: will do
jdeolive: we should probably also send this convo to the mailing list
aaime: I can grab the logs and post them to IRC?
jdeolive: cool
-------------------------------------------------------------
Made it here? Wow, you're persistent!
Anyways, here is the jira that tracks this change:
http://jira.codehaus.org/browse/GEOS-2763
Cheers
Andrea
--
Andrea Aime
OpenGeo - http://opengeo.org
Expert service straight from the developers.