gmane.text.xml.cocoon.devel

Thorsten Scherler | 21 Aug 14:13

[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.

(Continue reading)

Vadim Gritsenko | 22 Aug 01:40

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 {
>

(Continue reading)

Thorsten Scherler | 22 Aug 09:35

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 {

(Continue reading)

Thorsten Scherler | 22 Aug 10:49

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
===================================================================
---

(Continue reading)

Vadim Gritsenko | 22 Aug 14:18

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();

(Continue reading)

Thorsten Scherler | 22 Aug 14:57

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():

(Continue reading)

Vadim Gritsenko | 22 Aug 16:34

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.

(Continue reading)

Rainer Pruy | 22 Aug 17:26

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 {

(Continue reading)

Vadim Gritsenko | 26 Aug 01:59

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

Thorsten Scherler | 25 Aug 12:40

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.
> 
>

(Continue reading)

Vadim Gritsenko | 26 Aug 02:23

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.
>

(Continue reading)

Thorsten Scherler | 26 Aug 09:59

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.
>

(Continue reading)

Vadim Gritsenko | 28 Aug 13:44

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

(Continue reading)

Thorsten Scherler | 28 Aug 14:25

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();

(Continue reading)

Return

Return to gmane.text.xml.cocoon.devel.

Advertisement

Project Web Page

Cocoon development

Search Archive

Language

Change language

Options

Current view: Threads only / Showing only 20 lines / Not hiding cited text.
Change to All messages, whole messages, or hide cited text.

Post a message
NNTP Newsgroup
Classic Gmane web interface
XML

XML

RSS Feed
List Information

About Gmane