Re: [MirageOS-devel] Irmin watch API

[Thomas Gazagnaire <thomas@xxxxxxxxxxxxxx>]

> On 25 Apr 2015, at 10:46, Thomas Leonard <talex5@xxxxxxxxx> wrote:
> 
> On 24 April 2015 at 15:47, Thomas Gazagnaire <thomas@xxxxxxxxxxxxxx> wrote:
>> I've just merged the new watch API into the master branch of Irmin and I've 
>> updated the online docs.
>> 
>> Feedback is welcome, the new functions are described on the related pull 
>> request [1].
> 
> Some more documentation would be useful. Currently, I see:
> 
>  val watch_head: t -> ?init:head -> (head diff -> unit Lwt.t) ->
>    (unit -> unit Lwt.t) Lwt.t
>  (** [watch_tag t f] calls [f] every time the contents of [t]'s tag is
>      updated. Do nothing if [t] is not persistent. Return a clean-up
>      function to remove the watch handler.
> 

> "Do nothing if [t] is not persistent"
This bit can be improved actually...


>      {b Note:} even [f] might skip some head updates, it will never
>      be called concurrently: all consecutive calls to [f] are done in
>      sequence, so we ensure that the previous one ended before
>      calling the next one. *)
> 
> What does the "init" argument do? Will the callback be called
> immediately with the current value if it's not given? What's the
> recommended race-free way to start watching a branch?

The init argument initialise the initial value kept by the watcher. If set to 
None, you'll have a `Added x` as first diff value, if not you'll have an 
`Updated (x,y)` where `x` is your initial value and `y` the new head. So you 
should first read the current head, then call watch_head with that head as init.

> Currently (with the old API) I do:
> 
>      let watch_tags = I.watch_head (store "Watch branch") in
>      I.head (store "Get latest commit") >>= ... initial head ...
>      let head_id = ref (Some initial_head_id) in
>      async (fun () ->
>        watch_tags |> Lwt_stream.iter_s (function
>          ...
>          I.head (store "Get latest commit") >>= fun new_head_id ->
>          if new_head_id <> !head_id then (
>            head_id := new_head_id
>          )
>        )
>      )
> 
> Here, I'm assuming that any change made after I.watch_head returns
> will result in the callback being called. If the head has changed by
> the time the initial I.head thread returns then the callback might get
> called unnecessarily, but that's OK.
> 
> How should this be done with the new API? I'm guessing something like:
> 
> 1. Read the current head into a ref.
> 2. Call I.watch_head with a callback to update the ref.
> 3. Read the current head again in case it updated between (1) and (2).

Not sure why you need 3. I guess you need it if you want to be sure that the 
head hasn't changed between the time the callback has been prepared (ie. to 
fetch the current head) and then called by Irmin - but if it is the case, the 
callback will be called again with the new head just after that. It is not 
possible to protect the callback completely though, as you have an other 
thread/process which can modify the store behind your back: in that case Irmin 
will detect the change (more or less quickly) and call the callback again with 
`Update (x,y)` where x is the previous head sent to the callback and y the new 
store's head.

I'll improve the doc and give some examples though.

Thomas
_______________________________________________
MirageOS-devel mailing list
MirageOS-devel@xxxxxxxxxxxxxxxxxxxx
http://lists.xenproject.org/cgi-bin/mailman/listinfo/mirageos-devel