Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Thu, 2008-08-28 at 07:44 -0400, Vadim Gritsenko wrote: > On Aug 26, 2008, at 3:59 AM, Thorsten Scherler wrote: > > > On Mon, 2008-08-25 at 20:23 -0400, Vadim Gritsenko wrote: > >> On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: > > ... > >> Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is > >> actually unique in any environment. And since it is unique, it does > >> not make sense to lock on it at all - lock on unique object serves no > >> purpose :-) > > > > Makes me wonder since HttpServletRequest is always unique as well > > due to > > the headers, so does not violate as well the whole locking? > > HttpServletRequest is unique only in context of external request; but > it is same for multiple sub-requests of single external request. > Similarly, CommandLineRequest would be unique per single external > request, but shared across sub-requests. > > At the same time, ObjectModelHelper.REQUEST_OBJECT will be always > unique: it will either be HttpServletRequest/CommandLineRequest for > top level, or Wrapper for nested requests. > > > > >> You could, for example, have same effect with code: > >> > >> // Avoids NPE, does nothing > >> if (lock == null) lock = new Object(); > >> > >> > >> To get back to the problem... The original goal of pipeline locking > >> was to prevent separate external requests to start generating cached > >> response for the same resource-intensive resource. In other words, if > >> too external requests both (directly or through aggregation) hit '/ > >> slow/cached/foo' resource, only one request will trigger this > >> pipeline, while another will wait for the first to complete. > >> > >> This logic, in turn, had to be augmented to exclude single external > >> request hitting the same slow resource twice due to aggregation (and > >> parallel aggregation), hence locking against top level external > >> request was implemented. > >> > > > > Understand but I am confused due to the above fact that the > > HttpServletRequest is as well unique as it is my understanding. > > > >> Now, in situation when CommandLineEnvironment is used from the, ahem, > >> command line, two external requests will not happen. Which means that > >> whole pipeline locking thing is irrelevant and can be omitted > >> completely (like in 'lock = new Object()' snippet above). But, the > >> same CommandLineEnvironment stuff can be used in multi threaded > >> environment which uses the CocoonBean class. So, strictly speaking, > >> in > >> this scenario locking still should be performed against external > >> request object, and not against any RequestWrapper which is unique > >> for > >> each sub-request. > > > > I now are using the url as lock key since as I understand it perfectly > > qualifies as lock key. > > Why not CommandLineRequest? Hmm, because it would be the same as ObjectModelHelper.REQUEST_OBJECT this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, new CommandLineRequest(this, null, uri, null, attributes, parameters, headers)); However if you recommend to do: CommandLineRequest request = new CommandLineRequest(this, null, uri, null, attributes, parameters, headers); this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, request); this.objectModel.put(CLI_REQUEST, request); That is as well fine with me. wdyt? Thanks for your feedback. salu2 > > > > The patch is attached to COCOON-2241. As soon you are ok with it I > > will > > commit it. > > I'd rather use CommandLineRequest; or at least change one line to be: > >this.objectModel.put(CLI_REQUEST_ID, new String(uri)); > > > > Vadim > > > > Thanks for your patience. > > > > salu2 > > > >> > >> Hope now it all makes more sense. > >> > >> Regards, > >> Vadim > >> > >> > >>> In comparison the HttpEnvironment has in the constructor: > >>> this.objectModel.put(HTTP_REQUEST_OBJECT, req); > >>> where HttpServletRequest req. > >>> > >>> The uniqueness in both cases are, as I understand, the headers. > >>> > >>> salu2 > >> > > -- > > Thorsten Scherler > > thorsten.at.apache.org > > Open Source Java consulting, training and > > solutions > > > -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 26, 2008, at 3:59 AM, Thorsten Scherler wrote: On Mon, 2008-08-25 at 20:23 -0400, Vadim Gritsenko wrote: On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: ... Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is actually unique in any environment. And since it is unique, it does not make sense to lock on it at all - lock on unique object serves no purpose :-) Makes me wonder since HttpServletRequest is always unique as well due to the headers, so does not violate as well the whole locking? HttpServletRequest is unique only in context of external request; but it is same for multiple sub-requests of single external request. Similarly, CommandLineRequest would be unique per single external request, but shared across sub-requests. At the same time, ObjectModelHelper.REQUEST_OBJECT will be always unique: it will either be HttpServletRequest/CommandLineRequest for top level, or Wrapper for nested requests. You could, for example, have same effect with code: // Avoids NPE, does nothing if (lock == null) lock = new Object(); To get back to the problem... The original goal of pipeline locking was to prevent separate external requests to start generating cached response for the same resource-intensive resource. In other words, if too external requests both (directly or through aggregation) hit '/ slow/cached/foo' resource, only one request will trigger this pipeline, while another will wait for the first to complete. This logic, in turn, had to be augmented to exclude single external request hitting the same slow resource twice due to aggregation (and parallel aggregation), hence locking against top level external request was implemented. Understand but I am confused due to the above fact that the HttpServletRequest is as well unique as it is my understanding. Now, in situation when CommandLineEnvironment is used from the, ahem, command line, two external requests will not happen. Which means that whole pipeline locking thing is irrelevant and can be omitted completely (like in 'lock = new Object()' snippet above). But, the same CommandLineEnvironment stuff can be used in multi threaded environment which uses the CocoonBean class. So, strictly speaking, in this scenario locking still should be performed against external request object, and not against any RequestWrapper which is unique for each sub-request. I now are using the url as lock key since as I understand it perfectly qualifies as lock key. Why not CommandLineRequest? The patch is attached to COCOON-2241. As soon you are ok with it I will commit it. I'd rather use CommandLineRequest; or at least change one line to be: this.objectModel.put(CLI_REQUEST_ID, new String(uri)); Vadim Thanks for your patience. salu2 Hope now it all makes more sense. Regards, Vadim In comparison the HttpEnvironment has in the constructor: this.objectModel.put(HTTP_REQUEST_OBJECT, req); where HttpServletRequest req. The uniqueness in both cases are, as I understand, the headers. salu2 -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Mon, 2008-08-25 at 20:23 -0400, Vadim Gritsenko wrote: > On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: ... > Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is > actually unique in any environment. And since it is unique, it does > not make sense to lock on it at all - lock on unique object serves no > purpose :-) Makes me wonder since HttpServletRequest is always unique as well due to the headers, so does not violate as well the whole locking? > You could, for example, have same effect with code: > > // Avoids NPE, does nothing > if (lock == null) lock = new Object(); > > > To get back to the problem... The original goal of pipeline locking > was to prevent separate external requests to start generating cached > response for the same resource-intensive resource. In other words, if > too external requests both (directly or through aggregation) hit '/ > slow/cached/foo' resource, only one request will trigger this > pipeline, while another will wait for the first to complete. > > This logic, in turn, had to be augmented to exclude single external > request hitting the same slow resource twice due to aggregation (and > parallel aggregation), hence locking against top level external > request was implemented. > Understand but I am confused due to the above fact that the HttpServletRequest is as well unique as it is my understanding. > Now, in situation when CommandLineEnvironment is used from the, ahem, > command line, two external requests will not happen. Which means that > whole pipeline locking thing is irrelevant and can be omitted > completely (like in 'lock = new Object()' snippet above). But, the > same CommandLineEnvironment stuff can be used in multi threaded > environment which uses the CocoonBean class. So, strictly speaking, in > this scenario locking still should be performed against external > request object, and not against any RequestWrapper which is unique for > each sub-request. I now are using the url as lock key since as I understand it perfectly qualifies as lock key. The patch is attached to COCOON-2241. As soon you are ok with it I will commit it. Thanks for your patience. salu2 > > Hope now it all makes more sense. > > Regards, > Vadim > > > > In comparison the HttpEnvironment has in the constructor: > > this.objectModel.put(HTTP_REQUEST_OBJECT, req); > > where HttpServletRequest req. > > > > The uniqueness in both cases are, as I understand, the headers. > > > > salu2 > -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 25, 2008, at 6:40 AM, Thorsten Scherler wrote: On Fri, 2008-08-22 at 10:34 -0400, Vadim Gritsenko wrote: On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote: So which object you would suggest to lock? You wrote "Suitable alternative would be to lock against top most command line request." To what you are referring with "top most command line request"? That would be CommandLineRequest. Hmm, did you see the above objectModel? There is no such an object in the map. There are exactly six objects in the model 1) response 2) link-collection 3) request 4) context 5) source-resolver 6) org.apache.cocoon.components.CocoonComponentManager Yes, it currently does not exist. Following HTTP environment analogy, that would be CommandLineEnvironment.CLI_REQUEST_OBJECT. Which does not exist, right? Correct. At the moment, it does not exist. I opened an issue COCOON-2241 around the issue. The proposed solution is now taken the ObjectModelHelper.REQUEST_OBJECT if the request is from CLI, if not it is using the HttpEnvironment.HTTP_REQUEST_OBJECT. IMO it should not be a problem with deadlocks since the FileSavingEnvironment and LinkSamplingEnvironment (as only implementations for the Cli) have in the constructor: this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, new CommandLineRequest(this, null, uri, null, attributes, parameters, headers)); which IMO makes it unique for each sub-request. Yes, ObjectModelHelper.REQUEST_OBJECT object is always unique. It is actually unique in any environment. And since it is unique, it does not make sense to lock on it at all - lock on unique object serves no purpose :-) You could, for example, have same effect with code: // Avoids NPE, does nothing if (lock == null) lock = new Object(); To get back to the problem... The original goal of pipeline locking was to prevent separate external requests to start generating cached response for the same resource-intensive resource. In other words, if too external requests both (directly or through aggregation) hit '/ slow/cached/foo' resource, only one request will trigger this pipeline, while another will wait for the first to complete. This logic, in turn, had to be augmented to exclude single external request hitting the same slow resource twice due to aggregation (and parallel aggregation), hence locking against top level external request was implemented. Now, in situation when CommandLineEnvironment is used from the, ahem, command line, two external requests will not happen. Which means that whole pipeline locking thing is irrelevant and can be omitted completely (like in 'lock = new Object()' snippet above). But, the same CommandLineEnvironment stuff can be used in multi threaded environment which uses the CocoonBean class. So, strictly speaking, in this scenario locking still should be performed against external request object, and not against any RequestWrapper which is unique for each sub-request. Hope now it all makes more sense. Regards, Vadim In comparison the HttpEnvironment has in the constructor: this.objectModel.put(HTTP_REQUEST_OBJECT, req); where HttpServletRequest req. The uniqueness in both cases are, as I understand, the headers. salu2
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 22, 2008, at 11:26 AM, Rainer Pruy wrote: Hi, sorry for intruding this discussion. Not intruding at all :) just from what I can take from this discussion: Wouldn't it be easier to enclose this into a generalized Environment implementation that does support Environment.getTopRequest() and is stored with ObjectModel in place of those different "REQUEST_OBJECT" instances - all different? That way you could also have some getCurrentRequest() or getNthRequest(int n) or any other operation that has to deal with request environments and knowledge about can be kept more local. It is a nice suggestion but it has to be weighted in carefully with regards to all Environment implementations in 2.1 and different architecture in 2.2. Given my limited time, I'm not going to venture into that. Goal of my comments was mainly to point out to the problem, and not to suggest any particular solution. But you are certainly welcome to work out the best way on how to handle this problem :) Vadim
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Fri, 2008-08-22 at 10:34 -0400, Vadim Gritsenko wrote: > On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote: > > > On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote: > >> On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote: > >> > >>> How about: > >>>} else { > >>> -Object lock = > >>> env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); > >>> +Object lock = > >>> env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); > >>>try { > >>>transientStore.store(lockKey, lock); > >>>} catch (IOException e) { > >>> > >>> This way we always lock the same object. > >> > >> Exactly this thing I was trying to explain - it will not work because > >> instead of using top level request you are suggesting to use current > >> sub-request. > > > > Sorry I am not really following. As understand you we cannot use > > ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a > > sub-request (a request for the exact same resource in a concurrent > > situation). > > Exactly. > > > > If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we > > are in CLI mode. > > (patch quoted above does not have lock == null comparison anymore, > that's why it is dangerous for HTTP env) > > > > In CLI ASAIK there is no concurrent situation meaning > > the problem cannot occur. However since the issue is talking about > > includes that may occur. > > Yes. You can still use IncludeTransformer in CLI. So deadlock can > happen. > > > > Having a typical call from the cli gives for the env.getObjectModel(): > > > > HashMap values: > > [null, > > [EMAIL PROTECTED], > > link-collection=[], > > [EMAIL PROTECTED], > > null, null, null, null, > > context > > = > > org > > [EMAIL PROTECTED], > > null, null, source- > > resolver > > = > > org > > [EMAIL PROTECTED], > > org > > .apache > > .cocoon > > .components > > .CocoonComponentManager > > [EMAIL PROTECTED], null, > > null, null] > > > > So which object you would suggest to lock? > > > > You wrote "Suitable alternative would be to lock against top most > > command line request." > > > > To what you are referring with "top most command line request"? > > That would be CommandLineRequest. Hmm, did you see the above objectModel? There is no such an object in the map. There are exactly six objects in the model 1) response 2) link-collection 3) request 4) context 5) source-resolver 6) org.apache.cocoon.components.CocoonComponentManager > Following HTTP environment analogy, > that would be CommandLineEnvironment.CLI_REQUEST_OBJECT. Which does not exist, right? I opened an issue COCOON-2241 around the issue. The proposed solution is now taken the ObjectModelHelper.REQUEST_OBJECT if the request is from CLI, if not it is using the HttpEnvironment.HTTP_REQUEST_OBJECT. IMO it should not be a problem with deadlocks since the FileSavingEnvironment and LinkSamplingEnvironment (as only implementations for the Cli) have in the constructor: this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, new CommandLineRequest(this, null, uri, null, attributes, parameters, headers)); which IMO makes it unique for each sub-request. In comparison the HttpEnvironment has in the constructor: this.objectModel.put(HTTP_REQUEST_OBJECT, req); where HttpServletRequest req. The uniqueness in both cases are, as I understand, the headers. salu2 > > > Vadim > > > > salu2 > > > >> > >> Take a look at EnvironmentWrapper (which is used e.g. in > >> SitemapSource): > >> > >> // create new object model and replace the request object > >> Map oldObjectModel = env.getObjectModel(); > >> if (oldObjectModel instanceof HashMap) { > >> this.objectModel = (Map) > >> ((HashMap)oldObjectModel).clone(); > >> } else { > >> this.objectModel = new HashMap(oldObjectModel.size()*2); > >> Iterator entries = oldObjectModel.entrySet().iterator(); > >> Map.Entry entry; > >> while (entries.hasNext()) { > >> entry = (Map.Entry)entries.next(); > >> this.objectModel.put(entry.getKey(), > >> entry.getValue()); > >> } > >> } > >> this.request = new > >> RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel), > >> requestURI, > >> queryString, > >> this, > >> rawMode); > >> this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, > >> this.request); > >> > >> > >> Vadim > > -- > > Thorsten Scherler > > thorsten.at.apache.org > > Open Source Java consulting, training and > > solutions > > > -- Thorsten Scherler thorsten.at.apache.o
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
Hi, sorry for intruding this discussion. just from what I can take from this discussion: Wouldn't it be easier to enclose this into a generalized Environment implementation that does support Environment.getTopRequest() and is stored with ObjectModel in place of those different "REQUEST_OBJECT" instances - all different? That way you could also have some getCurrentRequest() or getNthRequest(int n) or any other operation that has to deal with request environments and knowledge about can be kept more local. Just my 0,02 EUR Rainer Vadim Gritsenko schrieb: > On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote: > >> On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote: >>> On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote: >>> How about: } else { -Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +Object lock = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); try { transientStore.store(lockKey, lock); } catch (IOException e) { This way we always lock the same object. >>> >>> Exactly this thing I was trying to explain - it will not work because >>> instead of using top level request you are suggesting to use current >>> sub-request. >> >> Sorry I am not really following. As understand you we cannot use >> ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a >> sub-request (a request for the exact same resource in a concurrent >> situation). > > Exactly. > > >> If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we >> are in CLI mode. > > (patch quoted above does not have lock == null comparison anymore, > that's why it is dangerous for HTTP env) > > >> In CLI ASAIK there is no concurrent situation meaning >> the problem cannot occur. However since the issue is talking about >> includes that may occur. > > Yes. You can still use IncludeTransformer in CLI. So deadlock can happen. > > >> Having a typical call from the cli gives for the env.getObjectModel(): >> >> HashMap values: >> [null, >> [EMAIL PROTECTED], >> link-collection=[], >> [EMAIL PROTECTED], >> null, null, null, null, >> [EMAIL PROTECTED], >> null, null, >> [EMAIL PROTECTED], >> [EMAIL PROTECTED], >> null, null, null] >> >> So which object you would suggest to lock? >> >> You wrote "Suitable alternative would be to lock against top most >> command line request." >> >> To what you are referring with "top most command line request"? > > That would be CommandLineRequest. Following HTTP environment analogy, > that would be CommandLineEnvironment.CLI_REQUEST_OBJECT. > > > Vadim > > >> salu2 >> >>> >>> Take a look at EnvironmentWrapper (which is used e.g. in SitemapSource): >>> >>> // create new object model and replace the request object >>> Map oldObjectModel = env.getObjectModel(); >>> if (oldObjectModel instanceof HashMap) { >>> this.objectModel = (Map)((HashMap)oldObjectModel).clone(); >>> } else { >>> this.objectModel = new HashMap(oldObjectModel.size()*2); >>> Iterator entries = oldObjectModel.entrySet().iterator(); >>> Map.Entry entry; >>> while (entries.hasNext()) { >>> entry = (Map.Entry)entries.next(); >>> this.objectModel.put(entry.getKey(), entry.getValue()); >>> } >>> } >>> this.request = new >>> RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel), >>> requestURI, >>> queryString, >>> this, >>> rawMode); >>> this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, >>> this.request); >>> >>> >>> Vadim >> -- >> Thorsten Scherler thorsten.at.apache.org >> Open Source Java consulting, training and solutions >> >
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 22, 2008, at 8:57 AM, Thorsten Scherler wrote: On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote: On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote: How about: } else { -Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +Object lock = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); try { transientStore.store(lockKey, lock); } catch (IOException e) { This way we always lock the same object. Exactly this thing I was trying to explain - it will not work because instead of using top level request you are suggesting to use current sub-request. Sorry I am not really following. As understand you we cannot use ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a sub-request (a request for the exact same resource in a concurrent situation). Exactly. If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we are in CLI mode. (patch quoted above does not have lock == null comparison anymore, that's why it is dangerous for HTTP env) In CLI ASAIK there is no concurrent situation meaning the problem cannot occur. However since the issue is talking about includes that may occur. Yes. You can still use IncludeTransformer in CLI. So deadlock can happen. Having a typical call from the cli gives for the env.getObjectModel(): HashMap values: [null, [EMAIL PROTECTED], link-collection=[], [EMAIL PROTECTED], null, null, null, null, context = org [EMAIL PROTECTED], null, null, source- resolver = org [EMAIL PROTECTED], org .apache .cocoon .components .CocoonComponentManager [EMAIL PROTECTED], null, null, null] So which object you would suggest to lock? You wrote "Suitable alternative would be to lock against top most command line request." To what you are referring with "top most command line request"? That would be CommandLineRequest. Following HTTP environment analogy, that would be CommandLineEnvironment.CLI_REQUEST_OBJECT. Vadim salu2 Take a look at EnvironmentWrapper (which is used e.g. in SitemapSource): // create new object model and replace the request object Map oldObjectModel = env.getObjectModel(); if (oldObjectModel instanceof HashMap) { this.objectModel = (Map) ((HashMap)oldObjectModel).clone(); } else { this.objectModel = new HashMap(oldObjectModel.size()*2); Iterator entries = oldObjectModel.entrySet().iterator(); Map.Entry entry; while (entries.hasNext()) { entry = (Map.Entry)entries.next(); this.objectModel.put(entry.getKey(), entry.getValue()); } } this.request = new RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel), requestURI, queryString, this, rawMode); this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, this.request); Vadim -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Fri, 2008-08-22 at 08:18 -0400, Vadim Gritsenko wrote: > On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote: > > > How about: > > } else { > > -Object lock = > > env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); > > +Object lock = > > env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); > > try { > > transientStore.store(lockKey, lock); > > } catch (IOException e) { > > > > This way we always lock the same object. > > Exactly this thing I was trying to explain - it will not work because > instead of using top level request you are suggesting to use current > sub-request. Sorry I am not really following. As understand you we cannot use ObjectModelHelper.REQUEST_OBJECT since we cannot be sure that is not a sub-request (a request for the exact same resource in a concurrent situation). If the lock is null for a HttpEnvironment.HTTP_REQUEST_OBJECT then we are in CLI mode. In CLI ASAIK there is no concurrent situation meaning the problem cannot occur. However since the issue is talking about includes that may occur. Having a typical call from the cli gives for the env.getObjectModel(): HashMap values: [null, [EMAIL PROTECTED], link-collection=[], [EMAIL PROTECTED], null, null, null, null, [EMAIL PROTECTED], null, null, [EMAIL PROTECTED], [EMAIL PROTECTED], null, null, null] So which object you would suggest to lock? You wrote "Suitable alternative would be to lock against top most command line request." To what you are referring with "top most command line request"? salu2 > > Take a look at EnvironmentWrapper (which is used e.g. in SitemapSource): > > // create new object model and replace the request object > Map oldObjectModel = env.getObjectModel(); > if (oldObjectModel instanceof HashMap) { > this.objectModel = (Map)((HashMap)oldObjectModel).clone(); > } else { > this.objectModel = new HashMap(oldObjectModel.size()*2); > Iterator entries = oldObjectModel.entrySet().iterator(); > Map.Entry entry; > while (entries.hasNext()) { > entry = (Map.Entry)entries.next(); > this.objectModel.put(entry.getKey(), entry.getValue()); > } > } > this.request = new > RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel), >requestURI, >queryString, >this, >rawMode); > this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, > this.request); > > > Vadim -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 22, 2008, at 3:35 AM, Thorsten Scherler wrote: How about: } else { -Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +Object lock = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); try { transientStore.store(lockKey, lock); } catch (IOException e) { This way we always lock the same object. Exactly this thing I was trying to explain - it will not work because instead of using top level request you are suggesting to use current sub-request. Take a look at EnvironmentWrapper (which is used e.g. in SitemapSource): // create new object model and replace the request object Map oldObjectModel = env.getObjectModel(); if (oldObjectModel instanceof HashMap) { this.objectModel = (Map)((HashMap)oldObjectModel).clone(); } else { this.objectModel = new HashMap(oldObjectModel.size()*2); Iterator entries = oldObjectModel.entrySet().iterator(); Map.Entry entry; while (entries.hasNext()) { entry = (Map.Entry)entries.next(); this.objectModel.put(entry.getKey(), entry.getValue()); } } this.request = new RequestWrapper(ObjectModelHelper.getRequest(oldObjectModel), requestURI, queryString, this, rawMode); this.objectModel.put(ObjectModelHelper.REQUEST_OBJECT, this.request); Vadim
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Fri, 2008-08-22 at 09:35 +0200, Thorsten Scherler wrote: > On Thu, 2008-08-21 at 19:40 -0400, Vadim Gritsenko wrote: > > On Aug 21, 2008, at 8:18 AM, Thorsten Scherler wrote: > > > > > still updating forrest to use cocoon-2.1.x and I found a problem in > > > the > > > AbstractCachingProcessingPipeline. > > > > > > I am not sure whether someone is using the cocoon cli ATM. Forrest is > > > based around this component. > > > > > > https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340 > > > #action_12624340 ... > > > Can somebody verify that it is a bug? Should I commit the patch? > > > > It is a bug, but with proposed change you can get a deadlock under > > some conditions (IIRC, when using parallels inclusion in the sitemap > > which ultimately pull from the same pipeline). The idea behind that > > lock is to synchronize on the main request (top most request), and not > > on any of sub-request object created during aggregation. Suitable > > alternative would be to lock against top most command line request. > > How about: Index: src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java === --- src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java (revision 681296) +++ src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java (working copy) @@ -36,7 +36,7 @@ import org.apache.cocoon.caching.ComponentCacheKey; import org.apache.cocoon.caching.PipelineCacheKey; import org.apache.cocoon.environment.Environment; -import org.apache.cocoon.environment.http.HttpEnvironment; +import org.apache.cocoon.environment.ObjectModelHelper; import org.apache.cocoon.transformation.Transformer; import org.apache.cocoon.util.HashUtil; import org.apache.excalibur.source.SourceValidity; @@ -192,7 +192,7 @@ } // Avoid deadlock with self (see JIRA COCOON-1985). -Object current = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +Object current = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); if (lock != null && lock != current) { if (getLogger().isDebugEnabled()) { getLogger().debug("Waiting on Lock '" + lockKey + "'"); @@ -242,7 +242,7 @@ getLogger().debug("Lock EXISTS: '" + lockKey + "'"); } } else { -Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +Object lock = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); try { transientStore.store(lockKey, lock); } catch (IOException e) { This way we always lock the same object. I had a look as well on COCOON-1985 and it should be compatible. Thanks for the feedback Vadim. salu2 -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Thu, 2008-08-21 at 19:40 -0400, Vadim Gritsenko wrote: > On Aug 21, 2008, at 8:18 AM, Thorsten Scherler wrote: > > > still updating forrest to use cocoon-2.1.x and I found a problem in > > the > > AbstractCachingProcessingPipeline. > > > > I am not sure whether someone is using the cocoon cli ATM. Forrest is > > based around this component. > > > > https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340 > > #action_12624340 > > > > I found that in > > org > > .apache > > .cocoon.components.pipeline.impl.AbstractCachingProcessingPipeline > > line 245 > > Object lock = > > env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); > > the lock is null which causes the NPE in the end. > > > > I changed it the following way: > > Object lock = > > env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); > > +if (null==lock){ > > + lock = > > env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); > > +} > > try { > > > > and now it is working as before. > > > > Can somebody verify that it is a bug? Should I commit the patch? > > It is a bug, but with proposed change you can get a deadlock under > some conditions (IIRC, when using parallels inclusion in the sitemap > which ultimately pull from the same pipeline). The idea behind that > lock is to synchronize on the main request (top most request), and not > on any of sub-request object created during aggregation. Suitable > alternative would be to lock against top most command line request. How about: Index: src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java === --- src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java (revision 681296) +++ src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java (working copy) @@ -36,6 +36,7 @@ import org.apache.cocoon.caching.ComponentCacheKey; import org.apache.cocoon.caching.PipelineCacheKey; import org.apache.cocoon.environment.Environment; +import org.apache.cocoon.environment.ObjectModelHelper; import org.apache.cocoon.environment.http.HttpEnvironment; import org.apache.cocoon.transformation.Transformer; import org.apache.cocoon.util.HashUtil; @@ -242,7 +243,7 @@ getLogger().debug("Lock EXISTS: '" + lockKey + "'"); } } else { -Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +Object lock = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); try { transientStore.store(lockKey, lock); } catch (IOException e) { This way we always lock the same object. Thanks for the feedback Vadim. salu2 > > Vadim -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions
Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
On Aug 21, 2008, at 8:18 AM, Thorsten Scherler wrote: still updating forrest to use cocoon-2.1.x and I found a problem in the AbstractCachingProcessingPipeline. I am not sure whether someone is using the cocoon cli ATM. Forrest is based around this component. https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340 #action_12624340 I found that in org .apache .cocoon.components.pipeline.impl.AbstractCachingProcessingPipeline line 245 Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); the lock is null which causes the NPE in the end. I changed it the following way: Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +if (null==lock){ + lock = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); +} try { and now it is working as before. Can somebody verify that it is a bug? Should I commit the patch? It is a bug, but with proposed change you can get a deadlock under some conditions (IIRC, when using parallels inclusion in the sitemap which ultimately pull from the same pipeline). The idea behind that lock is to synchronize on the main request (top most request), and not on any of sub-request object created during aggregation. Suitable alternative would be to lock against top most command line request. Vadim
[2.1] AbstractCachingProcessingPipeline and cocoon CLI
Hi all, still updating forrest to use cocoon-2.1.x and I found a problem in the AbstractCachingProcessingPipeline. I am not sure whether someone is using the cocoon cli ATM. Forrest is based around this component. https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340#action_12624340 I found that in org.apache.cocoon.components.pipeline.impl.AbstractCachingProcessingPipeline line 245 Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); the lock is null which causes the NPE in the end. I changed it the following way: Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT); +if (null==lock){ + lock = env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT); +} try { and now it is working as before. Can somebody verify that it is a bug? Should I commit the patch? TIA for any feedback. salu2 -- Thorsten Scherler thorsten.at.apache.org Open Source Java consulting, training and solutions