Mailing-List: contact dev-help@tomcat.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Tomcat Developers List" <dev@tomcat.apache.org>
Received-SPF: pass (herse.apache.org: local policy)
Message-ID: <466D1089.5060803@hanik.com>
Date: Mon, 11 Jun 2007 11:06:17 +0200
From: Filip Hanik - Dev Lists <devlists@hanik.com>
User-Agent: Thunderbird 1.5.0.12 (Windows/20070509)
MIME-Version: 1.0
To: Tomcat Developers List <dev@tomcat.apache.org>
Subject: Re: Proposed simplification of CometEvent
References: <46681613.4040903@apache.org> <466919E6.9040205@hanik.com>
 <46694438.2060809@apache.org> <46694E76.6010408@hanik.com>
 <466959C1.8060709@apache.org> <46697192.9000703@hanik.com>
 <46698730.5070400@apache.org>
In-Reply-To: <46698730.5070400@apache.org>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

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 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 demonstration that these 
> write events would cause a scalability problem. I am not convinced 
> since there is already some IO congestion at that point, and this sort 
> of event processing is even faster than doing a HelloWorld servlet 
> over a persistent connection - it's been shown Tomcat can do thousands 
> of these things per second. Also, the use case you describe is where 
> the servlet does its writes using a single background thread, so I 
> think it has good reasons to be interested in them. The situation when 
> the events are not needed is if the servlet doesn't care about not 
> sending a packet of data to the client, which could happen but may be 
> less common.
>
>> That is one of the main reason I like the trunk API, it gives a clear 
>> understanding of what events I'm about to receive. and yes, ERROR/END 
>> are an exception today, but that can be adjusted if one feels like 
>> it, personally I'm ok with those exceptions.
>
> Ok, so I am proposing a simpler API which does the same thing for the 
> user.
>
> I understand configuring many types of events could sound appealing, 
> and I certainly started on this side of the fence initially (the safe 
> design that you know will work), but after experimenting in the 
> sandbox I found it was not really needed and came up with this simpler 
> proposal.
>
> R�my
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>
>


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