[SAGA-RG] Saga question (for a change ...)

[Andre Merzky]

> Hi Andre,

Hi Ceriel, 

[Cc to list]


> I sent you the mail above about 3 weeks ago, did you get it?

*blush*  I did get it, but it slipped down the stack.  Sorry for
that, and thanks for the reminder!


Quoting [Ceriel Jacobs] (Sep 23 2010):
> 
> Ceriel Jacobs wrote:
>
>> Hi Andre,
>> 
>> I have yet again a couple of questions about the Saga
>> specifications, this time about streams.
>> 
>> - the stream_server::serve() method has a timeout parameter, and
>> the notes say: "if no client connects within the specified
>> timeout, a 'Timeout' exception is thrown". This seems to
>> contradict section 2.6.3, which sais: "These methods MUST not
>> cause a Timeout exception as the timeout period passes, but MUST
>> return silently". Should the serve() method indeed throw a
>> Timeout exception? I guess serve() could just return null in this
>> case.

I rather think that 2.6.3 is too vague here.  It says 

  "Several methods in the SAGA API support the synchronization of
  concurrent operations."

2.6.3 is about async ops mostly, and thread concurrency, and I don't
think the serve() method should be amongst those affected.  It may
be better to rename serve(timout) to serve(time-to-wait), to mak
clear that is different?

Returning a 0 is not a good option in many languages IMHO: the
return type is a stream instance - so you would need to return an
invalid instance in this case, i.e. one in Error state.  That is not
easily distinguishable from an instance which got successfully
created, but whose connection failed subsequently.

What do you think?



>> - related is the stream::connect method, which now also has a
>> timeout parameter. What should that method do when the timeout
>> expires? Note that it does not have a return value, and that the
>> PostCond: is not fulfilled in that case. So, should it throw a
>> NoSuccessException? That also seems to contradict the above
>> quoted sentence from 2.6.3. Or should the stream state just
>> remain "New"?

The call details say:

  "on failure, the stream state is changed to 
  ’Error’"

That makes more sense here, IMHO, as the instance already exists,
and state needs to be checked in any case.


> Also, I have some more questions/remarks now:
>
> 
> Task container
>
> - metric "task_container.state": should be a String, not an Int,
> since a Saga object id is a string. This was correct in an earlier
> version, but apparently never made it to the repository.

The metric value is set to the tasks 'cookie' (not object id), which
we define to be an int.  So, I think it is correct.  Makes sense?

Having said that, I agree that changing that to object ID would make
more sense, as we get rid of the cookie - which now *only* exists
for this specific use case (in combination with get_task(int
cookie).  However, that is an API change, and I am not sure if we
should push those, if they are not fixing a real error...

Either way, I noted that down for future changes.  


> - at some point, the add() notes said that it throws an
> AlreadyExists if the specified task is already a member, but
> AlreadyExists was not in the throws list. Now the specs say that
> the addition is silently ignored, which is OK, I guess, but
> remove() DOES throw a DoesNotExist exception if the task is not a
> member of the task container.

Yep.  The rationale is that adding a task twice will not hurt the
application in general: it will still get notified when that task
changes, etc.  On remove() however, we can assume that the
application logic is incorrect.

I agree though that this is not symmetric.  How much of a problem is
it though?  Noted it down for later, but do you think this requires
fixing right now?


> - there is no difference between list_tasks and get_tasks.  Should
> they still both exist?

list_tasks should go away, right.



> Buffer
>
> - set_data() and set_size() should also throw NoSuccess since they
> are equivalent to calling the corresponding version of the
> constructor, and the constructor throws NoSuccess.

Right.  Malloc failure for example should result in NoSuccess I
guess...


> Namespaces
>
> - ns_entry.remove(): the notes say to things that seem to
> contradict:
>
> a) if the Recursive flag is defined, the target is recursively
> removed if it is a directory, otherwise this flag is ignored
>
> b) a BadParameter is thrown if the entry is not a directory and
> the Recursive flag is set.

*sigh*  true.  I guess the latter is safer.  Your thaughts?


> - ns_directory.remove(URL target)
>
> the notes say: if the instance was not closed before, this call
> performs a close on the instance ....
>
> I don't understand that. Why close the ns_directory when an entry
> is removed from it? My guess is that this note should be in the
> ns_entry.remove() notes, not in the ns_directory.remove(URL
> target) notes.

Correct.


> I am trying to finally getting the Java language bindings
> up-to-par with the current version of the Saga specs. I think I am
> mostly done with that now, apart from the issues above (for as far
> as the language bingings are affected).

:-)

But isn't it disturbing that issues like the above keep popping up,
after so many eyes red the spec, and *implemented* it?  *scratch*

Many thanks for the thoroughness! :-D  Will you be in Brussels for
OGF by any chance?

Also, on a side note: do you have any thoughts on the message API,
which is in public comment right now?  Or do you know somebody at
VU, or elsewhere, who could give qualified comments?  You IBIS guys
do IPC all the time, so this is your area of expertiese after all
:-D

Best, Andre.



> Cheers,		Ceriel



-- 
Nothing is ever easy.