Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote: nice ultimatum :) let me go through both the proposals and look it over. after that I suggest we simply have/agree on a majority vote without vetoes to be able to move on properly. in the vote announcement, I think we should simply outline the differences so that folks understand that rather than getting confused in comet concepts. you and I can draft that outline together offline, I understand your answer as that your position on this proposal did not change. As far as I am concerned, proposals are consensus, not votes. I also don't think people are interested enough to vote. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Remy Maucherat wrote: Filip Hanik - Dev Lists wrote: nice ultimatum :) let me go through both the proposals and look it over. after that I suggest we simply have/agree on a majority vote without vetoes to be able to move on properly. in the vote announcement, I think we should simply outline the differences so that folks understand that rather than getting confused in comet concepts. you and I can draft that outline together offline, I understand your answer as that your position on this proposal did not change. As far as I am concerned, proposals are consensus, not votes. I also don't think people are interested enough to vote. A vote proposal should be [VOTE] in the subject. Like [VOTE] Which Comet implementation/API to use. You just answer to an old thread so that doesn't cache the attention of community. Cheers Jean-Frederic Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Remy Maucherat wrote: Filip Hanik - Dev Lists wrote: nice ultimatum :) let me go through both the proposals and look it over. after that I suggest we simply have/agree on a majority vote without vetoes to be able to move on properly. in the vote announcement, I think we should simply outline the differences so that folks understand that rather than getting confused in comet concepts. you and I can draft that outline together offline, I understand your answer as that your position on this proposal did not change. I got confirmation of this through a private email, and since I believe a community vote on this is meaningless (at least at this time, where it's obvious nobody cares), I am withdrawing my proposal at this time, as well as withdrawing my veto for your proposal. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Remy Maucherat wrote: Hi, I've been working on additions to CometEvent to implement the additional Comet functionality that was agreed upon before creating the trunk branch. Although not functional at the moment, I consider it to be developed enough from an algorithmic standpoint to be proposed and reviewed (it is also important to not continue an API fork for too long, since otherwise it would be harder to merge, so some resolution is needed). I went through a few revisions of the API, as more tweaks appeared possible. The idea was to allow the extra functionality without adding complexity or incompatible changes, since this is most likely going to be an interim API. The repository is at: https://svn.apache.org/repos/asf/tomcat/sandbox/comet I made a number of updates since this proposal (which has been vetoed) was made. The API makes implicit a lot of behaviors which I found are not needed for the end user, and attempts to be fairly high level. In particular, I have rewritten the javadoc, which was self conflicting (due to forgetting to update some wording inherited from TC 6.0). All 6.0.13 API usage should remain compatible (I do expect at least one behavior fix in 6.0.x, though). It has the following changes over the 6.0.x branch: - additions of three methods in CometEvent (sleep and resume, and the ready flag) - sleep() delays request processing until either resume() is called, or a timeout occurs - sleep() also disables read events until resume() is called (in addition to invoking a generic event, resume() also enables read events) - resume() allows waking up requests which are asleep or waiting for an IO event - non blocking IO exclusively (with, as an exception to the rule, a switch to blocking IO in very specific cases of synchronous IO operations) - no additional data structures - new ActionCode: ACTION_COMET_TIMEOUT (which is cleanup), ACTION_COMET_RESUME (called by the resume() method), ACTION_COMET_SLEEP (called by the sleep() method), ACTION_COMET_WRITE (called by the CometEvent.ready() method without the need for an explicit callback from the servlet - ready() knows the result of the last write, and the servlet is supposed to not attempt any further writing if ready() returns false) - no additional features, like verification of caller threads (which I consider useless) - change to o.a.comet package I hope I did not forget anything. Certain items could still be changed (like the package name, or the name of the ready() method - the problem with that one is that it does not particularly imply writing, but OTOH the old canWrite and isWriteable names were quite inelegant). I don't want to block development forever, so if this proposal is refused using an outright veto like the previous version of the proposal, I will not block competing proposals using a veto of my own. However, I will most likely stop participating in trunk development (which is not the end of the world since I've been working on Tomcat for pretty much forever, so it could be a good time to move on). Comments ? / Votes ? Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Remy Maucherat wrote: Remy Maucherat wrote: Hi, I've been working on additions to CometEvent to implement the additional Comet functionality that was agreed upon before creating the trunk branch. Although not functional at the moment, I consider it to be developed enough from an algorithmic standpoint to be proposed and reviewed (it is also important to not continue an API fork for too long, since otherwise it would be harder to merge, so some resolution is needed). I went through a few revisions of the API, as more tweaks appeared possible. The idea was to allow the extra functionality without adding complexity or incompatible changes, since this is most likely going to be an interim API. The repository is at: https://svn.apache.org/repos/asf/tomcat/sandbox/comet I made a number of updates since this proposal (which has been vetoed) was made. The API makes implicit a lot of behaviors which I found are not needed for the end user, and attempts to be fairly high level. In particular, I have rewritten the javadoc, which was self conflicting (due to forgetting to update some wording inherited from TC 6.0). All 6.0.13 API usage should remain compatible (I do expect at least one behavior fix in 6.0.x, though). It has the following changes over the 6.0.x branch: - additions of three methods in CometEvent (sleep and resume, and the ready flag) - sleep() delays request processing until either resume() is called, or a timeout occurs - sleep() also disables read events until resume() is called (in addition to invoking a generic event, resume() also enables read events) - resume() allows waking up requests which are asleep or waiting for an IO event - non blocking IO exclusively (with, as an exception to the rule, a switch to blocking IO in very specific cases of synchronous IO operations) - no additional data structures - new ActionCode: ACTION_COMET_TIMEOUT (which is cleanup), ACTION_COMET_RESUME (called by the resume() method), ACTION_COMET_SLEEP (called by the sleep() method), ACTION_COMET_WRITE (called by the CometEvent.ready() method without the need for an explicit callback from the servlet - ready() knows the result of the last write, and the servlet is supposed to not attempt any further writing if ready() returns false) - no additional features, like verification of caller threads (which I consider useless) - change to o.a.comet package I hope I did not forget anything. Certain items could still be changed (like the package name, or the name of the ready() method - the problem with that one is that it does not particularly imply writing, but OTOH the old canWrite and isWriteable names were quite inelegant). I don't want to block development forever, so if this proposal is refused using an outright veto like the previous version of the proposal, I will not block competing proposals using a veto of my own. However, I will most likely stop participating in trunk development (which is not the end of the world since I've been working on Tomcat for pretty much forever, so it could be a good time to move on). nice ultimatum :) let me go through both the proposals and look it over. after that I suggest we simply have/agree on a majority vote without vetoes to be able to move on properly. in the vote announcement, I think we should simply outline the differences so that folks understand that rather than getting confused in comet concepts. you and I can draft that outline together offline, Filip Comments ? / Votes ? Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Costin Manolache wrote: On 6/15/07, Filip Hanik - Dev Lists [EMAIL PROTECTED] wrote: correction, should read I can implement inputstream.read() to return 0 on both blocking and non blocking *Comet events*, (since sockets are always non blocking) Ok, my mistake - what I really meant to say is: I am only interested in Comet if it behaves similar with non-blocking sockets ( regardless of implementation ), i.e. read(), write() called inside a comet servlet will never block. well, then you are out of luck, cause that's not in the running right now. I earlier proposed that CometEvent would have two new methods nbRead(ByteBuffer) and nbWrite(ByteBuffer) that would do exactly that. That proposal was quickly shutdown since we thought that we should still be using the Servlet API. And you can't get NIO socket functionality using streams, readers or writers. From the description so far it seems the sandbox version has this property and the trunk has an option to make it blocking ( or so I understood from your comments ). I'm not familiar with the concept of 'non blocking *Comet events*' - never heard of an event that blocks or doesn't block before. If you mean that during a comet event read will be blocking ( based on config or whatever else ) - I think this is a bad design. That's how 6.0.13 is working, and I don't think its bad design. Take a look at the first example in http://people.apache.org/~fhanik/tomcat/aio.html#Example%20code%20snippets It's a real life example, and clearly shows how much easier it is to write code using blocking read/write methods. In the current proposal non blocking would also happen on the existing Servlet API making it a little harder to program, but we got there from deciding to stick with the Servlet API and not create any new read or write methods. Filip Costin No virus found in this incoming message. Checked by AVG Free Edition. Version: 7.5.472 / Virus Database: 269.9.0/852 - Release Date: 6/17/2007 8:23 AM - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote:
Ok, one use case scenario would be AJAX requests with asynchronous
responses on the server, I can see this be extremely common,
I've tried to add in comments to make the example clear, and I don't see
this example as stupid, as there are a lot more AJAX client frameworks
than Comet client frameworks.
So there are two things we are show casing
1. The ability to write asynchronously
2. The ability to pipeline HTTP requests (assumption: only dealing with
GET requests, no HTTP request body)
3. Difference between Comet non blocking and Comet blocking read and
writes (note, not related to sockets, API only)
What I'd like to point out is that this example, becomes easier with
blocking read/writes, so I've added that at the bottom for comparison
Let me know if I've made any mistakes here, so that we can work with a
correct example.
Non blocking read and writes
public class ExampleDelayedAjaxResponse implements CometProcessor {
...
public class DelayedResponder extends Thread {
public void run() {
...
Client[] clients = getClients();
for (int i=0; iclients.length; i++ ) {
CometEvent event = client.getEvent();
byte[] data = getDelayedResponse(event);
if (data!=null) {
if (event.isWriteable()) {
event.getHttpServletResponse().getOutputStream().write(data);
if (event.isWriteable()) { //did we write it all?
event.close(); //close the event, in anticipation of the
next request
event.register(OP_CALLBACK); //triggers an END event
} else { //we didn't write it all, trigger a WRITE event when
we are done with the write event.register(OP_WRITE);
}
} else {
//we are not able to write, let us know when we can
event.register(OP_WRITE);
How can you not be able to write, since you didn't write anything at all
yet for this request.
}
//remove the client from the async thread
//since we have received the data for this client.
clients.remove(event);
}
}
...
}
}
...
public void event(CometEvent event) throws IOException, ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
//configure non blocking
event.configureBlocking(false);
//deregister for READ since we want to enable pipe lining on the
connection //for the next HTTP request
event.unregister(OP_READ);
//add the client to the list
clients.add(event);
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
assert(this should never happen);
} if ( event.getEventType() == CometEvent.EventType.WRITE ) {
//unregister from the write event
event.unregister(OP_WRITE);
//we can now write
byte[] data = getDelayedResponse(event); if ( data !=
null ) {
event.getHttpServletResponse().getOutputStream().write(data);
}
if( data==null || event.isWriteable() ) {
event.close();
}
} if ( event.getEventType() == CometEvent.EventType.END ) {
clients.remove(event); } else if (...) {
...
}
...
}
}
Blocking read and writes
public class ExampleDelayedAjaxResponse implements CometProcessor {
...
public class DelayedResponder extends Thread {
public void run() {
...
Client[] clients = getClients();
for (int i=0; iclients.length; i++ ) {
CometEvent event = client.getEvent();
byte[] data = getDelayedResponse(event);
if (data!=null) {
//register for a write, better do that on a thread pool thread
//since write is blocking
event.register(OP_WRITE);
//remove the client from the async thread
clients.remove(event);
}
}
...
}
}
...
public void event(CometEvent event) throws IOException, ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
//configure non blocking
event.configureBlocking(false);
//deregister for READ since we want to enable pipe lining on the
connection //for the next HTTP request
No. The situation where the end of the entity body is reached is
detected very easily, and handled using a sleep (the 6.0 code is very
close from that already).
event.unregister(OP_READ);
//add the client to the list
clients.add(event);
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
assert(this should never happen);
} if ( event.getEventType() == CometEvent.EventType.WRITE ) {
//unregister from the write event
event.unregister(OP_WRITE);
//we can now write
byte[] data = getDelayedResponse(event); //note we don't
have to check for null data here
Re: Proposed simplification of CometEvent
On 6/15/07, Filip Hanik - Dev Lists [EMAIL PROTECTED] wrote: correction, should read I can implement inputstream.read() to return 0 on both blocking and non blocking *Comet events*, (since sockets are always non blocking) Ok, my mistake - what I really meant to say is: I am only interested in Comet if it behaves similar with non-blocking sockets ( regardless of implementation ), i.e. read(), write() called inside a comet servlet will never block. From the description so far it seems the sandbox version has this property and the trunk has an option to make it blocking ( or so I understood from your comments ). I'm not familiar with the concept of 'non blocking *Comet events*' - never heard of an event that blocks or doesn't block before. If you mean that during a comet event read will be blocking ( based on config or whatever else ) - I think this is a bad design. Costin
Re: Proposed simplification of CometEvent
Costin Manolache wrote: On 6/14/07, Filip Hanik - Dev Lists [EMAIL PROTECTED] 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. you're hung up on the servlet API, there is a *huge* difference. :) If you stick with the servlet API, the thread and connection is a 1 to 1 mapping. Ie, if you want to do comet over a regular servlet, you can't have more connections than max threads. With comet, this limitation goes away. And this limitation or the removal thereof has nothing to do with blocking or non blocking, With comet, you get an event when you need to read or can write, when you actually do the read/write the IO logic of it is blocking, but to you as a developer it is almost transparent. since it would only block in extremely rare cases, even with blocking IO. Please note, that neither Remy nor I have yet really talked about non blocking reads, so you might think sandbox is non blocking, well it is not. It is buffering, but not non blocking. a true non blocking read, would require a rewrite of all the buffer filters to keep state between read invocations. We get away from this on the write, cause we can store the last write and keep it at the bottom layer. this is not a true non blocking write, as this implementation prevents any further write attempts by throwing an IOException or IllegalStateException. 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
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote: Please note, that neither Remy nor I have yet really talked about non blocking reads, so you might think sandbox is non blocking, well it is not. It is buffering, but not non blocking. a true non blocking read, would require a rewrite of all the buffer filters to keep state between read invocations. It is non blocking, as the low level read is always non blocking (in the sandbox design). Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote:
public class ExampleCometStockStreamer implements CometProcessor {
...
public class StockUpdater extends Thread {
public void run() {
...
StockUpdates[] updates = fetchUpdates();
Client[] clients = getClients(updates);
for (int i=0; iclients.length; i++ ) {
StockUpdates[] clientList = getClientUpdates(client,updates);
client.setAndMergeNextUpdates(clientList);
client.getEvent().callback();
}
...
}
}
now its starting to look funny, In the trunk version of the example, I'm
interested if the socket buffer is ready to receive data,
but the sandbox version of it simply doesn't care, it just calls for a
tomcat thread.
sandbox:
client.getEvent().callback(); - no guarantee for writeability
trunk:
client.getEvent().register(OP_WRITE) - event fires when network buffer
is ready to receive data.
The only thing funny here is your example. The writes done in the
callback event are blocking if needed. For an equivalent of your code,
you could do
if (isWriteable()) { callback(); }
etc
All of these examples are not very good, I think :( In most of these,
you should not be using Comet I think: CPU usage will skyrocket,
killing off the benefits over regular Servlets with blocking IO.
Of course the examples aren't very real world'ish, the point was to
simply show the differences in the API.
However, I think we have uncovered a larger problem than what API do we
pick right now. The concept of Comet is very clear to the two of us.
I know how your impl would work, and I'm pretty sure you know how mine
would work. But beyond that, the basics around Comet is still not well
explained nor understood.
While we are debating method names and implementation details, our
thread is being misunderstood.
Actually, I believe many people understand event based IO, so I don't
see the problem. I am not debating an implementation detail or a method
name (I would like to get good method names ideas, actually), I am
arguing against your API design, which does not make sense to me.
I think starting to work on the Bayeux impl, or some other examples,
will shine some more light on this.
Great, I will be able to port this impl to a reasonable API :)
Rémy
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Remy Maucherat wrote: Filip Hanik - Dev Lists wrote: Please note, that neither Remy nor I have yet really talked about non blocking reads, so you might think sandbox is non blocking, well it is not. It is buffering, but not non blocking. a true non blocking read, would require a rewrite of all the buffer filters to keep state between read invocations. It is non blocking, as the low level read is always non blocking (in the sandbox design). so is the trunk impl, if you consider that non blocking. but if you use request.getInputStream().read() - that would require all the filters to be modified to keep the state of the last read, so that request.getInputStream().read() can indeed return 0. in trunk, I found a way to get away with this, I did the following on the isReadable() isReadable() == available()0 || InternalNioInputBuffer.hasData()==true || InternalNioInputBuffer.read()0 where the last InternalNioInputBuffer.read() is non blocking. are you of a different understanding that the filters would not need any modification to make request.getInputStream().read() non blocking? Filip - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Ok, so let me double check: the low level socket in sandbox is allways read after a poll() / select(), and never with a blocking read() ? After the poll(): the Comet servlet, in a tomcat thread will be able to call read() in the input stream, and that will return data or 0 if no more data is available ( or - call isReadable() and then read(), or whatever else - but the socket will not be put in blocking mode ) ? If this is true for sandbox - big +1. Costin On 6/15/07, Remy Maucherat [EMAIL PROTECTED] wrote: Filip Hanik - Dev Lists wrote: Please note, that neither Remy nor I have yet really talked about non blocking reads, so you might think sandbox is non blocking, well it is not. It is buffering, but not non blocking. a true non blocking read, would require a rewrite of all the buffer filters to keep state between read invocations. It is non blocking, as the low level read is always non blocking (in the sandbox design). Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Costin Manolache wrote: Ok, so let me double check: the low level socket in sandbox is allways read after a poll() / select(), and never with a blocking read() ? huh? :) After the poll(): the Comet servlet, in a tomcat thread will be able to call read() in the input stream, and that will return data or 0 if no more data is available ( or - call isReadable() and then read(), or whatever else - but the socket will not be put in blocking mode ) ? and some more huh :) it is an implementation detail if the socket is blocking or non blocking, and has nothing to do with read() returning 0 or not. now we are talking about the implementation, not the API. I can implement inputstream.read() to return 0 on both blocking and non blocking sockets. btw, for the NIO connector, we only have non blocking sockets, the API is an abstraction from the socket layer, If this is true for sandbox - big +1. not sure how the above relates to sandbox or trunk, since they use the same sockets the same way. Filip Costin On 6/15/07, Remy Maucherat [EMAIL PROTECTED] wrote: Filip Hanik - Dev Lists wrote: Please note, that neither Remy nor I have yet really talked about non blocking reads, so you might think sandbox is non blocking, well it is not. It is buffering, but not non blocking. a true non blocking read, would require a rewrite of all the buffer filters to keep state between read invocations. It is non blocking, as the low level read is always non blocking (in the sandbox design). Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
since we are getting pretty far off track here, I've started a Wiki page on what comet is and isn't. http://wiki.apache.org/tomcat/WhatIsComet It will be helpful in the future, or we can assimilate this into our docs eventually, but for now, I think it is a good drawing board to clear out things as we agree on them Filip Filip Hanik - Dev Lists wrote: Costin Manolache wrote: Ok, so let me double check: the low level socket in sandbox is allways read after a poll() / select(), and never with a blocking read() ? huh? :) After the poll(): the Comet servlet, in a tomcat thread will be able to call read() in the input stream, and that will return data or 0 if no more data is available ( or - call isReadable() and then read(), or whatever else - but the socket will not be put in blocking mode ) ? and some more huh :) it is an implementation detail if the socket is blocking or non blocking, and has nothing to do with read() returning 0 or not. now we are talking about the implementation, not the API. I can implement inputstream.read() to return 0 on both blocking and non blocking sockets. btw, for the NIO connector, we only have non blocking sockets, the API is an abstraction from the socket layer, If this is true for sandbox - big +1. not sure how the above relates to sandbox or trunk, since they use the same sockets the same way. Filip Costin On 6/15/07, Remy Maucherat [EMAIL PROTECTED] wrote: Filip Hanik - Dev Lists wrote: Please note, that neither Remy nor I have yet really talked about non blocking reads, so you might think sandbox is non blocking, well it is not. It is buffering, but not non blocking. a true non blocking read, would require a rewrite of all the buffer filters to keep state between read invocations. It is non blocking, as the low level read is always non blocking (in the sandbox design). Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Ok, one use case scenario would be AJAX requests with asynchronous
responses on the server, I can see this be extremely common,
I've tried to add in comments to make the example clear, and I don't see
this example as stupid, as there are a lot more AJAX client frameworks
than Comet client frameworks.
So there are two things we are show casing
1. The ability to write asynchronously
2. The ability to pipeline HTTP requests (assumption: only dealing with
GET requests, no HTTP request body)
3. Difference between Comet non blocking and Comet blocking read and
writes (note, not related to sockets, API only)
What I'd like to point out is that this example, becomes easier with
blocking read/writes, so I've added that at the bottom for comparison
Let me know if I've made any mistakes here, so that we can work with a
correct example.
Non blocking read and writes
public class ExampleDelayedAjaxResponse implements CometProcessor {
...
public class DelayedResponder extends Thread {
public void run() {
...
Client[] clients = getClients();
for (int i=0; iclients.length; i++ ) {
CometEvent event = client.getEvent();
byte[] data = getDelayedResponse(event);
if (data!=null) {
if (event.isWriteable()) {
event.getHttpServletResponse().getOutputStream().write(data);
if (event.isWriteable()) { //did we write it all?
event.close(); //close the event, in anticipation of the next
request
event.register(OP_CALLBACK); //triggers an END event
} else { //we didn't write it all, trigger a WRITE event when we are done with the write
event.register(OP_WRITE);
}
} else {
//we are not able to write, let us know when we can
event.register(OP_WRITE);
}
//remove the client from the async thread
//since we have received the data for this client.
clients.remove(event);
}
}
...
}
}
...
public void event(CometEvent event) throws IOException, ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
//configure non blocking
event.configureBlocking(false);
//deregister for READ since we want to enable pipe lining on the connection
//for the next HTTP request
event.unregister(OP_READ);
//add the client to the list
clients.add(event);
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
assert(this should never happen);
} if ( event.getEventType() == CometEvent.EventType.WRITE ) {
//unregister from the write event
event.unregister(OP_WRITE);
//we can now write
byte[] data = getDelayedResponse(event);
if ( data != null ) {
event.getHttpServletResponse().getOutputStream().write(data);
}
if( data==null || event.isWriteable() ) {
event.close();
}
} if ( event.getEventType() == CometEvent.EventType.END ) {
clients.remove(event);
} else if (...) {
...
}
...
}
}
Blocking read and writes
public class ExampleDelayedAjaxResponse implements CometProcessor {
...
public class DelayedResponder extends Thread {
public void run() {
...
Client[] clients = getClients();
for (int i=0; iclients.length; i++ ) {
CometEvent event = client.getEvent();
byte[] data = getDelayedResponse(event);
if (data!=null) {
//register for a write, better do that on a thread pool thread
//since write is blocking
event.register(OP_WRITE);
//remove the client from the async thread
clients.remove(event);
}
}
...
}
}
...
public void event(CometEvent event) throws IOException, ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
//configure non blocking
event.configureBlocking(false);
//deregister for READ since we want to enable pipe lining on the connection
//for the next HTTP request
event.unregister(OP_READ);
//add the client to the list
clients.add(event);
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
assert(this should never happen);
} if ( event.getEventType() == CometEvent.EventType.WRITE ) {
//unregister from the write event
event.unregister(OP_WRITE);
//we can now write
byte[] data = getDelayedResponse(event);
//note we don't have to check for null data here
event.getHttpServletResponse().getOutputStream().write(data);
event.close();
} if ( event.getEventType() == CometEvent.EventType.END ) {
clients.remove(event);
} else if (...) {
...
}
...
}
}
I think this makes a good use case of where blocking makes life easier,
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote: Costin Manolache wrote: Ok, so let me double check: the low level socket in sandbox is allways read after a poll() / select(), and never with a blocking read() ? huh? :) After the poll(): the Comet servlet, in a tomcat thread will be able to call read() in the input stream, and that will return data or 0 if no more data is available ( or - call isReadable() and then read(), or whatever else - but the socket will not be put in blocking mode ) ? and some more huh :) it is an implementation detail if the socket is blocking or non blocking, and has nothing to do with read() returning 0 or not. now we are talking about the implementation, not the API. I can implement inputstream.read() to return 0 on both blocking and non blocking sockets. correction, should read I can implement inputstream.read() to return 0 on both blocking and non blocking *Comet events*, (since sockets are always non blocking) btw, for the NIO connector, we only have non blocking sockets, the API is an abstraction from the socket layer, If this is true for sandbox - big +1. not sure how the above relates to sandbox or trunk, since they use the same sockets the same way. Filip Costin On 6/15/07, Remy Maucherat [EMAIL PROTECTED] wrote: Filip Hanik - Dev Lists wrote: Please note, that neither Remy nor I have yet really talked about non blocking reads, so you might think sandbox is non blocking, well it is not. It is buffering, but not non blocking. a true non blocking read, would require a rewrite of all the buffer filters to keep state between read invocations. It is non blocking, as the low level read is always non blocking (in the sandbox design). Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] 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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote:
here we go, some examples
http://people.apache.org/~fhanik/tomcat/aio.html#Example%20code%20snippets
and the entire document has been updated to reflect most changes
http://people.apache.org/~fhanik/tomcat/aio.html
Here is an alternative version of the examples using the sandbox API to
give people an idea, along with some comments. No renaming of
sleep/callback, or others at this time. Hopefully, I did not make too
many mistakes.
First example, which can enter a busy loop if all events start returning
false for isWriteable (which is not very likely, of course):
public class ExampleCometStockStreamer implements CometProcessor {
...
public class StockUpdater extends Thread {
public void run() {
...
StockUpdates[] updates = fetchUpdates();
Client[] clients = getClients(updates);
for (int i=0; iclients.length; i++ ) {
CometEvent event = client.getEvent();
StockUpdates[] clientList = getClientUpdates(client,updates);
client.setAndMergeNextUpdates(clientList);
if (event.isWriteable()) {
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
}
}
...
}
}
...
public void event(CometEvent event) throws IOException,
ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
String clientId = readClientInfo(event,stocks);
clients.add(clientId, event, stocks);
} if ( event.getEventType() == CometEvent.EventType.WRITE ) {
//we can now write
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
} else if (...) {
...
}
...
}
}
What this example should be doing is remove the client from the list
when isWriteable returns false, and add it back when it gets a write event.
I can translate the second example, but it could lead to an abusive
poller use and number of events (all writes are also done synchronously
with blocking IO, which never makes sense to me).
public class ExampleCometStockStreamer implements CometProcessor {
...
public class StockUpdater extends Thread {
public void run() {
...
StockUpdates[] updates = fetchUpdates();
Client[] clients = getClients(updates);
for (int i=0; iclients.length; i++ ) {
StockUpdates[] clientList = getClientUpdates(client,updates);
client.setAndMergeNextUpdates(clientList);
client.getEvent().callback();
}
...
}
}
...
public void event(CometEvent event) throws IOException,
ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
//configure blocking
event.configureBlocking(true);
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
String clientId = readClientInfo(event,stocks);
clients.add(clientId, event, stocks);
} if ( event.getEventType() == CometEvent.EventType.CALLBACK ) {
Client client = clients.get(event);
//we can now write
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
} else if (...) {
...
}
...
}
}
I think the third example is wrong: there's no reason for isWriteable or
isReadable to change its result unless they trigger a large amount of
logic and some IO operations. I thought you said it was wrong ;) Also,
it will be very vulnerable to busy loops. I can translate it by wrapping
the content of for (int j=0; jclients.size(); j++) { into a try/catch,
and removing the calls to isWriteable (which only introduce useless
events, and may cause additional busy loops).
Straight translation (since isWriteable will trigger a write event which
will flush, it will work, but busy loops are pretty much certain; it
also assumes things about the data to read):
public class ExampleAllReadThenWriteComet implements CometProcessor {
...
public class AllWriterThread extends Thread {
byte[] dataChunks = ...;
public void run() {
...
for (int i=0; idataChunks.length; i++ ) {
for (int j=0; jclients.size(); j++) {
boolean done = false;
while (!done) {
//first read the first request
//but only if our previous write was completed
if ( clients[j].getEvent().isWriteable()
clients[j].getEvent().isReadable() ) {
done = readClientData(clients[j]); //returns true if all
data has been received for a request
}
}
done = false;
while (!done) {
//write the response
Re: Proposed simplification of CometEvent
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 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. 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. - 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. i.e. SingelThreaded or reentrant ? Multiple
Re: Proposed simplification of CometEvent
Remy Maucherat wrote:
Filip Hanik - Dev Lists wrote:
here we go, some examples
http://people.apache.org/~fhanik/tomcat/aio.html#Example%20code%20snippets
and the entire document has been updated to reflect most changes
http://people.apache.org/~fhanik/tomcat/aio.html
Here is an alternative version of the examples using the sandbox API
to give people an idea, along with some comments. No renaming of
sleep/callback, or others at this time. Hopefully, I did not make too
many mistakes.
First example, which can enter a busy loop if all events start
returning false for isWriteable (which is not very likely, of course):
public class ExampleCometStockStreamer implements CometProcessor {
...
public class StockUpdater extends Thread {
public void run() {
...
StockUpdates[] updates = fetchUpdates();
Client[] clients = getClients(updates);
for (int i=0; iclients.length; i++ ) {
CometEvent event = client.getEvent();
StockUpdates[] clientList = getClientUpdates(client,updates);
client.setAndMergeNextUpdates(clientList);
if (event.isWriteable()) {
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
}
}
...
}
}
Yes, this difference here is one that I would vouch that the API would
be explicit, instead of implicit.
compare
if (event.isWriteable()) {
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
} else {
event.register(OP_WRITE);
}
with
if (event.isWriteable()) {
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
}
the implicit registration for a WRITE event is not made clear by the API
alone, its something you would have to discover.
and one could look for a use case, where a WRITE event wasnt desired.
...
public void event(CometEvent event) throws IOException,
ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
String clientId = readClientInfo(event,stocks);
clients.add(clientId, event, stocks);
} if ( event.getEventType() == CometEvent.EventType.WRITE ) {
//we can now write
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
} else if (...) {
...
}
...
}
}
What this example should be doing is remove the client from the list
when isWriteable returns false, and add it back when it gets a write
I can translate the second example, but it could lead to an abusive
poller use and number of events (all writes are also done
synchronously with blocking IO, which never makes sense to me).
public class ExampleCometStockStreamer implements CometProcessor {
...
public class StockUpdater extends Thread {
public void run() {
...
StockUpdates[] updates = fetchUpdates();
Client[] clients = getClients(updates);
for (int i=0; iclients.length; i++ ) {
StockUpdates[] clientList = getClientUpdates(client,updates);
client.setAndMergeNextUpdates(clientList);
client.getEvent().callback();
}
...
}
}
now its starting to look funny, In the trunk version of the example, I'm
interested if the socket buffer is ready to receive data,
but the sandbox version of it simply doesn't care, it just calls for a
tomcat thread.
sandbox:
client.getEvent().callback(); - no guarantee for writeability
trunk:
client.getEvent().register(OP_WRITE) - event fires when network
buffer is ready to receive data.
...
public void event(CometEvent event) throws IOException,
ServletException {
...
if ( event.getEventType() == CometEvent.EventType.BEGIN ) {
//configure blocking
event.configureBlocking(true);
} if ( event.getEventType() == CometEvent.EventType.READ ) {
//read client Id and stock list from client
//and add the event to our list
String clientId = readClientInfo(event,stocks);
clients.add(clientId, event, stocks);
} if ( event.getEventType() == CometEvent.EventType.CALLBACK ) {
Client client = clients.get(event);
//we can now write
byte[] data = getUpdateChunk(client.getNextUpdates());
event.getHttpServletResponse().getOutputStream().write(data);
} else if (...) {
...
}
...
}
}
I think the third example is wrong: there's no reason for isWriteable
or isReadable to change its result unless they trigger a large amount
of logic and some IO operations. I thought you said it was wrong ;)
Also, it will be very vulnerable to busy loops. I can translate it
Re: Proposed simplification of CometEvent
On 6/14/07, Filip Hanik - Dev Lists [EMAIL PROTECTED] 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.
Re: Proposed simplification of CometEvent
On Wed, 2007-06-13 at 12:04 +0200, Remy Maucherat wrote: Costin Manolache wrote: setTimeout() is not optional (the javadoc is out of date, sorry), there was an agreement on that earlier. Timeout sets the connection timeout, which is most likely useful even if there are events. It's quite possible sleep could use a timeout argument (I think calling setTimeout is more or less mandatory when using sleep, and OTOH calling setTimout is not as important otherwise). Ok - then sleep needs the extra argument, and will mean same as Thread.sleep(), but not-blocking ? This sleep is a non blocking call, and instructs the connection to do nothing until I wake you up (or a timeout occurs, of course). may be sleep is not the right name... Would suspend be a better name? Cheers Jean-Frederic - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
On 6/13/07, Filip Hanik - Dev Lists [EMAIL PROTECTED] wrote: Costin Manolache wrote: For a separate opinion: In the trunk version: - the '...' and array return seem strange and generate GC ( not a big issue those days, but still inconsistent with the rest of tomcat ) yes, its a new language feature, hence it wasn't available in previous JDKs or Tomcat. its a vararg, if we don't want it we can switch to arrays, it works the same way. Yes, I know it's a vararg and my concern is not that it is not available in all VMs, but that it creates temp arrays and GC, which is inconsistent with the rest of tomcat which is very careful with creating objects. Also - it's quite useless in this particular case, printf is a good use case but that doesn't mean it should be used everywhere. - the API seems a bit over-complex - for example, why setConfiguration(COMMET_BLOCKING) instead of setBlocking() ? I agree that it less likely to break implementations by adding to the enom instead of adding methods to the interface - not sure why it's not an abstract class in the first place. if we want to simplify it, we would call it configureBlocking(boolean) - consistent with JDK APIs :) 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. - 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(). - 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() 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. 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 [EMAIL PROTECTED] 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
Re: Proposed simplification of CometEvent
On 6/13/07, Remy Maucherat [EMAIL PROTECTED] wrote: Costin Manolache wrote: setTimeout() is not optional (the javadoc is out of date, sorry), there was an agreement on that earlier. Timeout sets the connection timeout, which is most likely useful even if there are events. It's quite possible sleep could use a timeout argument (I think calling setTimeout is more or less mandatory when using sleep, and OTOH calling setTimout is not as important otherwise). Ok - then sleep needs the extra argument, and will mean same as Thread.sleep(), but not-blocking ? This sleep is a non blocking call, and instructs the connection to do nothing until I wake you up (or a timeout occurs, of course). Please add this to the javadoc :-) And maybe call it 'setTimerEvent(timeMillis)' or something like that - to avoid confusion with Thread.sleep() which is blocking the thread. IMHO it would be very nice to also have a blocking waitForEvent(timeMillis) - as an alternative to adding blocking mode as in the trunk. Although the read event indicates there's data to read, isReadable indicates if it is possible to continue reading. My understanding was that the InputStream in request is used for actual reading - and available() could do the same thing. What is the difference then between the 2 ? isReadable is the same as is.available (and reader.ready). It's there for a bit of symmetry after adding isWriteable, which may or may not be useful, and I didn't care about it. Why not call it available() and return the same as the servlet method ? I think it's ok to have 'shortcuts' in the CometEvent - even read()/write() methods that would expose more efficient ways to send/get data than the InputStream. isWriteable indicates if the last write operation managed to write more than 0 bytes. If the last write wrote 0, then isWriteable will return false, so the servlet knows it should stop writing on this connection for now (since it cannot accept any output at the moment). Later on, the servlet will receive a write event, and can resume writing. I'm still a bit confused about this - my understanding was that write event means the previous buffers were written, and you can write more. There are some buffers on the OS side as well as buffers on the connector side. Yes, it means that: the servlet gets a write event, which means it can start writing again. What do you mean by 'managed to write more than 0 bytes' - the write in the OutputStream can go to some of the buffers, or to the client. I assume you don't mean the client ( due to TCP delays ). I was talking about the actual write on the socket (in APR, it's done in InternalAprOutputBuffer.flushBuffer), which may return 0. At the servlet layer, as per the contract of the OutputStream API, is must write everything or fail, and this isn't going to change. Where 'write everything' means 'maybe buffer some of it at OS or java layer'. Could you add more javadocs on how much can you expect to write if isWriteable() returns true ? Since the OutputStream method doesn't support partial writes - I suppose this would help more than isWriteable. - If doing synchronous writes inside some event (either a read or callback event, most likely), then both blocking and non blocking mode make sense. Some servlets may prefer to use blocking mode although it could be holding a thread for a while (for example if the idea is only maybe a 'waitForEvent()' method to allow a servlet to block if he wants to ? Or is sleep() supposed to do this - I'm not sure from the comments if sleep() will block or just triger an event when the interval expires ? Sleep will by itself only trigger timeout events (the method call itself will return immediately). The servlet can use the callback method to trigger an event before the timeout occurs. You mean sort of 'notify()' - i.e. someone calls callback() and will trigger the servlet to be executed, interrupting any sleep or wait ? Originally, this was indeed supposed to be called notify, but it's not possible due to Object.notify :( If you have another name to suggest to replace callback ... Well - servletNotify() or servletWakeup() or sendCometEvent() ? Costin In general ( both versions): - it would be great to move it from o.a.catalina to org.apache.comet It's a possibility. I think more comments and examples ( and maybe better names ) would be great. I think there's going to be some code using Comet soon. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
here we go, some examples http://people.apache.org/~fhanik/tomcat/aio.html#Example%20code%20snippets and the entire document has been updated to reflect most changes http://people.apache.org/~fhanik/tomcat/aio.html Filip Filip Hanik - Dev Lists wrote: I'll work on some examples to illustrate what I mean, It will be much clearer Filip Remy Maucherat 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.isWriteable all 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
Re: Proposed simplification of CometEvent
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. I think it's very reasonable to add a blocking waitForEvent() to allow servlets have a simpler ( but less efficient ) implementation. 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. - 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. - 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, 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 ). 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. All you need to poll data async is waitForEvent(), or select() if you like to emulate nio. 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 ?) 5. The current READ event in CoyoteAdapter ends up performing a blocking
Re: Proposed simplification of CometEvent
Costin Manolache wrote: 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. setTimeout() is not optional (the javadoc is out of date, sorry), there was an agreement on that earlier. Timeout sets the connection timeout, which is most likely useful even if there are events. It's quite possible sleep could use a timeout argument (I think calling setTimeout is more or less mandatory when using sleep, and OTOH calling setTimout is not as important otherwise). - 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. Although the read event indicates there's data to read, isReadable indicates if it is possible to continue reading. isWriteable indicates if the last write operation managed to write more than 0 bytes. If the last write wrote 0, then isWriteable will return false, so the servlet knows it should stop writing on this connection for now (since it cannot accept any output at the moment). Later on, the servlet will receive a write event, and can resume writing. It is possible to add a blockingMode flag, but I've looked into it a bit, and found out it is not really needed: - If doing asynchronous writes using a background thread, non blocking helps (if somehow a connection blocks, no other connection will get any data until the write completes or times out). I don't see how blocking mode could be desired in this case, and the servlet should always be using isWriteable. - If doing synchronous writes inside some event (either a read or callback event, most likely), then both blocking and non blocking mode make sense. Some servlets may prefer to use blocking mode although it could be holding a thread for a while (for example if the idea is only to delay the response), and some others could choose to be more scalable. Since it is possible to detect if the write is done inside a Tomcat thread, and since a servlet which wishes to handle incomplete writes would be using isWriteable, it is possible to know when we're in the situation where the servlet would prefer blocking. In that case, the algorithm in the write method of the connector would be: check if the last write returned 0, set blocking mode, flush leftover bytes and send the additional bytes which are being written, set non blocking mode. It's a trick (of course, it is possible to do the same thing for read), and the main drawback is that there are extra operations, but they would occur only if needed. - 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 ? Yes, sleep is meant mostly to handle the reply-later design easily. The servlet calls sleep(), and is supposed to call callback() (asnychronously, most likely) as a result of something. The connection will then wake and the servlet will get a callback event. If callback is not called, the connection will timeout and the servlet will get its timeout event (according to the connector timeout or the one that has been configured for the connection). In general ( both versions): - it would be great to move it from o.a.catalina to org.apache.comet It's a possibility. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
On 6/12/07, Remy Maucherat [EMAIL PROTECTED] wrote: Costin Manolache wrote: 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. setTimeout() is not optional (the javadoc is out of date, sorry), there was an agreement on that earlier. Timeout sets the connection timeout, which is most likely useful even if there are events. It's quite possible sleep could use a timeout argument (I think calling setTimeout is more or less mandatory when using sleep, and OTOH calling setTimout is not as important otherwise). Ok - then sleep needs the extra argument, and will mean same as Thread.sleep(), but not-blocking ? - 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. Although the read event indicates there's data to read, isReadable indicates if it is possible to continue reading. My understanding was that the InputStream in request is used for actual reading - and available() could do the same thing. What is the difference then between the 2 ? isWriteable indicates if the last write operation managed to write more than 0 bytes. If the last write wrote 0, then isWriteable will return false, so the servlet knows it should stop writing on this connection for now (since it cannot accept any output at the moment). Later on, the servlet will receive a write event, and can resume writing. I'm still a bit confused about this - my understanding was that write event means the previous buffers were written, and you can write more. There are some buffers on the OS side as well as buffers on the connector side. What do you mean by 'managed to write more than 0 bytes' - the write in the OutputStream can go to some of the buffers, or to the client. I assume you don't mean the client ( due to TCP delays ). It is possible to add a blockingMode flag, but I've looked into it a bit, and found out it is not really needed: I agree, it is cleaner to not mix them. - If doing synchronous writes inside some event (either a read or callback event, most likely), then both blocking and non blocking mode make sense. Some servlets may prefer to use blocking mode although it could be holding a thread for a while (for example if the idea is only maybe a 'waitForEvent()' method to allow a servlet to block if he wants to ? Or is sleep() supposed to do this - I'm not sure from the comments if sleep() will block or just triger an event when the interval expires ? - 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 ? Yes, sleep is meant mostly to handle the reply-later design easily. The servlet calls sleep(), and is supposed to call callback() (asnychronously, most likely) as a result of something. The connection will then wake and the servlet will get a callback event. If callback is not called, the connection will timeout and the servlet will get its timeout event (according to the connector timeout or the one that has been configured for the connection). You mean sort of 'notify()' - i.e. someone calls callback() and will trigger the servlet to be executed, interrupting any sleep or wait ? In general ( both versions): - it would be great to move it from o.a.catalina to org.apache.comet It's a possibility. I think more comments and examples ( and maybe better names ) would be great. In any case - the sandbox version seems better as an API, I really don't like the arrays and setConfig in the trunk, and certainly don't like having 2 variants. Costin
Re: Proposed simplification of CometEvent
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. 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. 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. 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. 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.isWriteable all 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 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. 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. Where does that leave us, well: a) We are almost in sync on the implementation side of it 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 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? Filip Remy Maucherat wrote: Filip Hanik - Dev Lists wrote: I think I did present understandable explanations, so if you don't bother reading (as usual ...), I suppose I should stop wasting my time. throwing your hands in the air and saying you're not going to waste your time, is in your personal interest only and you are of course welcome to do anything you wish. Throwing in derogatory statements like the one above, wont help you achieve your goal, it will only risk you being taken less seriously, and it doesn't benefit you nor the community. You are writing self conflicting statements in the previous emails, which makes discussion difficult. Here it is now, where we (magically) agree on core definitions: async - using more than one thread to do stuff and that is what you are proposing, to dispatch the write to the poller thread. non blocking - a method call which returns immediately without, well, blocking yes And in the previous email, it was: Actually, this is very simple to implement. Read is obvious. For write, the algorithm for the flushBuffer method would be very similar to what it is now, but if a write returns 0, leftover bytes would be put in a ByteChunk. isWriteable, if called, will return false, and place the socket in the poller with write notifications. that's
Re: Proposed simplification of CometEvent
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.isWriteable all 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
Re: Proposed simplification of CometEvent
I'll work on some examples to illustrate what I mean, It will be much clearer Filip Remy Maucherat 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.isWriteable all 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,
Re: Proposed simplification of CometEvent
For a separate opinion: In the trunk version: - the '...' and array return seem strange and generate GC ( not a big issue those days, but still inconsistent with the rest of tomcat ) - the API seems a bit over-complex - for example, why setConfiguration(COMMET_BLOCKING) instead of setBlocking() ? I agree that it less likely to break implementations by adding to the enom instead of adding methods to the interface - not sure why it's not an abstract class in the first place. - 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. - see bellow - I don't think I understand the benefits of mixing blocking and non-blocking in this interface, it is quite confusing. 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 [EMAIL PROTECTED] 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
Re: Proposed simplification of CometEvent
If you don't care about this stuff, but still read the first line, do our community a favor and get involved :) The API's are pretty much identical, Remy: https://svn.apache.org/viewvc/tomcat/sandbox/comet/java/org/apache/catalina/CometEvent.java?view=markup Filip: https://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/CometEvent.java?view=markup There are a few differences isReadable() - Remy - same functionality as ServletInputStream.available() - Filip - ServletInputStream.available() || InternalNioInputBuffer.hasData || InternalNioInputBuffer.nonBlockRead().hasData ie, is readable in my API means that there is some data from the bottom layer of the API. Doesn't mean that it would actually result in data to a ServletInputStream.read(), but most cases it would. The benefit of this method is that it actually does check the socket for data in a non blocking way if the first two conditions have not been met. API has been implemented in trunk. so isAvailable() is different, and both methods can be used for different purposes isWriteable() This functions the same way in both APIs. After playing with the API this function is useless unless you have true non blocking writes. Why? Well, the function will always return true, unless there is another thread writing, but write is not thread safe, so another thread should never attempt to write. And in a blocking way, isWriteable()-true : write(blocking) : isWriteable()-true, always. However, in the blocking API, there is a true way to find out if we can write or not, and that is by calling CometEvent.register(OP_WRITE), this will actually check with the poller to see if the network buffer is ready to receive more data and dispatch a worker thread. callback() - Remy - one time callback from the container on a Tomcat worker thread - Filip - same functionality implemented, CometEvent.register(OP_CALLBACK) I definitely like the register/unregister for events more than I like the one time functionality of callback()/sleep(). OP_CALLBACK currently is implemented and working in trunk. Why do I like register more? Cause we can add new operations in the future without changing the API. This leads to more flexibility. Currently OP_CALLBACK stays registered until it is unregister-ed. If we wanted to achieve callback() functionality, we can add a OP_CALLBACK_ONCE operation or simply change the behavior of existing callback, without changing the API. it is also possible to retrieve the state of a comet connection by calling getRegisteredOps, something not available in the sandbox API. sleep() - Remy - deregister from the READ events - Filip - same functionality exists, CometEvent.unregister(OP_READ,OP_WRITE,OP_CALLBACK) or simply CometEvent.unregister(OP_READ) if nothing else is registered. Again, I think the trunk API is more flexible, and cleaner. sleep() is associated with Thread.sleep() in every programmers head. and doesn't really apply to an event based API. This is implemented in trunk and working. Conclusion, both APIs are almost identical. I believe the trunk API is more flexible and allows for additions of functionality in the future without changing the API. Remy's initial -1 to the trunk was based on the implementation, having to synchronize etc. All that has been simplified and removed. So therefor, the API's are almost identical, I would vote sandbox API - -1 - API is simple but not extensible, names are not intuitive as they are not event oriented. trunk API - +1 - API is equally simple, more geared towards event based API, more flexible, as I am sure new needs will need new operations and events The only thing not working in trunk at this moment is non blocking, but after doing the research, this is a major surgery to try to do within the existing buffer/filter API I'd rather delay such an attempt until we can reach consensus on the API and the simple stuff. Looks like we are gonna have to have a pancake flipping contest :) Filip Remy Maucherat wrote: Hi, I've been working on additions to CometEvent to implement the additional Comet functionality that was agreed upon before creating the trunk branch. Although not functional at the moment, I consider it to be developed enough from an algorithmic standpoint to be proposed and reviewed (it is also important to not continue an API fork for too long, since otherwise it would be harder to merge, so some resolution is needed). I went through a few revisions of the API, as more tweaks appeared possible. The idea was to allow the extra functionality without adding complexity or incompatible changes, since this is most likely going to be an interim API. The repository is at: https://svn.apache.org/repos/asf/tomcat/sandbox/comet It has the following changes over the 6.0.x branch: - additions of four methods in CometEvent (sleep and callback, and the two flags isRead(Write)able) - sleep delays request processing until either callback is
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote:
If you don't care about this stuff, but still read the first line, do
our community a favor and get involved :)
I'm not sure I understand this. It could be interpreted in a derogatory way.
The API's are pretty much identical,
Yes, they are pretty much functionally identical. The problem is that
the interpretation of the same thing you are proposing is
overengineered, convoluted and very GC unfriendly. The sync was a funny
choice which made me start working on a separate branch, since:
- you were ignoring my messages all of a sudden for some reason
- you started adding more and more stuff I disagreed with
It's ok to do that as part of a proposal (like I did), but it's not the
right way if you do it in a regular branch.
callback()
- Remy - one time callback from the container on a Tomcat worker thread
- Filip - same functionality implemented, CometEvent.register(OP_CALLBACK)
I definitely like the register/unregister for events more than I like
the one time functionality of callback()/sleep().
OP_CALLBACK currently is implemented and working in trunk.
Why do I like register more? Cause we can add new operations in the
future without changing the API. This leads to more flexibility.
Currently OP_CALLBACK stays registered until it is unregister-ed. If
we wanted to achieve callback() functionality, we can add a
OP_CALLBACK_ONCE operation or simply change the behavior of existing
callback, without changing the API.
it is also possible to retrieve the state of a comet connection by
calling getRegisteredOps, something not available in the sandbox API.
sleep()
- Remy - deregister from the READ events
- Filip - same functionality exists,
CometEvent.unregister(OP_READ,OP_WRITE,OP_CALLBACK) or simply
CometEvent.unregister(OP_READ) if nothing else is registered.
Again, I think the trunk API is more flexible, and cleaner. sleep() is
associated with Thread.sleep() in every programmers head. and doesn't
really apply to an event based API.
This is implemented in trunk and working.
The connector should only care about IO, which only knows read and write
(and error, but obviously this is a special case, and I don't see you
registering for that).
Conclusion, both APIs are almost identical. I believe the trunk API is
more flexible and allows for additions of functionality in the future
without changing the API.
Remy's initial -1 to the trunk was based on the implementation, having
to synchronize etc. All that has been simplified and removed.
I would like to understand what extensions are desirable or envisioned,
since this this sort of extensions will make the connector
implementation to skyrocket in terms of complexity. I think the
connector should only care about IO, so this means: read, write, and a
generic callback. The user will write a piece of code which can listen
to any sort of event, and use the callback method: the callback is the
extension point as far as I understand.
Another flaw IMO is that read and write do not behave the same as far as
their registration go.
The trunk API also has the following additional methods and constructs:
- public enum CometConfiguration {COMET_BLOCKING, COMET_NON_BLOCKING};
- public void configure(CometConfiguration... options) throws
IllegalStateException; (note: as I have discussed, it seems possible to
abstract away blocking configuration from the user)
- public enum CometOperation {OP_CALLBACK, OP_READ, OP_WRITE};
- public CometOperation[] getRegisteredOps();
- the need to ask for a write event (it is quite crucial, and I found
out it could easily be hidden away)
So, similarly, the configuration is extensible. I would like to
understand what additional configuration should be exposed to the user
(personally, I solved the configuration problem when I noticed blocking
/ non blocking configuration was pointless).
I also dislike the thread checks you are using.
The last reason I would like to have a really simple (yet fully
functional) API is to avoid having duplicate APIs. The Servlet API may
tackle the Comet topic, and knowing Sun APIs, you may get the API of
your dreams :D
So therefor, the API's are almost identical, I would vote
sandbox API - -1 - API is simple but not extensible, names are not
intuitive as they are not event oriented.
trunk API - +1 - API is equally simple, more geared towards event
based API, more flexible, as I am sure new needs will need new
operations and events
I think the APIs are quite different in form, yet expose similar
functionality (at the moment, I'm waiting for the OP_JMS ;) ). Since
it's a user visible API, it's an important issue regardless of what you
think. I have my own vote (hehe ;) ):
- sandbox - +1
- trunk - -1 (overenginneered, more complex, thread checks)
The only thing not working in trunk at this moment is non blocking, but
after doing the research, this is a major surgery to try to do within
the existing buffer/filter API
Re: Proposed simplification of CometEvent
Remy Maucherat wrote:
Filip Hanik - Dev Lists wrote:
If you don't care about this stuff, but still read the first line, do
our community a favor and get involved :)
I'm not sure I understand this. It could be interpreted in a
derogatory way.
if it is just you and I, then its your -1 against my -1 and we get nowhere.
simply trying to invite folks to be a part of the decision making.
The API's are pretty much identical,
Yes, they are pretty much functionally identical. The problem is that
the interpretation of the same thing you are proposing is
overengineered, convoluted and very GC unfriendly. The sync was a
funny choice which made me start working on a separate branch, since:
- you were ignoring my messages all of a sudden for some reason
no messages ignored. some messages were pure statements, and didn't
invite for comments.
- you started adding more and more stuff I disagreed with
to the API? (CometEvent.java) like what?
It's ok to do that as part of a proposal (like I did), but it's not
the right way if you do it in a regular branch.
callback()
- Remy - one time callback from the container on a Tomcat worker thread
- Filip - same functionality implemented,
CometEvent.register(OP_CALLBACK)
I definitely like the register/unregister for events more than I like
the one time functionality of callback()/sleep().
OP_CALLBACK currently is implemented and working in trunk.
Why do I like register more? Cause we can add new operations in the
future without changing the API. This leads to more flexibility.
Currently OP_CALLBACK stays registered until it is unregister-ed.
If we wanted to achieve callback() functionality, we can add a
OP_CALLBACK_ONCE operation or simply change the behavior of existing
callback, without changing the API.
it is also possible to retrieve the state of a comet connection by
calling getRegisteredOps, something not available in the sandbox API.
sleep()
- Remy - deregister from the READ events
- Filip - same functionality exists,
CometEvent.unregister(OP_READ,OP_WRITE,OP_CALLBACK) or simply
CometEvent.unregister(OP_READ) if nothing else is registered.
Again, I think the trunk API is more flexible, and cleaner. sleep()
is associated with Thread.sleep() in every programmers head. and
doesn't really apply to an event based API.
This is implemented in trunk and working.
The connector should only care about IO, which only knows read and
write (and error, but obviously this is a special case, and I don't
see you registering for that).
But the connector must care, if you do a CALLBACK, you must ensure that
you are not spawning a 2nd thread if a READ comes in on the socket.
otherwise, you are forcing the comet developers to synchronize,
something in today's API, is done in such a way that if you do stuff on
the Tomcat thread, you're safe.
Conclusion, both APIs are almost identical. I believe the trunk API
is more flexible and allows for additions of functionality in the
future without changing the API.
Remy's initial -1 to the trunk was based on the implementation,
having to synchronize etc. All that has been simplified and removed.
I would like to understand what extensions are desirable or
envisioned, since this this sort of extensions will make the connector
implementation to skyrocket in terms of complexity. I think the
connector should only care about IO, so this means: read, write, and a
generic callback. The user will write a piece of code which can listen
to any sort of event, and use the callback method: the callback is the
extension point as far as I understand.
Another flaw IMO is that read and write do not behave the same as far
as their registration go.
The trunk API also has the following additional methods and constructs:
- public enum CometConfiguration {COMET_BLOCKING, COMET_NON_BLOCKING};
- public void configure(CometConfiguration... options) throws
IllegalStateException; (note: as I have discussed, it seems possible
to abstract away blocking configuration from the user)
- public enum CometOperation {OP_CALLBACK, OP_READ, OP_WRITE};
- public CometOperation[] getRegisteredOps();
- the need to ask for a write event (it is quite crucial, and I found
out it could easily be hidden away)
There is no need to ask for a WRITE, you can write anytime,
asynchronously too. OP_WRITE is if you want to be sure that the network
buffer is ready to receive data.
otherwise, I already pointed out the flaws in isWriteable and that
isWriteable==true doesn't mean that it is writeable.
So, similarly, the configuration is extensible. I would like to
understand what additional configuration should be exposed to the user
(personally, I solved the configuration problem when I noticed
blocking / non blocking configuration was pointless).
configure() can be removed, or taken out for a later consideration, have
no problem with that.
I also dislike the thread checks you are using.
I can pull those out as well, since in my impl I able to
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote: In my personal opinion I am no longer sure a non blocking write is needed. Ok. async - using more than one thread to do stuff non blocking - a method call which returns immediately without, well, blocking I think I did present understandable explanations, so if you don't bother reading (as usual ...), I suppose I should stop wasting my time. I agree a consensus should have been reached *on month ago* :) we'd be in the same shoes we are today. I still think a pancake flipping contest would be the way to go. although I might consider stone,paper and scissors too. No problem with that, feel free to consider it any way you like. What I see in your response is that we actually agree on very little. I'll try in order (one last time, most likely): The connector should only care about IO, which only knows read and write (and error, but obviously this is a special case, and I don't see you registering for that). But the connector must care, if you do a CALLBACK, you must ensure that you are not spawning a 2nd thread if a READ comes in on the socket. otherwise, you are forcing the comet developers to synchronize, something in today's API, is done in such a way that if you do stuff on the Tomcat thread, you're safe. Well, not really, the callback goes all the way to the poller, using the same add lists, which allows ensuring that events are not sent in a concurrent way. Actually, this is very simple to implement. Read is obvious. For write, the algorithm for the flushBuffer method would be very similar to what it is now, but if a write returns 0, leftover bytes would be put in a ByteChunk. isWriteable, if called, will return false, and place the socket in the poller with write notifications. that's not non blocking, that's a async write, and I could ten times more easier implement it in an AbstractCometProcessor.java, then having to fiddle with pollers and everything else down the chain. non blocking is a different concept than async. I made an effort to explain in detail algorithms, and you come back with definitions I do not understand and vague ideas (I have no idea what would be implemented in AbstractCometProcessor). does not work either. isWriteable (when blocking) only is in effect for other threads, but write is not thread safe, so this method is useless then. and during an async write, you have the same issue, async writes still should only be done on one thread. ? Ok, so the plan is that there's only one thread attempting to write on one particular connection. The lifeclyle seems very simple to me: - check isWriteable - write stuff - repeat until isWriteable puts the socket in the add-write list for the poller, and returns false - wait until you get a write event before writing again on the connection Since this is non blocking IO, it is indeed possible and recommended to use a single thread for writing, thus maximizing scalability. If you advocate a design where the user would write using multiple threads, never sync anything, and hope it works, then I'd like to hear about a use case first :) Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Filip Hanik - Dev Lists wrote: I think I did present understandable explanations, so if you don't bother reading (as usual ...), I suppose I should stop wasting my time. throwing your hands in the air and saying you're not going to waste your time, is in your personal interest only and you are of course welcome to do anything you wish. Throwing in derogatory statements like the one above, wont help you achieve your goal, it will only risk you being taken less seriously, and it doesn't benefit you nor the community. You are writing self conflicting statements in the previous emails, which makes discussion difficult. Here it is now, where we (magically) agree on core definitions: async - using more than one thread to do stuff and that is what you are proposing, to dispatch the write to the poller thread. non blocking - a method call which returns immediately without, well, blocking yes And in the previous email, it was: Actually, this is very simple to implement. Read is obvious. For write, the algorithm for the flushBuffer method would be very similar to what it is now, but if a write returns 0, leftover bytes would be put in a ByteChunk. isWriteable, if called, will return false, and place the socket in the poller with write notifications. that's not non blocking, that's a async write, and I could ten times more easier implement it in an AbstractCometProcessor.java, then having to fiddle with pollers and everything else down the chain. non blocking is a different concept than async. (stars added to point out it was plainly obvious I was talking about non blocking write calls, I never talked at all about blocking IO - except to say they were most likely useless to the user, to which you said I am no longer sure a non blocking write is needed; at this point, I'd like you to explain to me how I could agree on anything you said) I made an effort to explain in detail algorithms, and you come back with definitions I do not understand and vague ideas (I have no idea what would be implemented in AbstractCometProcessor). ok, you suggest that a non block write is simply a dispatch of the writing to the poller in the event of the data written didn't fit in the network buffer, that sounds like multiple threads are indeed involved. No, I suggest the possibility, as is written in black and white, that rather than rely on a (un)register API, that the isWriteable call has the possibility to safely put the socket in the poller. This seems to be well understood now. So this is just a terminology misunderstanding. What you are explaining is similar to the SEND_FILE way of doing it, there is still a thread on our side involved in writing the data. I do understand your solution, and it would still work with my API. Not at all, the lifecycle I provided is very clear. Our thread is the event thread before it invokes the event. When a write event is sent, the following occurs: - go out of the poller with the write notification - dispatch to a worker thread - in the Http11(Apr/Nio)Processor, flush leftover data (if any) - if all data was flushed, invoke the adapter with a write event (isWriteable will now return true, but the servlet will wait before getting a write event to start dealing with writing on this connection), otherwise go back to the poller Writes are done in two threads, but never concurrently. It is also possible to not care and not do any flush before going into the servlet (the servlet first write would do the flush), but it seemed more efficient to do so. As I have described in a previous email, if the servlet does its writes during the processing of read and callback events, and the servlet is not using isWriteable, I think it would be fair if the write call blocks. it would be like this a) check isWriteable b) write stuff possibly goto f) c) check isWriteable, if true goto b) else goto d), d) event.register(OP_WRITE) e) wait for a WRITE event to come in f) done It's almost the same scenario, but the event.register(OP_WRITE) is done inside isWriteable. This allows simplifying a lot the API, since the only feature which remains is the ability to disable read events. in your API, you are forcing the WRITE event notification, I may just not be interested in it, and that is why I like the trunk API better, it If you are not interested in the write notification, then don't process it in the servlet, and use a tight loop on isWriteable, it will work without any particular problem (but would not be very sensible). Another way is to discard messages, which seems uncommon in web land. is event driven and it is the developer who controls what notifications are going to be coming or not. Since a WRITE event does take a thread out of the thread pool, that may not be desired. Looks like a better argument to me, since you don't imply that the solution I proposed is unsafe ;) In that case, I would like more concrete
Proposed simplification of CometEvent
Hi, I've been working on additions to CometEvent to implement the additional Comet functionality that was agreed upon before creating the trunk branch. Although not functional at the moment, I consider it to be developed enough from an algorithmic standpoint to be proposed and reviewed (it is also important to not continue an API fork for too long, since otherwise it would be harder to merge, so some resolution is needed). I went through a few revisions of the API, as more tweaks appeared possible. The idea was to allow the extra functionality without adding complexity or incompatible changes, since this is most likely going to be an interim API. The repository is at: https://svn.apache.org/repos/asf/tomcat/sandbox/comet It has the following changes over the 6.0.x branch: - additions of four methods in CometEvent (sleep and callback, and the two flags isRead(Write)able) - sleep delays request processing until either callback is called, or a timeout occurs - callback allows waking up requests which are asleep or waiting for an IO event - non blocking IO exclusively in Comet mode, since it now seems to me it is algorithmically equivalent to blocking IO; for read, it has been demonstrated previously; for write, it seems there should always be write events after a properly handled congestion (where the servlet uses isWriteable), since if the servlet does not, it will be best to simply fail-fast the connection using an IOException (the servlet would effectively only handle clients which are fast enough if using the Comet API like in Tomcat 6.0, which avoids a lot of issues) - isRead(Write)able indicate if data may be read or written (since this is non blocking IO, reading or writing would read or write 0 bytes); if it is false and reading or writing is done, an IO exception will be thrown - no additional data structures - new ActionCode: ACTION_COMET_TIMEOUT (which is cleanup), ACTION_COMET_CALLBACK (called by the callback() method), ACTION_COMET_SLEEP (called by the sleep() method), ACTION_COMET_WRITE (called by the CometEvent.isWriteable method without the need for an explicit callback from the servlet - isWriteable knows the result of the last write, and the servlet is supposed to not attempt any further writing if isWriteable returns false) - no additional features, like verification of caller threads (which I consider useless) I hope I did not forget anything. Comments ? / Votes ? Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Hi, On 6/7/07, Remy Maucherat [EMAIL PROTECTED] wrote: fail-fast the connection using an IOException (the servlet would effectively only handle clients which are fast enough if using the Comet API like in Tomcat 6.0, which avoids a lot of issues) This is an interesting assumption, but I think it's valid for this type of API. Comments ? / Votes ? I like the callback approach especially, and otherwise I see no major problems. Cool improvements. Yoav - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: Proposed simplification of CometEvent
Yoav Shapira wrote: Hi, On 6/7/07, Remy Maucherat [EMAIL PROTECTED] wrote: fail-fast the connection using an IOException (the servlet would effectively only handle clients which are fast enough if using the Comet API like in Tomcat 6.0, which avoids a lot of issues) This is an interesting assumption, but I think it's valid for this type of API. Yes, it's a bit an assumption. Since there's no blocking or non blocking ? question mark, it simplifies significantly the documentation of the API. The main issue is how to handle a write which would block, and would most probably cause problems when doing async writes (as others pointed out - Filip, I think), so in that case, if the Servlet chooses not to handle it using isWriteable - which makes sense in many cases -, I think the exception throwing is fine. The only situation where it could cause real problems is if the write is done inside another event (like in the common reply later pattern), and I think it would most likely work well enough due to the existence of buffers at the servlet layer. It's quite easy to add back blocking mode if needed, and tricks could be used rather than rely on direct configuration. For example, while I think adding code to detect what is called asynchronously for error checking purposes is useless, it could be possible to detect if the problematic write is sync or async, and adjust blocking mode accordingly at the connector level rather than throwing the exception. The idea is to make the API simpler overall than what it is in 6.0 (despite the addition of 3 methods and 2 event types, I think it would be simpler to work with due to the sleep/callback methods which allow doing common things very easily), and avoid introducing heavier constructs and method calls. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
