[openstack-dev] [nova] Versioned notifications... who cares about the version?

Andrew Laski andrew at lascii.com
Mon Nov 30 19:31:49 UTC 2015


On 11/30/15 at 07:32am, Sean Dague wrote:
>On 11/24/2015 10:09 AM, John Garbutt wrote:
>> On 24 November 2015 at 15:00, Balázs Gibizer
>> <balazs.gibizer at ericsson.com> wrote:
>>>> From: Andrew Laski [mailto:andrew at lascii.com]
>>>> Sent: November 24, 2015 15:35
>>>> On 11/24/15 at 10:26am, Balázs Gibizer wrote:
>>>>>> From: Ryan Rossiter [mailto:rlrossit at linux.vnet.ibm.com]
>>>>>> Sent: November 23, 2015 22:33
>>>>>> On 11/23/2015 2:23 PM, Andrew Laski wrote:
>>>>>>> On 11/23/15 at 04:43pm, Balázs Gibizer wrote:
>>>>>>>>> From: Andrew Laski [mailto:andrew at lascii.com]
>>>>>>>>> Sent: November 23, 2015 17:03
>>>>>>>>>
>>>>>>>>> On 11/23/15 at 08:54am, Ryan Rossiter wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 11/23/2015 5:33 AM, John Garbutt wrote:
>>>>>>>>>>> On 20 November 2015 at 09:37, Balázs Gibizer
>>>>>>>>>>> <balazs.gibizer at ericsson.com> wrote:
>>>>>>>>>>>> <snip>
>>>>>>>>>>>> <snip>
>>>>>>>>>>
>>>>>>>>>>> There is a bit I am conflicted/worried about, and thats when we
>>>>>>>>>>> start including verbatim, DB objects into the notifications. At
>>>>>>>>>>> least you can now quickly detect if that blob is something
>>>>>>>>>>> compatible with your current parsing code. My preference is
>>>>>>>>>>> really to keep the Notifications as a totally separate object
>>>>>>>>>>> tree, but I am sure there are many cases where that ends up
>>>>>>>>>>> being seemingly stupid duplicate work. I am not expressing this
>>>>>>>>>>> well in text form :(
>>>>>>>>>> Are you saying we don't want to be willy-nilly tossing DB
>>>>>>>>>> objects across the wire? Yeah that was part of the rug-pulling
>>>>>>>>>> of just having the payload contain an object. We're
>>>>>>>>>> automatically tossing everything with the object then, whether
>>>>>>>>>> or not some of that was supposed to be a secret. We could add
>>>>>>>>>> some sort of property to the field like
>>>>>>>>>> dont_put_me_on_the_wire=True (or I guess a
>>>>>>>>>> notification_ready() function that helps an object sanitize
>>>>>>>>>> itself?) that the notifications will look at to know if it puts
>>>>>>>>>> that on the wire-serialized dict, but that's adding a lot more
>>>>>>>>>> complexity and work to a pile that's already growing rapidly.
>>>>>>>>>
>>>>>>>>> I don't want to be tossing db objects across the wire.  But I
>>>>>>>>> also am not convinced that we should be tossing the current
>>>>>>>>> objects over the wire either.
>>>>>>>>> You make the point that there may be things in the object that
>>>>>>>>> shouldn't be exposed, and I think object version bumps is another
>>>>>>>>> thing to watch out for.
>>>>>>>>> So far the only object that has been bumped is Instance but in
>>>>>>>>> doing so no notifications needed to change.  I think if we just
>>>>>>>>> put objects into notifications we're coupling the notification
>>>>>>>>> versions to db or RPC changes unnecessarily.  Some times they'll
>>>>>>>>> move together but other times, like moving flavor into
>>>>>>>>> instance_extra, there's no reason to bump notifications.
>>>>>>>>
>>>>>>>>
>>>>>>>> Sanitizing existing versioned objects before putting them to the
>>>>>>>> wire is not hard to do.
>>>>>>>> You can see an example of doing it in
>>>>>>>> https://review.openstack.org/#/c/245678/8/nova/objects/service.py,
>>>>>>>> cm
>>>>>>>> L382.
>>>>>>>> We don't need extra effort to take care of minor version bumps
>>>>>>>> because that does not break a well written consumer. We do have to
>>>>>>>> take care of the major version bumps but that is a rare event and
>>>>>>>> therefore can be handled one by one in a way John suggested, by
>>>>>>>> keep sending the previous major version for a while too.
>>>>>>>
>>>>>>> That review is doing much of what I was suggesting.  There is a
>>>>>>> separate notification and payload object.  The issue I have is that
>>>>>>> within the ServiceStatusPayload the raw Service object and version
>>>>>>> is being dumped, with the filter you point out.  But I don't think
>>>>>>> that consumers really care about tracking Service object versions
>>>>>>> and dealing with compatibility there, it would be easier for them
>>>>>>> to track the ServiceStatusPayload version which can remain
>>>>>>> relatively stable even if Service is changing to adapt to db/RPC changes.
>>>>>> Not only do they not really care about tracking the Service object
>>>>>> versions, they probably also don't care about what's in that filter list.
>>>>>>
>>>>>> But I think you're getting on the right track as to where this needs
>>>>>> to go. We can integrate the filtering into the versioning of the payload.
>>>>>> But instead of a blacklist, we turn the filter into a white list. If
>>>>>> the underlying object adds a new field that we don't want/care if
>>>>>> people know about, the payload version doesn't have to change. But if
>>>>>> we add something (or if we're changing the existing fields) that we
>>>>>> want to expose, we then assert that we need to update the version of
>>>>>> the payload, so the consumer can look at the payload and say "oh, in
>>>>>> 1.x, now I get _______" and can add the appropriate checks/compat.
>>>>>> Granted with this you can get into rebase nightmares ([1] still
>>>>>> haunts me in my sleep), but I don't see us frantically changing the
>>>>>> exposed fields all too often. This way gives us some form of
>>>>>> pseudo-pinning of the subobject. Heck, in this method, we could even
>>>>>> pass the whitelist on the wire right? That way we tell the consumer
>>>> explicitly what's available to them (kinda like a fake schema).
>>>>>
>>>>> I think see your point, and it seems like a good way forward. Let's
>>>>> turn the black list to a white list. Now I'm thinking about creating a
>>>>> new Field type something like WhiteListedObjectField which get a type
>>>>> name (as the ObjectField) but also get a white_list that describes which
>>>> fields needs to be used from the original type.
>>>>> Then this new field serializes only the white listed fields from the
>>>>> original type and only forces a version bump on the parent object if
>>>>> one of the white_listed field changed or a new field added to the
>>>> white_list.
>>>>> What it does not solve out of the box is the transitive dependency. If
>>>>> today we Have an o.vo object having a filed to another o.vo object and
>>>>> we want to put the first object into a notification payload but want to
>>>>> white_list fields from the second o.vo then our white list needs to be
>>>>> able to handle not just first level fields but subfields too. I guess
>>>>> this is doable but I'm wondering if we can avoid inventing a syntax
>>>> expressing something like 'field.subfield.subsubfield'
>>>>> in the white list.
>>>>
>>>> Rather than a whitelist/blacklist why not just define the schema of the
>>>> notification within the notification object and then have the object code
>>>> handle pulling the appropriate fields, converting formats if necessary, from
>>>> contained objects.  Something like:
>>>>
>>>> class ServicePayloadObject(NovaObject):
>>>>      SCHEMA = {'host': ('service', 'host'),
>>>>                'binary': ('service', 'binary'),
>>>>                'compute_node_foo': ('compute_node', 'foo'),
>>>>               }
>>>>
>>>>      fields = {
>>>>          'service': fields.ObjectField('Service'),
>>>>          'compute_node': fields.ObjectField('ComputeNode'),
>>>>      }
>>>>
>>>>      def populate_schema(self):
>>>>          self.compute_node = self.service.compute_node
>>>>          notification = {}
>>>>          for key, (obj, field) in schema.iteritems():
>>>>              notification[key] = getattr(getattr(self, obj), field)
>>>>
>>>> Then object changes have no effect on the notifications unless there's a
>>>> major version bump in which case a SCHEMA_VNEXT could be defined if
>>>> necessary.
>>>
>>> Nice idea I will try it. Thanks! It is seems to avoid the sub object field white lists
>>> problem as the needed notification field can always be pulled directly from an object field.
>>
>> +1
>> This is my preference, specific notification objects that are
>> independently versioned.
>> It feels like time saving to re-use existing objects, but it breaks
>> the interface really.
>
>Ok, so that means we'll now have:
>
>* REST representation (strongly versioned / documented)
>* Notification representation (strongly versioned / documented )
>* Nova objects representation (strongly versioned / documented only in code)
>* Nova db objects (versioned by schema, documentated only in code)
>
>If the notifications are not going to be raw Nova objects, I think we
>need to really think about why they aren't the REST objects. Having a
>whole other additional interface surface seems really really weird.

Having notifications use the REST representation makes a lot of sense to 
me, for those things that have a REST representation.  My point has 
always just been that I think the raw Nova objects is wrong because the 
versions move for things that notification consumers don't care about at 
all, like a new remotable method being added to the object.


>
>	-Sean
>
>-- 
>Sean Dague
>http://dague.net
>
>__________________________________________________________________________
>OpenStack Development Mailing List (not for usage questions)
>Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
>http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



More information about the OpenStack-dev mailing list