Re: TTL Nodes: ZOOKEEPER-2169

2016-05-15 Thread Jordan Zimmerman
OK - let’s table this then. A new Issue with a long term solution is in order.

-Jordan

> On May 15, 2016, at 7:23 PM, Raúl Gutiérrez Segalés  
> wrote:
> 
> On 15 May 2016 at 21:01, Jordan Zimmerman  >
> wrote:
> 
>> Very good point. I guess we could add fields for future growth. So,
>> something like:
>> 
>>  class CreateRequest2 {
>>   ustring path;
>>   buffer data;
>>   vector acl;
>>   int flags;
>>   long ttl;
>>   long future1;
>>   long future2;
>>   long future3;
>>   long future4;
>>   }
>> 
>> Or is that silly?
>> 
> 
> It isn't, except it won't work now that I come to think of it. The way
> read-only support is handled is by not declaring the new (boolean) field in
> jute but by just trying to read it directly, i.e.:
> 
>boolean readOnly = false;
>try {
>readOnly = bia.readBool("readOnly");
>cnxn.isOldClient = false;
>} catch (IOException e) {
>// this is ok -- just a packet from an old client which
>// doesn't contain readOnly field
>LOG.warn("Connection request from old client "
>+ cnxn.getRemoteSocketAddress()
>+ "; will be dropped if server is in r-o mode");
>}
> 
> which destroys encapsulation. So more of those hacks is probably not
> sustainable. Moving those hacks into the jute compiler so that it handles
> optional fields (like the thrift compiler does) is probably more
> sustainable, but not a trivial effort. But, given that moving to thrift (or
> something else) is probably way harder we could explore that route.
> 
> 
>> 
>> Also, I know that Flavio and Patrick have been discussion file format
>> issues. It would be great if we could fix the hackiness of the
>> ephemeralOwner. Any thoughts on that?
>> 
> 
> Same as above — we need to make jute smarter or move to thrift to solve it.
> I suspect that anything else would be too much of a hack.
> 
> So, back to square one: we are tied with specialized classes for every new
> operation. At which point, we could probably introduce Stat2 to get away of
> the client-side ephemeralOwner hack. For the server-side, we are probably
> out of luck for now.
> 
> Thoughts?
> 
> 
> -rgs
> 
> 
> 
>> -Jordan
>> 
>>> On May 15, 2016, at 12:18 PM, Raúl Gutiérrez Segalés <
>> r...@itevenworks.net> wrote:
>>> 
>>> Hey Jordan,
>>> 
>>> On 8 May 2016 at 10:50, Jordan Zimmerman 
>> wrote:
>>> 
 Flavio,
 
 Do you think you can review and merge this change? I think the ZK
 community would really like this feature.
 
>>> 
>>> I am doing another pass now. While at it, I am thinking of this new
>> request
>>> type that's being introduced:
>>> 
>>> ```
>>>  class CreateTTLRequest {
>>>   ustring path;
>>>   buffer data;
>>>   vector acl;
>>>   int flags;
>>>   long ttl;
>>>   }
>>> ```
>>> 
>>> Should we make this more generic? This could very well be named
>>> CreateRequest2.
>>> 
>>> In the future, we could easily extend it by adding more fields. This is
>> how
>>> we extended the ConnectRequest class to support read-only in a backwards
>>> compatible way, for instance.
>>> 
>>> What do you all think? Is it worthwhile to make the (jute) type
>> extensible
>>> as opposed to having to many specialized request types?
>>> 
>>> 
>>> -rgs
>>> 
>>> 
>>> 
 -Jordan



Re: TTL Nodes: ZOOKEEPER-2169

2016-05-15 Thread Raúl Gutiérrez Segalés
On 15 May 2016 at 21:01, Jordan Zimmerman 
wrote:

> Very good point. I guess we could add fields for future growth. So,
> something like:
>
>   class CreateRequest2 {
>ustring path;
>buffer data;
>vector acl;
>int flags;
>long ttl;
>long future1;
>long future2;
>long future3;
>long future4;
>}
>
> Or is that silly?
>

It isn't, except it won't work now that I come to think of it. The way
read-only support is handled is by not declaring the new (boolean) field in
jute but by just trying to read it directly, i.e.:

boolean readOnly = false;
try {
readOnly = bia.readBool("readOnly");
cnxn.isOldClient = false;
} catch (IOException e) {
// this is ok -- just a packet from an old client which
// doesn't contain readOnly field
LOG.warn("Connection request from old client "
+ cnxn.getRemoteSocketAddress()
+ "; will be dropped if server is in r-o mode");
}

which destroys encapsulation. So more of those hacks is probably not
sustainable. Moving those hacks into the jute compiler so that it handles
optional fields (like the thrift compiler does) is probably more
sustainable, but not a trivial effort. But, given that moving to thrift (or
something else) is probably way harder we could explore that route.


>
> Also, I know that Flavio and Patrick have been discussion file format
> issues. It would be great if we could fix the hackiness of the
> ephemeralOwner. Any thoughts on that?
>

Same as above — we need to make jute smarter or move to thrift to solve it.
I suspect that anything else would be too much of a hack.

So, back to square one: we are tied with specialized classes for every new
operation. At which point, we could probably introduce Stat2 to get away of
the client-side ephemeralOwner hack. For the server-side, we are probably
out of luck for now.

Thoughts?


-rgs



> -Jordan
>
> > On May 15, 2016, at 12:18 PM, Raúl Gutiérrez Segalés <
> r...@itevenworks.net> wrote:
> >
> > Hey Jordan,
> >
> > On 8 May 2016 at 10:50, Jordan Zimmerman 
> wrote:
> >
> >> Flavio,
> >>
> >> Do you think you can review and merge this change? I think the ZK
> >> community would really like this feature.
> >>
> >
> > I am doing another pass now. While at it, I am thinking of this new
> request
> > type that's being introduced:
> >
> > ```
> >   class CreateTTLRequest {
> >ustring path;
> >buffer data;
> >vector acl;
> >int flags;
> >long ttl;
> >}
> > ```
> >
> > Should we make this more generic? This could very well be named
> > CreateRequest2.
> >
> > In the future, we could easily extend it by adding more fields. This is
> how
> > we extended the ConnectRequest class to support read-only in a backwards
> > compatible way, for instance.
> >
> > What do you all think? Is it worthwhile to make the (jute) type
> extensible
> > as opposed to having to many specialized request types?
> >
> >
> > -rgs
> >
> >
> >
> >> -Jordan
>
>


Re: TTL Nodes: ZOOKEEPER-2169

2016-05-15 Thread Jordan Zimmerman
Very good point. I guess we could add fields for future growth. So, something 
like:

  class CreateRequest2 {
   ustring path;
   buffer data;
   vector acl;
   int flags;
   long ttl;
   long future1;
   long future2;
   long future3;
   long future4;
   }

Or is that silly?

Also, I know that Flavio and Patrick have been discussion file format issues. 
It would be great if we could fix the hackiness of the ephemeralOwner. Any 
thoughts on that?

-Jordan

> On May 15, 2016, at 12:18 PM, Raúl Gutiérrez Segalés  
> wrote:
> 
> Hey Jordan,
> 
> On 8 May 2016 at 10:50, Jordan Zimmerman  wrote:
> 
>> Flavio,
>> 
>> Do you think you can review and merge this change? I think the ZK
>> community would really like this feature.
>> 
> 
> I am doing another pass now. While at it, I am thinking of this new request
> type that's being introduced:
> 
> ```
>   class CreateTTLRequest {
>ustring path;
>buffer data;
>vector acl;
>int flags;
>long ttl;
>}
> ```
> 
> Should we make this more generic? This could very well be named
> CreateRequest2.
> 
> In the future, we could easily extend it by adding more fields. This is how
> we extended the ConnectRequest class to support read-only in a backwards
> compatible way, for instance.
> 
> What do you all think? Is it worthwhile to make the (jute) type extensible
> as opposed to having to many specialized request types?
> 
> 
> -rgs
> 
> 
> 
>> -Jordan



Re: TTL Nodes: ZOOKEEPER-2169

2016-05-15 Thread Raúl Gutiérrez Segalés
Hey Jordan,

On 8 May 2016 at 10:50, Jordan Zimmerman  wrote:

> Flavio,
>
> Do you think you can review and merge this change? I think the ZK
> community would really like this feature.
>

I am doing another pass now. While at it, I am thinking of this new request
type that's being introduced:

```
   class CreateTTLRequest {
ustring path;
buffer data;
vector acl;
int flags;
long ttl;
}
```

Should we make this more generic? This could very well be named
CreateRequest2.

In the future, we could easily extend it by adding more fields. This is how
we extended the ConnectRequest class to support read-only in a backwards
compatible way, for instance.

What do you all think? Is it worthwhile to make the (jute) type extensible
as opposed to having to many specialized request types?


-rgs



> -Jordan


Re: TTL Nodes: ZOOKEEPER-2169

2016-05-08 Thread Jordan Zimmerman
Flavio,

Do you think you can review and merge this change? I think the ZK community 
would really like this feature.

-Jordan

Re: TTL Nodes: ZOOKEEPER-2169

2016-05-04 Thread Jordan Zimmerman
I see your point. The Container works as a double barrier, but not a single 
barrier. What’s interesting is that we could combine a Container with a TTL to 
get the single barrier. Allow Containers to have a TTL so that they aren’t 
deleted until a) they’ve had at least 1 child; b) they no longer have any 
children; c) TTL has elapsed without the Container being modified. The downside 
is that this forces the barrier to exist for the full length of the TTL.

Regarding immediate container deletion, that wouldn’t be hard to add. 

> On May 4, 2016, at 4:14 PM, Flavio Junqueira  wrote:
> 
> I was thinking of the following situation. Say that I have two clients 
> participating in the barrier. The first one immediately adds a node to the 
> container, but the other client is a bit slow. The first client crosses the 
> barrier and deletes the corresponding child. If the other client is still 
> slow and doesn't add a child node to the container, then the container 
> manager will delete the container incorrectly, indicating that everyone has 
> crossed the barrier. I suppose the way around it is to have all clients 
> waiting until everyone has added a child to the container, for that, all 
> clients need to know the number of children to expect and even that can be 
> tricky because a getChildren after a notification might miss a znode that has 
> already been deleted.
> 
> The issue, which is kind of arguable whether it is an issue or not, is that 
> I'd expect the container to go away as soon as the last child is deleted 
> rather than wait until the container manager deletes it. We could for example 
> expose a deleteContainer call and fail it if the delete condition doesn't 
> match. The delete condition could be no children or perhaps no children and 
> cversion >= number of clients in the barrier.
> 
> -Flavio 
> 
>> On 04 May 2016, at 18:32, Jordan Zimmerman  
>> wrote:
>> 
>> Containers do not get deleted if they’ve never had children (the cversion 
>> must be > 0). Also, containers are not deleted until they have no children. 
>> So, it does seem like a perfect fit. I don’t think the issue you listed are 
>> a concern unless I misunderstand.
>> 
>> -Jordan
>> 
>>> On May 4, 2016, at 12:01 PM, Flavio Junqueira  wrote:
>>> 
>>> Thanks, Jordan. Given that this is in the context of containers, after a 
>>> discussion offline with Ben, we were wondering what we would need to do to 
>>> implement barriers with containers.
>>> 
>>> A couple of issues we see are that:
>>> 
>>> - The container manager could potentially end up deleting a container even 
>>> before all nodes have joined the barrier by creating a child.
>>> - We depend on the container manager to delete the container, and it might 
>>> be better for barriers to enable it to be deleted explicitly upon the last 
>>> node crossing the barrier (deleting the last child).
>>> 
>>> Any thoughts here?
>>> 
>>> -Flavio
>>> 
 On 04 May 2016, at 17:28, Jordan Zimmerman  
 wrote:
 
 FYI
 
 I’ve posted an implementation for TTL Nodes, 
 https://issues.apache.org/jira/browse/ZOOKEEPER-2169 
 
 
 https://reviews.apache.org/r/46983/ 
 
 -Jordan
>>> 
>> 
> 



Re: TTL Nodes: ZOOKEEPER-2169

2016-05-04 Thread Flavio Junqueira
Thanks, Jordan. Given that this is in the context of containers, after a 
discussion offline with Ben, we were wondering what we would need to do to 
implement barriers with containers.

A couple of issues we see are that:

- The container manager could potentially end up deleting a container even 
before all nodes have joined the barrier by creating a child.
- We depend on the container manager to delete the container, and it might be 
better for barriers to enable it to be deleted explicitly upon the last node 
crossing the barrier (deleting the last child).

Any thoughts here?

-Flavio

> On 04 May 2016, at 17:28, Jordan Zimmerman  wrote:
> 
> FYI
> 
> I’ve posted an implementation for TTL Nodes, 
> https://issues.apache.org/jira/browse/ZOOKEEPER-2169 
> 
> 
> https://reviews.apache.org/r/46983/ 
> 
> -Jordan



TTL Nodes: ZOOKEEPER-2169

2016-05-04 Thread Jordan Zimmerman
FYI

I’ve posted an implementation for TTL Nodes, 
https://issues.apache.org/jira/browse/ZOOKEEPER-2169 


https://reviews.apache.org/r/46983/ 

-Jordan