host events - unused/unneeded values

[Nir Soffer]

Hi David,

While testing the python bindings, I noticed that get_event always return host_id=0:

{'data': 4,
 'event': 3,
 'from_generation': 13,
 'from_host_id': 3,
 'generation': 22,
 'host_id': 0}

If I'm not wrong, sanlock needs the host_id only when sending events, so this is
field is not used when sending events to listeners.

The generation field also seems unneeded, as sanlock filter evnets with wrong
generation, so you will never get an event with generation other then the current
generation - right?

Maybe we should change the api so:

When sending events:

    host_id = recipient host id (to)
    generation = recipient generation

When receiving events:

    host_id = sender host id (from)
    generation = sender generation

So we can eliminate the from_host_id and from_generation in get_event, simplifying
the api and eliminating some reads/writes.

I can eliminate the unneeded fields in the python bindings, but I prefer to return
the same data you get in the c api.

what do you think?

Nir

[David Teigland]

On Sun, Aug 10, 2014 at 01:44:23PM -0400, Nir Soffer wrote:
>  Hi David,
>  
>  While testing the python bindings, I noticed that get_event always return host_id=0:
>  
>  {'data': 4,
>   'event': 3,
>   'from_generation': 13,
>   'from_host_id': 3,
>   'generation': 22,
>   'host_id': 0}
>  
>  If I'm not wrong, sanlock needs the host_id only when sending events, so this is
>  field is not used when sending events to listeners.
>  
>  The generation field also seems unneeded, as sanlock filter evnets with wrong
>  generation, so you will never get an event with generation other then the current
>  generation - right?
>  
>  Maybe we should change the api so:
>  
>  When sending events:
>  
>      host_id = recipient host id (to)
>      generation = recipient generation
>  
>  When receiving events:
>  
>      host_id = sender host id (from)
>      generation = sender generation
>  
>  So we can eliminate the from_host_id and from_generation in get_event, simplifying
>  the api and eliminating some reads/writes.
>  
>  I can eliminate the unneeded fields in the python bindings, but I prefer to return
>  the same data you get in the c api.
>  
>  what do you think? 
I that approach when writing it, and it became messy and confusing, so I
chose instead that the sanlk_host_event struct that is received from
get_event is the same sanlk_host_event struct that was sent in set_event.

Technically, the host_id field should not be zero on the destination, though,
it should be the host_id of the destination itself.  This commit changes that:

diff --git a/src/lockspace.c b/src/lockspace.c
index 8309d25a11f5..316cad7e6b79 100644
--- a/src/lockspace.c
+++ b/src/lockspace.c
@@ -337,6 +337,7 @@ void check_other_leases(struct space *sp, char *buf)
                 */
 
                memset(&he, 0, sizeof(he));
+               he.host_id = sp->host_id;
                he.generation = leader->write_id;
                he.event = leader->write_generation;
                he.data = leader->write_timestamp;

[Federico Simoncelli]

----- Original Message -----
>  From: "David Teigland" <teigland(a)redhat.com&gt;
>  To: "Nir Soffer" <nsoffer(a)redhat.com&gt;
>  Cc: "Allon Mureinik" <amureini(a)redhat.com&gt;,
> sanlock-devel(a)lists.fedorahosted.org
>  Sent: Monday, August 11, 2014 5:49:28 PM
>  Subject: Re: host events - unused/unneeded values
>  
>  On Sun, Aug 10, 2014 at 01:44:23PM -0400, Nir Soffer wrote:
>  > Hi David,
>  > 
>  > While testing the python bindings, I noticed that get_event always return
>  > host_id=0:
>  > 
>  > {'data': 4,
>  >  'event': 3,
>  >  'from_generation': 13,
>  >  'from_host_id': 3,
>  >  'generation': 22,
>  >  'host_id': 0}
>  > 
>  > If I'm not wrong, sanlock needs the host_id only when sending events, so
>  > this is
>  > field is not used when sending events to listeners.
>  > 
>  > The generation field also seems unneeded, as sanlock filter evnets with
>  > wrong
>  > generation, so you will never get an event with generation other then the
>  > current
>  > generation - right?
>  > 
>  > Maybe we should change the api so:
>  > 
>  > When sending events:
>  > 
>  >     host_id = recipient host id (to)
>  >     generation = recipient generation
>  > 
>  > When receiving events:
>  > 
>  >     host_id = sender host id (from)
>  >     generation = sender generation
>  > 
>  > So we can eliminate the from_host_id and from_generation in get_event,
>  > simplifying
>  > the api and eliminating some reads/writes.
>  > 
>  > I can eliminate the unneeded fields in the python bindings, but I prefer to
>  > return
>  > the same data you get in the c api.
>  > 
>  > what do you think?
>  
>  I that approach when writing it, and it became messy and confusing, so I
>  chose instead that the sanlk_host_event struct that is received from
>  get_event is the same sanlk_host_event struct that was sent in set_event.
>  
>  Technically, the host_id field should not be zero on the destination, though,
>  it should be the host_id of the destination itself.  This commit changes
>  that:
>  
>  diff --git a/src/lockspace.c b/src/lockspace.c
>  index 8309d25a11f5..316cad7e6b79 100644
>  --- a/src/lockspace.c
>  +++ b/src/lockspace.c
>  @@ -337,6 +337,7 @@ void check_other_leases(struct space *sp, char *buf)
>                   */
>   
>                  memset(&he, 0, sizeof(he));
>  +               he.host_id = sp->host_id;
>                  he.generation = leader->write_id;
>                  he.event = leader->write_generation;
>                  he.data = leader->write_timestamp; 
+1

Minor suggestion, names I'd prefer are:

{'data': 4,
 'event': 3,
 'src_generation': 13,
 'src_host_id': 3,
 'dst_generation': 22,
 'dst_host_id': 0}

-- 
Federico

[David Teigland]

>  Minor suggestion, names I'd prefer are:
>  
>  {'data': 4,
>   'event': 3,
>   'src_generation': 13,
>   'src_host_id': 3,
>   'dst_generation': 22,
>   'dst_host_id': 0} 
src/dst are nice and clear when displayed together, but the values are not
adjacent in the API, so I don't see a big enough advantage to use prefixes
there.

struct sanlk_host_event {
        uint64_t host_id;
        uint64_t generation;
        uint64_t event;
        uint64_t data;
};

int sanlock_set_event(const char *ls_name, struct sanlk_host_event *he,
		      uint32_t flags);

int sanlock_get_event(int fd, uint32_t flags, struct sanlk_host_event *he,
                      uint64_t *from_host_id, uint64_t *from_generation);