You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by Carl Trieloff <cc...@redhat.com> on 2011/05/18 17:54:53 UTC

Change to store interface / binding encode/decode

In working a patch to add ownership to the broker model for ACL, I see
that bindings are the only object we don't use encode and decode. This
meant that the first version of my patch required a change to the
encode/decode of binding in the store code. This is a break in
abstraction, where the broker should be providing the encode/ decode.
The same problem then also exists in the cluster for bindings, in
talking with Alan.  (below)

The idea is to move to an encode/ decode in binding and the update the
signature of the MessageStore.h and remove the encode/ decode from the
inline code and add an interface method to the Recoverable objects.

Unless there are any objections, I'll proceed.  My end game is to be
able to provide simpler ACL scoping for cloud usage of Qpid brokers, to
do so we need the identity of who created which objects.


(11:31:41 AM) cctrieloff: where in the cluster do we sync bindings
(11:31:58 AM) cctrieloff: question is because binding is the one object
that does not have an encode and decode
(11:32:22 AM) cctrieloff: so If I add another member, what do I need to
do to make sure the cluster will sync it
(11:32:58 AM) aconway: bindings get synced with the rest of the update,
let me find the code...
(11:34:24 AM) aconway: UpdateClient::updateBinding - it just uses the
standard AMPQ bind command.
(11:36:26 AM) aconway: It iterates using Queue::eachBinding
(11:37:18 AM) cctrieloff: darn
(11:37:24 AM) cctrieloff: ok, here is the problem
(11:37:50 AM) cctrieloff: I need to add who created the binding into the
model for ACL.
(11:38:38 AM) cctrieloff: the issue is that is the cluster uses bind
then the user will be not the original user but rather other broker in
the cluster
(11:39:14 AM) cctrieloff: any ideas?
(11:39:14 AM) aconway: right. we need to add that info. I suggest we add
an encode to bindings for consistency.
(11:39:38 AM) cctrieloff: would you change the cluster to then use the
encode/ decode
(11:40:01 AM) cctrieloff: if so we should update the store to also use
the encode / decode, that would be a good cleanup
(11:41:01 AM) aconway: Exactly.
(11:41:40 AM) cctrieloff: great, is that something you want to do as you
know the cluster better than I do?
(11:43:40 AM) aconway: Sure, can you raise a BZ for me?
(11:44:04 AM) cctrieloff: I'm gong to post the IRC to the list and I'll
raise a BZ. that way people know what we are doing
(11:44:11 AM) aconway: Great.

Re: Change to store interface / binding encode/decode

Posted by Carl Trieloff <cc...@redhat.com>.
On 05/18/2011 03:00 PM, Gordon Sim wrote:
> On 05/18/2011 06:33 PM, Carl Trieloff wrote:
>> On 05/18/2011 12:41 PM, Gordon Sim wrote:
>>> On 05/18/2011 04:54 PM, Carl Trieloff wrote:
>>>>
>>>> In working a patch to add ownership to the broker model for ACL, I see
>>>> that bindings are the only object we don't use encode and decode. This
>>>> meant that the first version of my patch required a change to the
>>>> encode/decode of binding in the store code. This is a break in
>>>> abstraction, where the broker should be providing the encode/ decode.
>>>> The same problem then also exists in the cluster for bindings, in
>>>> talking with Alan.  (below)
>>>>
>>>> The idea is to move to an encode/ decode in binding and the update the
>>>> signature of the MessageStore.h and remove the encode/ decode from the
>>>> inline code and add an interface method to the Recoverable objects.
>>>>
>>>> Unless there are any objections, I'll proceed.  My end game is to be
>>>> able to provide simpler ACL scoping for cloud usage of Qpid
>>>> brokers, to
>>>> do so we need the identity of who created which objects.
>>>
>>> I don't have any objection however important to explicitly call out
>>> any changes that would break backward compatibility for recorded data
>>> if that cannot be avoided.
>>
>>
>> correct, if we leave the decode / encode in the store code it may be
>> possible to make it backwards compat, but is more error prove for
>> cluster for example.  This change will break compat for binding records
>> on the store, but will allow all future changes to be compat, and not
>> impact the cluster, because we have centralized the encode and decode
>> for bindings
>
> The cluster doesn't use the encode/decode for queues either. If you
> are adding new data to that stored for bindings (or indeed any other
> object) - e.g. an owner - then that will very much affect the cluster.

ack, I think I've got that one, will post patch for alan to check

>
> Again, I'm not necessarily objecting, just raising issues to be aware of.
>
>> It will still however be possible to read an old Store, and forward
>> migrate based on the jrnl version if that is desired.
>
> Reading an old store is essentially what I had in mind. I.e. ideally
> upgrade will not require you to discard previously stored data.

ack, that is what I implied.

>
>>> It would also be important to share the design around ACL changes.
>>> Again backward compatibility is relevant but even more critical is a
>>> well thought out (and well described) permissions model. What we have
>>> at present seems quite ad hoc already and we don't want to exacerbate
>>> that.
>>>
>>
>> All ACL work I have in mind will be backwards compat. and so far not
>> introducing more ACL syntax, just getting the impl behind the owner tag
>> that is already defined on the wiki etc, which simplifies may ACL's
>
> Excellent, do you have a URL for the details?

no. will first provide patch that adds ownership.

then will add QMF filter, or hit ted up for that.

then add the acl check for owner in another patch.

If others on the other hand want to help code.... well then ....

regards
Carl.


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: Change to store interface / binding encode/decode

Posted by Alan Conway <ac...@redhat.com>.
On 05/18/2011 03:00 PM, Gordon Sim wrote:
> On 05/18/2011 06:33 PM, Carl Trieloff wrote:
>> On 05/18/2011 12:41 PM, Gordon Sim wrote:
>>> On 05/18/2011 04:54 PM, Carl Trieloff wrote:
>>>>
>>>> In working a patch to add ownership to the broker model for ACL, I see
>>>> that bindings are the only object we don't use encode and decode. This
>>>> meant that the first version of my patch required a change to the
>>>> encode/decode of binding in the store code. This is a break in
>>>> abstraction, where the broker should be providing the encode/ decode.
>>>> The same problem then also exists in the cluster for bindings, in
>>>> talking with Alan. (below)
>>>>
>>>> The idea is to move to an encode/ decode in binding and the update the
>>>> signature of the MessageStore.h and remove the encode/ decode from the
>>>> inline code and add an interface method to the Recoverable objects.
>>>>
>>>> Unless there are any objections, I'll proceed. My end game is to be
>>>> able to provide simpler ACL scoping for cloud usage of Qpid brokers, to
>>>> do so we need the identity of who created which objects.
>>>
>>> I don't have any objection however important to explicitly call out
>>> any changes that would break backward compatibility for recorded data
>>> if that cannot be avoided.
>>
>>
>> correct, if we leave the decode / encode in the store code it may be
>> possible to make it backwards compat, but is more error prove for
>> cluster for example. This change will break compat for binding records
>> on the store, but will allow all future changes to be compat, and not
>> impact the cluster, because we have centralized the encode and decode
>> for bindings
>
> The cluster doesn't use the encode/decode for queues either. If you are adding
> new data to that stored for bindings (or indeed any other object) - e.g. an
> owner - then that will very much affect the cluster.
>

IMO that's a bug in the cluster, I would like to completely normalize the 
cluster update to use the exact same encode/decode as the store. It's almost 
there now, Carl's proposal would make it possible to complete it.

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: Change to store interface / binding encode/decode

Posted by Gordon Sim <gs...@redhat.com>.
On 05/18/2011 06:33 PM, Carl Trieloff wrote:
> On 05/18/2011 12:41 PM, Gordon Sim wrote:
>> On 05/18/2011 04:54 PM, Carl Trieloff wrote:
>>>
>>> In working a patch to add ownership to the broker model for ACL, I see
>>> that bindings are the only object we don't use encode and decode. This
>>> meant that the first version of my patch required a change to the
>>> encode/decode of binding in the store code. This is a break in
>>> abstraction, where the broker should be providing the encode/ decode.
>>> The same problem then also exists in the cluster for bindings, in
>>> talking with Alan.  (below)
>>>
>>> The idea is to move to an encode/ decode in binding and the update the
>>> signature of the MessageStore.h and remove the encode/ decode from the
>>> inline code and add an interface method to the Recoverable objects.
>>>
>>> Unless there are any objections, I'll proceed.  My end game is to be
>>> able to provide simpler ACL scoping for cloud usage of Qpid brokers, to
>>> do so we need the identity of who created which objects.
>>
>> I don't have any objection however important to explicitly call out
>> any changes that would break backward compatibility for recorded data
>> if that cannot be avoided.
>
>
> correct, if we leave the decode / encode in the store code it may be
> possible to make it backwards compat, but is more error prove for
> cluster for example.  This change will break compat for binding records
> on the store, but will allow all future changes to be compat, and not
> impact the cluster, because we have centralized the encode and decode
> for bindings

The cluster doesn't use the encode/decode for queues either. If you are 
adding new data to that stored for bindings (or indeed any other object) 
- e.g. an owner - then that will very much affect the cluster.

Again, I'm not necessarily objecting, just raising issues to be aware of.

> It will still however be possible to read an old Store, and forward
> migrate based on the jrnl version if that is desired.

Reading an old store is essentially what I had in mind. I.e. ideally 
upgrade will not require you to discard previously stored data.

>> It would also be important to share the design around ACL changes.
>> Again backward compatibility is relevant but even more critical is a
>> well thought out (and well described) permissions model. What we have
>> at present seems quite ad hoc already and we don't want to exacerbate
>> that.
>>
>
> All ACL work I have in mind will be backwards compat. and so far not
> introducing more ACL syntax, just getting the impl behind the owner tag
> that is already defined on the wiki etc, which simplifies may ACL's

Excellent, do you have a URL for the details?


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: Change to store interface / binding encode/decode

Posted by Carl Trieloff <cc...@redhat.com>.
On 05/18/2011 12:41 PM, Gordon Sim wrote:
> On 05/18/2011 04:54 PM, Carl Trieloff wrote:
>>
>> In working a patch to add ownership to the broker model for ACL, I see
>> that bindings are the only object we don't use encode and decode. This
>> meant that the first version of my patch required a change to the
>> encode/decode of binding in the store code. This is a break in
>> abstraction, where the broker should be providing the encode/ decode.
>> The same problem then also exists in the cluster for bindings, in
>> talking with Alan.  (below)
>>
>> The idea is to move to an encode/ decode in binding and the update the
>> signature of the MessageStore.h and remove the encode/ decode from the
>> inline code and add an interface method to the Recoverable objects.
>>
>> Unless there are any objections, I'll proceed.  My end game is to be
>> able to provide simpler ACL scoping for cloud usage of Qpid brokers, to
>> do so we need the identity of who created which objects.
>
> I don't have any objection however important to explicitly call out
> any changes that would break backward compatibility for recorded data
> if that cannot be avoided.


correct, if we leave the decode / encode in the store code it may be
possible to make it backwards compat, but is more error prove for
cluster for example.  This change will break compat for binding records
on the store, but will allow all future changes to be compat, and not
impact the cluster, because we have centralized the encode and decode
for bindings

It will still however be possible to read an old Store, and forward
migrate based on the jrnl version if that is desired.


>
> It would also be important to share the design around ACL changes.
> Again backward compatibility is relevant but even more critical is a
> well thought out (and well described) permissions model. What we have
> at present seems quite ad hoc already and we don't want to exacerbate
> that.
>

All ACL work I have in mind will be backwards compat. and so far not
introducing more ACL syntax, just getting the impl behind the owner tag
that is already defined on the wiki etc, which simplifies may ACL's

This can be detailed with the patches.

Carl.




---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


Re: Change to store interface / binding encode/decode

Posted by Gordon Sim <gs...@redhat.com>.
On 05/18/2011 04:54 PM, Carl Trieloff wrote:
>
> In working a patch to add ownership to the broker model for ACL, I see
> that bindings are the only object we don't use encode and decode. This
> meant that the first version of my patch required a change to the
> encode/decode of binding in the store code. This is a break in
> abstraction, where the broker should be providing the encode/ decode.
> The same problem then also exists in the cluster for bindings, in
> talking with Alan.  (below)
>
> The idea is to move to an encode/ decode in binding and the update the
> signature of the MessageStore.h and remove the encode/ decode from the
> inline code and add an interface method to the Recoverable objects.
>
> Unless there are any objections, I'll proceed.  My end game is to be
> able to provide simpler ACL scoping for cloud usage of Qpid brokers, to
> do so we need the identity of who created which objects.

I don't have any objection however important to explicitly call out any 
changes that would break backward compatibility for recorded data if 
that cannot be avoided.

It would also be important to share the design around ACL changes. Again 
backward compatibility is relevant but even more critical is a well 
thought out (and well described) permissions model. What we have at 
present seems quite ad hoc already and we don't want to exacerbate that.


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org


RE: Change to store interface / binding encode/decode

Posted by Steve Huston <sh...@riverace.com>.
Sounds perfectly reasonable, Carl.

Could you please post a review board item for this? Also, it sounds like
(without looking) the stored info for bindings will change after this
patch. If that's true, store databases will need to be changed or
replaced across the version change. Could you note this in your patch
for review?

Thanks,
-Steve

> -----Original Message-----
> From: Carl Trieloff [mailto:cctrieloff@redhat.com] 
> Sent: Wednesday, May 18, 2011 11:55 AM
> To: dev@qpid.apache.org
> Subject: Change to store interface / binding encode/decode
> 
> 
> 
> In working a patch to add ownership to the broker model for 
> ACL, I see that bindings are the only object we don't use 
> encode and decode. This meant that the first version of my 
> patch required a change to the encode/decode of binding in 
> the store code. This is a break in abstraction, where the 
> broker should be providing the encode/ decode. The same 
> problem then also exists in the cluster for bindings, in 
> talking with Alan.  (below)
> 
> The idea is to move to an encode/ decode in binding and the 
> update the signature of the MessageStore.h and remove the 
> encode/ decode from the inline code and add an interface 
> method to the Recoverable objects.
> 
> Unless there are any objections, I'll proceed.  My end game 
> is to be able to provide simpler ACL scoping for cloud usage 
> of Qpid brokers, to do so we need the identity of who created 
> which objects.
> 
> 
> (11:31:41 AM) cctrieloff: where in the cluster do we sync 
> bindings (11:31:58 AM) cctrieloff: question is because 
> binding is the one object that does not have an encode and 
> decode (11:32:22 AM) cctrieloff: so If I add another member, 
> what do I need to do to make sure the cluster will sync it 
> (11:32:58 AM) aconway: bindings get synced with the rest of 
> the update, let me find the code... (11:34:24 AM) aconway: 
> UpdateClient::updateBinding - it just uses the standard AMPQ 
> bind command. (11:36:26 AM) aconway: It iterates using 
> Queue::eachBinding (11:37:18 AM) cctrieloff: darn (11:37:24 
> AM) cctrieloff: ok, here is the problem (11:37:50 AM) 
> cctrieloff: I need to add who created the binding into the 
> model for ACL. (11:38:38 AM) cctrieloff: the issue is that is 
> the cluster uses bind then the user will be not the original 
> user but rather other broker in the cluster (11:39:14 AM) 
> cctrieloff: any ideas? (11:39:14 AM) aconway: right. we need 
> to add that info. I suggest we add an encode to bindings for 
> consistency. (11:39:38 AM) cctrieloff: would you change the 
> cluster to then use the encode/ decode (11:40:01 AM) 
> cctrieloff: if so we should update the store to also use the 
> encode / decode, that would be a good cleanup (11:41:01 AM) 
> aconway: Exactly. (11:41:40 AM) cctrieloff: great, is that 
> something you want to do as you know the cluster better than 
> I do? (11:43:40 AM) aconway: Sure, can you raise a BZ for me? 
> (11:44:04 AM) cctrieloff: I'm gong to post the IRC to the 
> list and I'll raise a BZ. that way people know what we are 
> doing (11:44:11 AM) aconway: Great.
> 


---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscribe@qpid.apache.org