[saga-rg] SAGA spec proof read

[Pascal Kleijer]

Hi,

some in-line comments...


>> 4. add_location can throw AlreadyExists but then tells in the notes that 
>> if the location exists in the set, no error is issued.
> 
> Nice catch!  I would propose to remove the exception - for
> most app level use cases I can think of this would be
> easier.

Yes I would agree that a silent "ignore" is the best. This also simplify 
the API a little.



>> Streams:
>> 1. Note, the document does not specify if the following pattern is 
>> allowed ?connect->close->connect?. If it is not stated we can assume 
>> that a stream can be ?reopened? (have a loop in the state machine).
> 
> We should add a state diagram.  That should make clear that
> loops are not possible.
> 
> Hmmm, on a second thought: re-open would be easy to
> implement (does not need to be the same stream on
> implementation level) - does it make sense for some use
> cases?  
> 
> Please note that the proposed messageing API does not make
> any assumption about the connection life time: that would
> allow to implement the message API with
> open/send/close/re-open/send/close ...

I think that a full state diagram is welcome. I also think that we can 
implement loops because the saga stream object is not the real stream, 
so it would not conflict with a re-open loop. The stream object as I 
could understand from the documentation is not self-persistent that 
would be up to the application to handle it. If the stream is dropped we 
could reconnect it with the same object without having the recreate a 
full context. It would also be coherent with a system that needs very 
irregular connections to a remote peer without using constant resources. 
Your messaging proposal is typically the case.




>> 2. ?read? should add that it will read at least 1 byte
>> from the stream. 
> 
> Hmm, read on a non-blocking stream might very well return
> 0 bytes, w/o meeting any error.
> 
> But the spec is confusing about the blocking issue: it says
> read is blocking - but that refers to the read method.  The
> stream the read occurs on might be a non-blocking stream
> (see attribute 'blocking').
> 
> I'll clean that up.
> 
> For a blocking read on a blocking stream: I agree there
> should be min 1 byte read.

OK, then a distinction between blocking and non-blocking in the 
description should be added. That would properly clarify the case.


>> Some extensions:
>> Purpose: ?Reads up to ?size? bytes of data from the stream into an array 
>> of bytes. An attempt is made to read as many as ?size? bytes, but a 
>> smaller number but may be read. The number of bytes actually read is 
>> returned as ?nbytes?. If ?size? is zero, then no bytes are read and 
>> nbytes is 0; otherwise, there is an attempt to read at least one byte.?
> 
> That should already be the case - that is why 'nbytes' is
> returned, indicating the actual number of bytes copied.
> nbytes can be smaller than size w/o posing an error
> condition (as in POSIX).  Is that what you propose?

Well I think the whole description should be redone based on the 
blocking or not behavior. Also the 'nbytes' returned should add, like in 
POSIX, the error code (negative). IF a new read is made after an fatal 
error, then the exception should kick in. See below.


> 
> 
>> 3. In this implementation we have no choice but to wait for an exception 
>> to be thrown when the stream has dried up (use the read method). It 
>> would be nice to have -1 set to nbytes to first time the end of stream 
>> is reached (if applies), then the next call to read will throw an exception.
> 
> What is the error condition the exception is supposed to
> catch:
> 
>   - stream died
>   - no data available
> 
> The first one is available via the state of the stream.
> 
> The second one is what you refer to, right?  But I think
> that this is not an error condition which should raise an
> exception: data might be available again a second later.
> 
> If data availability poses an error condition or not should
> be decided on application level IMHO.  
> 
> Am I missing something?

Basically the exception should be thrown if the last read did return 
with an error. If the error was rectified in the mean time, the read 
should go one as a new call. Therefore the state machine is important to 
  have, because a change of state will flush the last error.


> 
>> 4. There is no way to check the size of the data waiting for read. It 
>> would be nice if we can query the stream on how many bytes in the pipe 
>> are waiting. This is important if the client must allocate a buffer to 
>> read this data. So an additional method like ?available? should be 
>> added. Note that this will slightly overlap with the ?wait? method for 
>> ?read? or ?any? mask, but will return a more accurate figure. Another 
>> solution is to extend the wait method.
> 
> I would have no idea how to implement that on a stream:
> without an additional protocol, you have no chance to know
> how many bytes will arrive.
> 
> However, that is one of the reasons why the message API was
> proposed: that implies an additional protocol on the wire,
> but will allow the application to query the message size
> before performing the message read.

I don't say how many will arrive, but how many 'has' arrived. That mean 
how many are currently stored somewhere within the stream and that are 
ensured to be returned when the next call the read is done. In that case 
we can test for a possible block (if blocking) and size of the buffer to 
allocate (if necessary) as well as to avoid a catch block.


> 
> 
>> 5. It is not stated in the document if the stream ensures direct writing 
>> when ?write? is called. In the case there is an internal buffer is it up 
>> to the implementation to ensure the flush or is it best to add a flush 
>> method to the API? I would suggest adding a flush method which can be 
>> explicitly called by the user?s code, even if the implementation ignores it.
> 
> There is an attribute 'nodelay' which requires the
> implementation to avoid caching is possible.  That is
> neccessary for remote shell type applications (the enter
> after a command should be sent immediately, and not wait for
> another 1023 bytes ;-)
> 
> I'll make the description of that attribute more clear.

OK.


-- 

Best regards,
Pascal Kleijer

----------------------------------------------------------------
   HPC Marketing Promotion Division, NEC Corporation
   1-10, Nisshin-cho, Fuchu, Tokyo, 183-8501, Japan.
   Tel: +81-(0)42/333.6389       Fax: +81-(0)42/333.6382
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 4385 bytes
Desc: S/MIME Cryptographic Signature
Url : http://www.ogf.org/pipermail/saga-rg/attachments/20060609/a429ea76/attachment-0003.bin