tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Costin Manolache" <cos...@gmail.com>
Subject Re: Proposed simplification of CometEvent
Date Thu, 14 Jun 2007 21:33:08 GMT
On 6/14/07, Filip Hanik - Dev Lists <devlists@hanik.com> wrote:
>
> Costin Manolache wrote:
> >> >
> >> >
> >> > Sounds better - but as Remy explained you would first need to explain
> >> > why blocking is needed in this context and how to deal with the
> >> confusion
> >> > of mixing blocking and non-blocking for users, and the implementation
> >> > complexities it adds.
> >> trunk doesn't mix them. a comet connection is either blocking or non
> >> blocking, it doesn't shift between the two,
> >> and it allows developers to choose what they want. Just like a
> >> SocketChannel in java.nio.
> >> there is nothing confusing about that, unless java.nio is confusing :)
> >
> >
> >
> > Well, nio is far from perfect - but that's not the point.
> >
> > Servlets have a very nice blocking mechanism already - it's the
> > servlet API
> > :-).
> > The question is why would you need to have a Commet connection blocking.




because comet is not a servlet, and the ease of use for having blocking
> write and read is huge,
> especially when Tomcat can notify you when you can write or read, there
> is no need to scratch your head and try to use non blocking.
> non blocking is more complex, and not for everyone.



I'm not sure I understand - there is a perfectly fine blocking read/write
interface, it's the
plain servlet API.

I also agree that 'blocking' mode is sometimes easier to code than
event-based, and
I can see the benefit of doing some stuff in blocking mode and some in
non-blocking.

My concern was making the read and write blocking in a commet servlet based
on
a config in the CometEvent.

The alternative is to have the comet servlet allways non-blocking for
read/write, but
provide a convenience method that will simulate the blocking read or write (
which is
easy, all you need is a blocking waitForEvent(time) ). Benefits:
- the read/write implementation is simpler ( no need to check the config
mode or do tricks ),
- comet is easier to understand
- you can do more advanced things, like using reads in non-blocking mode and
writes in blocking
mode.

The code using waitForEvent() is a bit more complicated than the code for
blocking read/write (
but if you want the simplest solution - use regular servlet ), and it is
simpler than a pure
callback based model.



>
> > I think it's very reasonable to add a blocking waitForEvent() to allow
> > servlets have a
> > simpler ( but less efficient ) implementation.




Comet inherently is a "wait for event" system. Tomcat acts as a
> multiplexer/dispatcher,and fires events into the CometProcessor.


Comet seems to be an event-based system, that's why setting it to 'blocking
mode' is so
confusing and bad. I'm not sure I agree it has to be a 'blocking wait for
event' system - in Remy
examples at least it is not.



The "waitForEvent()" method is obsolete in the Comet implementation, as
> you are either on a Tomcat thread,
> which you can activate at any time or you are on a async thread, and not
> sure why you would need to block an async background thread.
> >
> > Think about utilities that take the event as param - would they need to
> > check first
> > if it's blocking or not ? And what would blocking give you in addition
> to
> > waitForEvent() -
> > which is actually better since it allows you to un-block on any event,
> > not
> > only a specific
> > read/write.
> I can see that there is a huge misunderstanding of how comet actually
> does work and what it actually is.
> It is true that is essentially just a TCP socket between server and
> client, but in addition to that,
> Tomcat provides a shell around it, and instead of you having to manage
> when to read or when to write,
> tomcat becomes an event API.


Sure, it hides some limitations of NIO ( the IO thread that has to do all
things )
and adds some higher-level HTTP stuff.




> >> >> > - please don't call the method configure(), it's commonly used
> >> with a
> >> >> > different meaning ( i.e. setting the port or general
> >> configuration).
> >> >> > setConnectionMode, etc. And using the enum doesn't sound
> consistent
> >> >> with
> >> >> > other APIs either.
> >> >> we can call it whatever we want. But saying not using enum, its not
> >> >> consistent with other APIs in Tomcat,
> >> >> means would never take advantage of new language features ever, I
> >> think
> >> >> that would be a shame.
> >> >
> >> >
> >> > Same as above - the question is not about using new features, but
> >> if the
> >> > features
> >> > fit the use. I have no problem with using enums for the event types -
> >> > just
> >> > for
> >> > configure, in the context of configure(enum) versus setBlocking(),
> >> > setFoo().
> >> this has been adjusted based on the feedback, the method is now
> >> configureBlocking(boolean)
> >> the state of it can be used by calling isBlocking()
> >>
> >> register is using enums, mainly cause Remy, while he was working with
> >> this API insisted on it.
> >> I had preferred using an int, just like the socket API, but since Remy
> >> had initially agreed to register, and proposed enum and unregister
> >> we went with that.
> >
> >
> > Ok.
> I still think an int is better :)
> >
> >
> >>
> >> >
> >> >
> >> >
> >> >
> >> >> > - see bellow - I don't think I understand the benefits of mixing
> >> >> blocking
> >> >> > and non-blocking in this interface, it is quite confusing.
> >> >> It would be mixing it, its a one time config, during the BEGIN
> event,
> >> >> you say
> >> >> configureBlocking(true) or configureBlocking(false).
> >> >> Comet is very much connection centric, so you can't mix it.
> >> >>
> >> >> In the trunk API, its clear to what you are using, blocking or non
> >> >> blocking, in the sandbox API, the swap
> >> >> of it happens when invoking isWriteable or isReadable, making the
> >> state
> >> >> of the comet connection confusing to the developer.
> >> >
> >> >
> >> > I'm not sure it's true - my understanding is that sandbox is all
> >> > non-blocking.
> >> > Invoking isWriteable is not blocking.
> >> >
> >> > I think it would be ok to add a blocking waitForEvent() - combined
> >> with
> >> > isReadable()/isWriteable()
> >> this would be a dead lock, as the Comet API must guarantee that a
> >> CometProcessor.event
> >> is only invoked by one worker thread at any time. The blocking you are
> >> talking about can be done
> >> using an async thread can be done by registering for the event you wish
> >> to be notified in and then
> >> maybe await a latch countdown, or doing a sync/wait() combo.
> >
> >
> > What would happen in the blocking case when a different event happens ?
> > Isn't it the same, if you want to guarantee single-threaded behaviour ?
> >
> > Well - is there any docs on what is the intended thread model of comet
> > servlets,
> A comet servlet is not single-threaded, but a comet event is. from
> Tomcat's stand point.
> furthermore, a comet event (which represent a socket connection) can be
> used multithreaded,
> but it will be the developers responsibility to sync appropriately.



Ok, it makes sense. But I assume the comet event is not imutable - i.e.
you get the event, and its state can change ( isReadable() will change,
it can represent another state, etc ).



> i.e. SingelThreaded or reentrant ? Multiple events can happen at same
> > time
> > ( or very close -
> > one event soon after a Comet servlet is called, before it returns), so
> > this
> > is a general problem.
> >
> >
> >> it will allow a sort-of blocking model. But having methods that are
> >> > blocking
> >> > sometimes
> >> > and sometimes not - based on some config - is dangerous and
> confusing.
> >> As I mentioned, the trunk Comet API is actually very similar to
> java.nio
> >> API, hence developers familiar with that will instantly recognize
> >> themselves and should get comfortable with the API.
> >
> >
> > I still don't recognize it as NIO :-), and I am not very comfortable
> with
> > the
> > changes between blocking and non-blocking in NIO ( including all the
> > crazy
> > things that can happen only in the IO thread ).
> exactly, and that is why in Tomcat you have the ability to truly rely on
> the Tomcat
> worker threads to get your work done. That way you never have to sync or
> worry about stuff like that
> Albeit there will be programmers that need more, and for those we can
> offer more.
> >
> >
> > Here are some of the things that might need to be clarified about the
> >> trunk API:
> >>
> >> 1. It is possible to not be registered for any events, and even do
> >> polling for data asynchronously
> >
> >
> >
> > You would still get the BEGIN event I assume.
> Yes, and events are always sent on a Tomcat thread,
> and events are guaranteed to be one-thread-per-comet-event




>
> > All you need to poll data async is waitForEvent(), or select() if you
> > like
> > to emulate nio.
> you don't even need that. this is an event API, register for the events,
> and they will happen.


Sure - all I was saying is that you could get most of the blocking
programming
model by using a waitForEvent() instead of setting the socket in blocking
mode,
just like NIO selects() ( of course, nio select must be in the io thread-
and comet will
hide this ).


>
> > 2. ERROR and END events, are an exception, you cant unregister for
> >> those, as those might require the
> >>    underlying socket to be closed, or has already been closed
> >
> >
> > In what thread would you get those ( in blocking mode ?)
> Tomcat thread,
> >
> >
> >
> > 5. The current READ event in CoyoteAdapter ends up performing a blocking
> >> read if data has been received but its not enough
> >>    for ServletInputStream.read() to actually generate data, just so
> that
> >> it can produce a real READ event,
> >>    there is no real way to get around this without a major effort
> >> described in 4)
> >
> >
> >
> >
> >> 6. sleep() and callback() are names that already have confused folks in
> >> this thread, they just don't make sense.
> >
> >
> > I agree, but easy to change :-)
> >
> >   But its not the names alone, people think of Comet as
> >> request/response stuff, when it isn't.
> >>    Comet is centered around a socket connection, and the events
> >> generated are a direct derivative of that.
> >>    That is why I have chosen to make the API similar to what's been in
> >> Java since 1.4
> >
> >
> > As long as it doesn't require you to do special things in the select()
> > thread, I'm happy :-)
> The comet programmer never has to worry about a select() thread, that
> logic is already taken care of for you, and working in 6.0.13


Thanks for the extra info, I think I understand much better now the 2
comets.
I would still vote for the sandbox version and a pure event system ( with
emulated blocking if needed, via a waitForEvent() mechanism ). In any case -
the
2 seem close enough and could be easily merged, at least at API level, but I

hope the merge will not keep the blocking configuration.


Costin

Filip
>
> >
> >
> >   Comet is definitely more advanced than a servlet, and the complexity
> >> of it is higher,
> >>    having less methods on an API wont make it less complex, since the
> >> complexity sits in understanding the concept.
> >>
> >>
> >> I'm gonna work up some examples so that we can take a look at different
> >> scenarios and how the APIs would differ
> >>
> >> Filip
> >>
> >>
> >>
> >> >
> >> > Costin
> >> >
> >> > Filip
> >> >> >
> >> >> > In the sandbox version:
> >> >> > - sleep() and setTimeout(int) -> why not sleep(int millis)
? I
> >> think
> >> >> it's
> >> >> > confusing to have both and the interactions between them, in
> >> >> > particular setTimeout is marked optional ? It makes sense to have
> >> >> > setTimeout() as a general timeout.
> >> >> >
> >> >> > - not sure I understand the use case for
> >> isReadable()/isWriteable() -
> >> >> > when
> >> >> > will this be called ? My understanding was that when
> >> >> > the connection is readable, a callback will be generated with
> >> >> > EventType ==
> >> >> > READ. Also it's very confusing to mix the 'blocking' in the
> >> >> > isReadable()/isWriteable() - it would be much cleaner to say
> >> that the
> >> >> > connection is always non-blocking, and add a method to switch
to
> >> >> blocking
> >> >> > mode ( then use the regular servlet methods maybe ). Using the
> >> >> > ComentEvent
> >> >> > for both is IMHO dangerous.
> >> >> >
> >> >> > - will sleep() still allow callbacks ( and if not - what will
> >> happen
> >> >> with
> >> >> > them )? What's going to happen with callbacks if callback() is
not
> >> >> > called ?
> >> >> >
> >> >> >
> >> >> > In general ( both versions):
> >> >> > - it would be great to move it from o.a.catalina to
> >> org.apache.comet
> >> >> >
> >> >> > Costin
> >> >> >
> >> >> > On 6/11/07, Remy Maucherat <remm@apache.org> wrote:
> >> >> >>
> >> >> >> Filip Hanik - Dev Lists wrote:
> >> >> >> > Ok, let me see if I can summarize.
> >> >> >> >
> >> >> >> > 1. Whether you write out the stored buffer using the
Poller
> >> thread,
> >> >> >> or a
> >> >> >> > Tomcat worker thread (flushed in Http11xxxProcessor)
as
> >> described
> >> >> >> below
> >> >> >> > I originally thought of this as async write, as we are
simply
> >> >> doing a
> >> >> >> > write with another one of our threads. Originally when
we were
> >> >> talking
> >> >> >> > non blocking writes, I was thinking along the lines of
non
> >> blocking
> >> >> to
> >> >> >> > where the Comet developer had to do that logic, just
as he was
> >> >> >> writing a
> >> >> >> > socket, possibly like (but not suggested) a
> >> >> >> > CometEvent.nonBlockWrite(ByteBuffer).
> >> >> >> >
> >> >> >> > 2. Do we need non blocking? with the methods of isWriteable
and
> >> the
> >> >> >> > ability to register(OP_WRITE)->event(WRITE), if the
number of
> >> bytes
> >> >> >> you
> >> >> >> > write is usually smaller than the socket buffer, chances
are
> >> that
> >> >> most
> >> >> >> > writes will be non blocking. I would even argue a large
> majority
> >> >> would
> >> >> >> > be non blocking, and thus the implementation or the complexity
> >> >> thereof
> >> >> >> > would not be needed. And with the ability to do async
writes,
> >> >> means I
> >> >> >> > can create my own thread pool/write queue to perform
these
> >> writes.
> >> >> >>
> >> >> >> You are writing the opposite thing to the previous email,
and
> >> we are
> >> >> >> back to "non blocking is useless". The problem is that I
> >> understand
> >> >> >> blocking IO as "write this data, and return when it's done".
If
> >> the
> >> >> >> socket is in blocking mode, any write done by the servlet
may
> >> block,
> >> >> >> regardless of what isWriteable says. Of course, it's very
> >> unlikely,
> >> >> >> which is why Comet in 6.0.x works.
> >> >> >>
> >> >> >> > 3. isWriteable - simple method, but I don't like that
the
> method
> >> in
> >> >> >> > itself performs actions like adding the socket to a poller
etc.
> >> >> >> >   Instead isWriteable==true means that you can write
on the
> >> socket,
> >> >> >> > isWriteable==false you cannot. This method should be
able to be
> >> >> >> invoked
> >> >> >> > as many times as its wanted, and is thread safe and doesn't
do
> >> >> >> anything
> >> >> >> > funky underneath.
> >> >> >>
> >> >> >> Ok, so you prefer a more complex API (if I follow "just in
case
> it
> >> >> was
> >> >> >> useful"). I started with an API which would expose all
> operations,
> >> >> and
> >> >> >> looked into removing what was not explicitly useful.
> >> >> >>
> >> >> >> > 4. isWriteable - I'm also reading in that you are also
> >> suggesting
> >> >> that
> >> >> >> > we use this method to declare if we want blocking or
non
> >> blocking
> >> >> >> writes.
> >> >> >>
> >> >> >> No. The situation where write could (maybe) block is if the
> >> servlet
> >> >> >> writes in a Tomcat thread. Typically, this is the reply-later
> >> design,
> >> >> >> using the sleep/callback methods. The isWriteable method is
not
> >> used,
> >> >> >> since the servlet merely wants (in that common design) to
send a
> >> >> >> response as fast as possible, and typically this sort of
> >> response is
> >> >> not
> >> >> >> too large and unlikely to cause IO problems. This blocking
> >> >> behavior is
> >> >> >> allowed in that case to avoid forcing the user to put in more
> >> complex
> >> >> >> logic to deal with the partial write + event, and is set just
for
> >> the
> >> >> >> amount of time it takes to perform the write (note that this
).
> >> >> >>
> >> >> >> >   At this point this method is doing three things:
> >> >> >> >   a) returns true/false if we can write data
> >> >> >> >   b) delegates a socket to the poller to write data and
> >> generate a
> >> >> >> > event(WRITE) to the comet processor
> >> >> >> >   c) configures a write to be blocking or non blocking
> >> >> >> >   This is for sure not what I would expect of a "simple
API",
> if
> >> >> >> simple
> >> >> >> > means less keystrokes than yes, but simple to me also
means
> >> >> intuitive
> >> >> >> > and easily understood.
> >> >> >>
> >> >> >> So you have plenty of methods to do the same thing.
> >> >> >>
> >> >> >> > Given points 1-4, this is what is going to happen to
every
> >> single
> >> >> >> developer
> >> >> >> >  I) They are going to use stream.write and event.isWriteableall
> >> >> the
> >> >> >> > time, not realizing what it actually does
> >> >> >> > II) They are going to get confused when they receive
an
> >> IOException
> >> >> >> for
> >> >> >> > trying to perform a write, cause they used isWriteable
and the
> >> >> socket
> >> >> >> > went into non blocking mode
> >> >> >>
> >> >> >> If that's what you want to believe ...
> >> >> >>
> >> >> >> > At this point, this 'simple' API, obviously not so simple,
> >> >> instead it
> >> >> >> > becomes very complicated, as I would almost have to reverse
> >> >> >> engineer the
> >> >> >> > code to truly understand what it does.
> >> >> >> > It may be simple to you and me, but that is because we
are
> >> >> >> implementing
> >> >> >> it.
> >> >> >>
> >> >> >> I really don't see what is complex, especially when you look
at
> >> the
> >> >> code
> >> >> >> the user would write for the simple cases, where you don't
even
> >> >> have to
> >> >> >> use any API besides stream.write:
> >> >> >> - reply later
> >> >> >> - wait for read events, and write data in response to it
> >> >> >>
> >> >> >> The complex case deals with handling incomplete async writes
if
> >> you
> >> >> >> don't simply drop connection.
> >> >> >>
> >> >> >> > so what does this mean to 'isReadable'? That I'm automatically
> >> >> >> > registering for a READ event if it returns false? Maybe
I don't
> >> >> want
> >> >> a
> >> >> >> > READ event, I just want to see if any data has trickled
in.
> >> so if
> >> I
> >> >> >> call
> >> >> >>
> >> >> >> > sleep(), should I then call isReadable() to reregister
for the
> >> >> >> read. how
> >> >> >> > is this simpler than that register/unregister.
> >> >> >>
> >> >> >> Read events always occur, unless you use sleep/callback. If
this
> >> >> is not
> >> >> >> written clearly in the javadocs already, I need to change
them.
> >> >> >>
> >> >> >> > Where does that leave us, well:
> >> >> >> > a) We are almost in sync on the implementation side of
it
> >> >> >>
> >> >> >> Not really, there's a big disconnect in the understanding
of non
> >> >> >> blocking vs blocking, and according to you, non blocking is
not
> >> >> useful
> >> >> >> (again).
> >> >> >>
> >> >> >> > b) I believe your API is not intuitive, nor usable, as
it
> simply
> >> >> >> doesn't
> >> >> >> > reflect what is going on and/or what a programmer wants
done
> >> >> >> > c) Your API doesn't become simpler just cause we merge
three
> >> >> methods
> >> >> >> > into one -> configure(NON_BLOCK), write(byte[]),
> >> register(OP_WRITE)
> >> >> >> > c) The API I propose, you think is overengineered, I
think it
> >> just
> >> >> >> makes
> >> >> >> > things much clearer, the fact that it automatically allows
for
> >> >> future
> >> >> >> > extension is a side effect rather than design decision
> >> >> >>
> >> >> >> My API is simpler because the code the user has to write is
more
> >> >> >> straightforward and easier to understand. Feel free to write
> small
> >> >> >> examples to see for yourself.
> >> >> >>
> >> >> >> > So bottom line is, user will get the same implementation
(or
> >> very
> >> >> >> close
> >> >> >> > to what we've talked about), question is what API are
they
> going
> >> to
> >> >> >> get?
> >> >> >>
> >> >> >> RÃ(c)my
> >> >> >>
> >> >> >>
> >> ---------------------------------------------------------------------
> >> >> >> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> >> >> >> For additional commands, e-mail: dev-help@tomcat.apache.org
> >> >> >>
> >> >> >>
> >> >> >
> >> >>
> >>
> ------------------------------------------------------------------------
> >> >> >
> >> >> > Internal Virus Database is out-of-date.
> >> >> > Checked by AVG Free Edition.
> >> >> > Version: 7.5.472 / Virus Database: 269.8.11/837 - Release Date:
> >> >> 6/6/2007
> >> >> 2:03 PM
> >> >> >
> >> >>
> >> >>
> >> >>
> ---------------------------------------------------------------------
> >> >> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> >> >> For additional commands, e-mail: dev-help@tomcat.apache.org
> >> >>
> >> >>
> >> >
> >>
> ------------------------------------------------------------------------
> >> >
> >> > No virus found in this incoming message.
> >> > Checked by AVG Free Edition.
> >> > Version: 7.5.472 / Virus Database: 269.8.15/848 - Release Date:
> >> 6/13/2007 12:50 PM
> >> >
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> >> For additional commands, e-mail: dev-help@tomcat.apache.org
> >>
> >>
> > ------------------------------------------------------------------------
> >
> > No virus found in this incoming message.
> > Checked by AVG Free Edition.
> > Version: 7.5.472 / Virus Database: 269.8.15/848 - Release Date:
> 6/13/2007 12:50 PM
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message