You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@cxf.apache.org by Jan Hallonstén <ja...@predictly.se> on 2018/10/18 19:17:54 UTC

Response always handled on current thread when WorkQueue rejects execution

Hi,

When the WorkQueue rejects the execution in
HttpConduit.handleResponseOnWorkqueue(boolean allowCurrentThread, boolean
forceWQ) and allowCurrentThread is false the handling of the response is
always done on the current thread. I would expect it to be the other way
around. In my scenario the method is called from the AsyncHttpConduit and
the response will be handled by one of the io core threads which is not
something I want. I would like to be able to set
AsyncExecuteTimeoutRejection when using the async cxf client. So is this a
bug that should be reported or does it work as designed?

} catch (RejectedExecutionException rex) {
     if (allowCurrentThread
         && policy != null
         && policy.isSetAsyncExecuteTimeoutRejection()
         && policy.isAsyncExecuteTimeoutRejection()) {
         throw rex;
     }
     if (!hasLoggedAsyncWarning) {
         LOG.warning("EXECUTOR_FULL_WARNING");
         hasLoggedAsyncWarning = true;
     }
     LOG.fine("EXECUTOR_FULL");
     handleResponseInternal();
}

I would like to negate allowCurrentThread in the if statement instead

if (!allowCurrentThread
      && policy != null
      && policy.isSetAsyncExecuteTimeoutRejection()
      && policy.isAsyncExecuteTimeoutRejection()) {
      throw rex;
}

Link to the relevant code at Github
https://github.com/apache/cxf/blob/540bb76f6f3d3d23944c566905f9f395c6f86b79/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java#L1245

Regards,
Jan

Re: Response always handled on current thread when WorkQueue rejects execution

Posted by Jan Hallonstén <ja...@predictly.se>.
Created Jira https://issues.apache.org/jira/browse/CXF-7881

On Fri, Oct 19, 2018 at 11:34 PM Andy McCright <j....@gmail.com>
wrote:

> Hi Jan,
>
> Yes, that code definitely looks suspect.  Can you open a JIRA[1] for this?
> I have a test case that runs through your changes and confirms that if
> allowCurrentThread is true, it runs handles the response on the same
> thread, and if it is false (and policy is non-null and it's
> asyncExecuteTimeoutRejection property is true), then it throws the
> RejectedExecutionException.
>
> Thanks for reporting this issue!
>
> Andy
>
> [1]
>
> https://issues.apache.org/jira/projects/CXF/issues/CXF-7431?filter=allopenissues
>
> On Fri, Oct 19, 2018 at 5:25 AM Jan Hallonstén <ja...@predictly.se> wrote:
>
> > I wasn't really thinking yesterday the behavior I would expect is rather
> >
> > } catch (RejectedExecutionException rex) {
> >      if (!allowCurrentThread
> >          || (policy != null
> >          && policy.isSetAsyncExecuteTimeoutRejection()
> >          && policy.isAsyncExecuteTimeoutRejection())) {
> >          throw rex;
> >      }
> >      if (!hasLoggedAsyncWarning) {
> >          LOG.warning("EXECUTOR_FULL_WARNING");
> >          hasLoggedAsyncWarning = true;
> >      }
> >      LOG.fine("EXECUTOR_FULL");
> >      handleResponseInternal();
> > }
> >
> > On Thu, Oct 18, 2018 at 9:17 PM Jan Hallonstén <ja...@predictly.se> wrote:
> >
> > > Hi,
> > >
> > > When the WorkQueue rejects the execution in
> > > HttpConduit.handleResponseOnWorkqueue(boolean allowCurrentThread,
> boolean
> > > forceWQ) and allowCurrentThread is false the handling of the response
> is
> > > always done on the current thread. I would expect it to be the other
> way
> > > around. In my scenario the method is called from the AsyncHttpConduit
> and
> > > the response will be handled by one of the io core threads which is not
> > > something I want. I would like to be able to set
> > > AsyncExecuteTimeoutRejection when using the async cxf client. So is
> this
> > a
> > > bug that should be reported or does it work as designed?
> > >
> > > } catch (RejectedExecutionException rex) {
> > >      if (allowCurrentThread
> > >          && policy != null
> > >          && policy.isSetAsyncExecuteTimeoutRejection()
> > >          && policy.isAsyncExecuteTimeoutRejection()) {
> > >          throw rex;
> > >      }
> > >      if (!hasLoggedAsyncWarning) {
> > >          LOG.warning("EXECUTOR_FULL_WARNING");
> > >          hasLoggedAsyncWarning = true;
> > >      }
> > >      LOG.fine("EXECUTOR_FULL");
> > >      handleResponseInternal();
> > > }
> > >
> > > I would like to negate allowCurrentThread in the if statement instead
> > >
> > > if (!allowCurrentThread
> > >       && policy != null
> > >       && policy.isSetAsyncExecuteTimeoutRejection()
> > >       && policy.isAsyncExecuteTimeoutRejection()) {
> > >       throw rex;
> > > }
> > >
> > > Link to the relevant code at Github
> > >
> > >
> >
> https://github.com/apache/cxf/blob/540bb76f6f3d3d23944c566905f9f395c6f86b79/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java#L1245
> > >
> > > Regards,
> > > Jan
> > >
> >
>

Re: Response always handled on current thread when WorkQueue rejects execution

Posted by Andy McCright <j....@gmail.com>.
Hi Jan,

Yes, that code definitely looks suspect.  Can you open a JIRA[1] for this?
I have a test case that runs through your changes and confirms that if
allowCurrentThread is true, it runs handles the response on the same
thread, and if it is false (and policy is non-null and it's
asyncExecuteTimeoutRejection property is true), then it throws the
RejectedExecutionException.

Thanks for reporting this issue!

Andy

[1]
https://issues.apache.org/jira/projects/CXF/issues/CXF-7431?filter=allopenissues

On Fri, Oct 19, 2018 at 5:25 AM Jan Hallonstén <ja...@predictly.se> wrote:

> I wasn't really thinking yesterday the behavior I would expect is rather
>
> } catch (RejectedExecutionException rex) {
>      if (!allowCurrentThread
>          || (policy != null
>          && policy.isSetAsyncExecuteTimeoutRejection()
>          && policy.isAsyncExecuteTimeoutRejection())) {
>          throw rex;
>      }
>      if (!hasLoggedAsyncWarning) {
>          LOG.warning("EXECUTOR_FULL_WARNING");
>          hasLoggedAsyncWarning = true;
>      }
>      LOG.fine("EXECUTOR_FULL");
>      handleResponseInternal();
> }
>
> On Thu, Oct 18, 2018 at 9:17 PM Jan Hallonstén <ja...@predictly.se> wrote:
>
> > Hi,
> >
> > When the WorkQueue rejects the execution in
> > HttpConduit.handleResponseOnWorkqueue(boolean allowCurrentThread, boolean
> > forceWQ) and allowCurrentThread is false the handling of the response is
> > always done on the current thread. I would expect it to be the other way
> > around. In my scenario the method is called from the AsyncHttpConduit and
> > the response will be handled by one of the io core threads which is not
> > something I want. I would like to be able to set
> > AsyncExecuteTimeoutRejection when using the async cxf client. So is this
> a
> > bug that should be reported or does it work as designed?
> >
> > } catch (RejectedExecutionException rex) {
> >      if (allowCurrentThread
> >          && policy != null
> >          && policy.isSetAsyncExecuteTimeoutRejection()
> >          && policy.isAsyncExecuteTimeoutRejection()) {
> >          throw rex;
> >      }
> >      if (!hasLoggedAsyncWarning) {
> >          LOG.warning("EXECUTOR_FULL_WARNING");
> >          hasLoggedAsyncWarning = true;
> >      }
> >      LOG.fine("EXECUTOR_FULL");
> >      handleResponseInternal();
> > }
> >
> > I would like to negate allowCurrentThread in the if statement instead
> >
> > if (!allowCurrentThread
> >       && policy != null
> >       && policy.isSetAsyncExecuteTimeoutRejection()
> >       && policy.isAsyncExecuteTimeoutRejection()) {
> >       throw rex;
> > }
> >
> > Link to the relevant code at Github
> >
> >
> https://github.com/apache/cxf/blob/540bb76f6f3d3d23944c566905f9f395c6f86b79/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java#L1245
> >
> > Regards,
> > Jan
> >
>

Re: Response always handled on current thread when WorkQueue rejects execution

Posted by Jan Hallonstén <ja...@predictly.se>.
I wasn't really thinking yesterday the behavior I would expect is rather

} catch (RejectedExecutionException rex) {
     if (!allowCurrentThread
         || (policy != null
         && policy.isSetAsyncExecuteTimeoutRejection()
         && policy.isAsyncExecuteTimeoutRejection())) {
         throw rex;
     }
     if (!hasLoggedAsyncWarning) {
         LOG.warning("EXECUTOR_FULL_WARNING");
         hasLoggedAsyncWarning = true;
     }
     LOG.fine("EXECUTOR_FULL");
     handleResponseInternal();
}

On Thu, Oct 18, 2018 at 9:17 PM Jan Hallonstén <ja...@predictly.se> wrote:

> Hi,
>
> When the WorkQueue rejects the execution in
> HttpConduit.handleResponseOnWorkqueue(boolean allowCurrentThread, boolean
> forceWQ) and allowCurrentThread is false the handling of the response is
> always done on the current thread. I would expect it to be the other way
> around. In my scenario the method is called from the AsyncHttpConduit and
> the response will be handled by one of the io core threads which is not
> something I want. I would like to be able to set
> AsyncExecuteTimeoutRejection when using the async cxf client. So is this a
> bug that should be reported or does it work as designed?
>
> } catch (RejectedExecutionException rex) {
>      if (allowCurrentThread
>          && policy != null
>          && policy.isSetAsyncExecuteTimeoutRejection()
>          && policy.isAsyncExecuteTimeoutRejection()) {
>          throw rex;
>      }
>      if (!hasLoggedAsyncWarning) {
>          LOG.warning("EXECUTOR_FULL_WARNING");
>          hasLoggedAsyncWarning = true;
>      }
>      LOG.fine("EXECUTOR_FULL");
>      handleResponseInternal();
> }
>
> I would like to negate allowCurrentThread in the if statement instead
>
> if (!allowCurrentThread
>       && policy != null
>       && policy.isSetAsyncExecuteTimeoutRejection()
>       && policy.isAsyncExecuteTimeoutRejection()) {
>       throw rex;
> }
>
> Link to the relevant code at Github
>
> https://github.com/apache/cxf/blob/540bb76f6f3d3d23944c566905f9f395c6f86b79/rt/transports/http/src/main/java/org/apache/cxf/transport/http/HTTPConduit.java#L1245
>
> Regards,
> Jan
>