[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-06 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

--- Comment #8 from Remy Maucherat  ---
That's a useful clarification actually, the standard executor is not an
ExecutorService, there's another thread pool in there that is an
ExecutorService. It is likely I'll adjust the connector startup logging to make
it clear a separate executor should probably not be used with NIO2.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-04 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

--- Comment #7 from Ravi Sanwal  ---
I partly understand what you said. AND as I mentioned in my previous comment
about the workaround, there is a way out.
Your statement, however, about the tomcat standard executor being an
ExecutorService (for which there is a check) is not correct. It indeed is a
java.util.concurrent.Executor, but not ExecutorService.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-03 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

--- Comment #6 from Remy Maucherat  ---
So you're saying effectively that the NIO2 connector would be better off if it
used *two* separate executors: one ExecutorService internally (NIO2 needs one),
and an external executor. And most processing will need to be executed twice on
two different executors. Since I didn't think the behavior would be useful, I
wrapped the executor assuming it is an ExceutorService (the thread pool Tomcat
provides that is the standard implementation for the Executor element is one)
since that is the most efficient solution.

It is open source, you can submit a patch but I'm quite skeptical. It would
have to use a master flag (most likely the internalExecutor flag can work).

I would instead recommend trying to use a pool that implements ExecutorService,
it looks to me like a useful interface for thread management.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-03 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

--- Comment #5 from Ravi Sanwal  ---
Typo in previous comment.
It should be.

We should at lease fix the connector documentation for this behavior and
explain that it is better to not use an external executor *but* an internal one
specific to the connector.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-03 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

--- Comment #4 from Ravi Sanwal  ---
Can it at least give the user some control put a cap on the number of threads?
May be I am not explaining my problem correctly.
Regardless.
Tomcat NIO2 connector still doesn't follow the rules of a defined executor
service.
Why are the async channel group threads processing requests? They are not
supposed to. There is a dedicated executor for that.
Moreover, if this is allowed, what's the point of having an executor at all.

One of the reasons people use an executor is to limit the number of request
being processed concurrently. If we can't limit this by using an executor with
bounds, then the server may just get overwhelmed with unbounded number of
requests.

Anyway, there is still a way to limit the number of threads by not using an
executor but setting the bounds on number of threads on the connector itself so
that is an internal executor thus passing the ExecutorService check. So we may
have to resort to using that.

We should at lease fix the connector documentation for this behavior and
explain that it is better to not use an external executor by an internal one
specific to the connector.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-03 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

Remy Maucherat  changed:

   What|Removed |Added

   Severity|normal  |enhancement
 Resolution|--- |WONTFIX
 Status|REOPENED|RESOLVED

--- Comment #3 from Remy Maucherat  ---
The NIO2 connector only works with an ExecutorService that is exclusive to this
connector. A side effect of the design really.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-03 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

Ravi Sanwal  changed:

   What|Removed |Added

 Resolution|INVALID |---
 Status|RESOLVED|REOPENED

--- Comment #2 from Ravi Sanwal  ---
OK,
My email reply didn't make it here.

The snippet you mentioned here checks if the supplied executor is instance of
ExecutorService is.
Well, an executor configured in server.xml is not. Which means that the
threadGroup is never created (with the provided thread pool) and eventually it
ends up using the default case. Which is an unbounded thread pool.

In addition to this undesired behavior, for NIO2 connector, these threads (the
ones to process async IO) are reused as-is to process new requests. If we have
configured an executor in server.xml, our expectation is that requests are
processed (or in case of async, are started to be processed) by the executor
threads.

A small example would reveal this problem.
Configure a server with an executor of size 1 (max). Write an async servlet to
serve a filesystem resource. Hit it from a client with lots of requests coming
in parallel and see what threads on tomcat jvm are doing.

Another one to try is, have two servlets one async and one non-async. Send a
request from a browser to the async servlet first and then to the other one.
You'll see that the executor thread is not used for the second one, instead
since the first request was "kept-alive" the second one lands on the same
connection and so, uses the thread that was previously used to write output on
that connection for the async request. Which is not an executor thread, but a
thread from the async channel processor group.

The side-effect of this is that there is no way to throttle threads. They just
keep growing, even if most of the just sit idle.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org



[Bug 59650] NIO2 connector does not respect executor bounds, reuses NIO.2 threads for processing request on connections

2016-06-01 Thread bugzilla
https://bz.apache.org/bugzilla/show_bug.cgi?id=59650

Remy Maucherat  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |INVALID
 OS||All

--- Comment #1 from Remy Maucherat  ---
The NIO2 connector wraps the executor as the thread group, this executor should
respect the thread count limits, etc.
// Create worker collection
if ( getExecutor() == null ) {
createExecutor();
}
if (getExecutor() instanceof ExecutorService) {
threadGroup =
AsynchronousChannelGroup.withThreadPool((ExecutorService) getExecutor());
}

What is the problem exactly ?

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org