On Fri, 2012-06-01 at 14:02 +0200, Greg Wilkins wrote:
> On 1 June 2012 02:49, Rajiv Mordani <rajiv.mordani_at_oracle.com> wrote:
> > We have discussed this in the past and what we said was b above.
>
> It was discussed, but I must have missed the decision to go for b, nor
> is it clear in the document or javadoc.
>
> I think this is poor choice as it means that a container and
> application cannot control their memory footprint. Consider an
> application that allocates some large byte arrays in some kind of
> content cache. It can be configured with a maximum memory usage and
> can be constrained. But when it starts sending these buffers
> asynchronously, it does not know if a write will copy the data. It
> might right the same content to an unlimited number of connections, so
> it may do an unlimited number of copies. Such an application would
> always be vulnerable to OOM and there is nothing the container or
> application can do to prevent it other than limit the number of
> connections - but the whole point of async IO is to increase
> scalability not to limit connections!
We get the idea, I believe. The idea is to limit the amount of changes,
and provide something easy to use.
The application should be concerned about how it sends its
data. If the programmer cannot code, he will not be able to use this new
async API. Since there's the servlet buffer, using a trivial loop over
write(byte[], int off, int len) will not change anything to the actual
IO, and your edge case is covered. This edge case is best covered with a
sendfile-like API, to go along with the option b) which we much prefer
[since it works better everywhere else]. I am ok with adding it, but
others are apparently not.
> > I have clarified this earlier as well and I think it is in the spec too -
> > but the onWritePossible is called once for every write possible. Once
> > invoked, it will not be invoked till a write operation is performed or the
> > canWrite is flipped.
>
>
> The document says:
>
> void onWritePossible(). This method will be invoked by the container for
> the associated NonBlockingOutputSink object when it is possible to write data.
> This method will be invoked once for every write operation possible. Once a
> write operation is performed, this method will subsequently be invoked by the
> container when further writes are possible.
>
> The javadoc says:
> void onWritePossible(ServletResponse response)
> This callback will be invoked when data can be written without blocking.
>
> Neither those descriptions nor the one you gave in your response make
> it very clear exactly what is meant. You say "will not be invoked
> till a write operation is performed" and the document says "invoked
> once for every write operation possible"
> So all of these imply that onWritePossible will be called after every
> write operations - is this correct? I thought the NIO.2 style was
> rejected because of the expense of callbacks, yet this proposal will
> have a callback per write?
>
> I had thought that the intention was that it would only be called if a
> write had causes canWrite status to flip from true to false.
> However, thinking about this now, that represents a horrible race
> condition as canWrite might flip to false during the call to write,
> but then might flip back to true before it is called, so the caller
> will not know if onWritePossilble will be called or not. If the
> intention is to have the callback happen only if the status is
> flipped, then to avoid the race then I think we need to say that
> onWritePossible is called ONLY if a call to canWrite() has returned
> false. ie if the internal canWrite status momentarily is false,
> that does not mean onWritePossible will be called, because if the
> write completes before canWrite() is called, then there is no need for
> the callback.
Yes, this is why I said that the callback must happen after the
canWrite() method is called and returns false, this is actually the
trigger [it is not when the flag flips]. This is an important trick to
make it work.
I think I described the algorithm in detail already.
> Also the document and javadoc has no text explaining the context of
> the callback thread. I assume that it is a thread fully anointed
> with servletcontext scope an security stuff? But for a suspended
> request that may have passed through several contexts, which context
> will this be? Also if the model really is a callback after every
> write, then many such IO frameworks will use the thread that called
> write to do the callback if the write completed. If we allow this,
> then we may have to do similar stuff to NIO.2 to prevent unconstrained
> stack growth if the callback does another write that does another
> callback etc. This problem is avoided if onWritePossible is only
> called if canWrite returns false.
Yes, all callbacks are in the EE context, like all Servlet invocations.
We already discussed this and the implications of the callback count,
and the need to limit it, please look at the archives. I think you were
actually the one advocating a NIO2 style API, with a callback after
every write.
> Is there a reference implementation of this available anywhere?
> What about some sample code that uses this API?
You can look at JBoss AS 7, it is not the same API, the behavior is
different sometimes, but it has a similar concept where the usual
blocking IO is extend with async capabilities [and protocol upgrade
support].
--
Remy Maucherat <rmaucher_at_redhat.com>
Red Hat Inc