Discussion:
Response always handled on current thread when WorkQueue rejects execution
Jan Hallonstén
2018-10-18 19:17:54 UTC
Permalink
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
Jan Hallonstén
2018-10-19 10:24:48 UTC
Permalink
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();
}
Post by Jan Hallonstén
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
Andy McCright
2018-10-19 21:34:15 UTC
Permalink
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
Post by Jan Hallonstén
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();
}
Post by Jan Hallonstén
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
Post by Jan Hallonstén
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
Post by Jan Hallonstén
Regards,
Jan
Jan Hallonstén
2018-10-22 15:45:33 UTC
Permalink
Created Jira https://issues.apache.org/jira/browse/CXF-7881
Post by Andy McCright
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
Post by Jan Hallonstén
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();
}
Post by Jan Hallonstén
Hi,
When the WorkQueue rejects the execution in
HttpConduit.handleResponseOnWorkqueue(boolean allowCurrentThread,
boolean
Post by Jan Hallonstén
Post by Jan Hallonstén
forceWQ) and allowCurrentThread is false the handling of the response
is
Post by Jan Hallonstén
Post by Jan Hallonstén
always done on the current thread. I would expect it to be the other
way
Post by Jan Hallonstén
Post by Jan Hallonstén
around. In my scenario the method is called from the AsyncHttpConduit
and
Post by Jan Hallonstén
Post by Jan Hallonstén
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
Post by Jan Hallonstén
a
Post by Jan Hallonstén
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
Post by Jan Hallonstén
Post by Jan Hallonstén
Regards,
Jan
Loading...