D-Bus interface to update multiple properties in one shot

D-Bus interface to update multiple properties in one shot

[Deepak Kodihalli]

Hi,

I've come across openbmc apps that implement D-Bus interfaces comprised 
of more than one D-Bus property (phosphor-networkd is a good example), 
and they need to react only after all the properties or a specific set 
of properties have been set, and not after every individual property is 
set. This can be due to functional or performance reasons. For instance 
in the case of phosphor-networkd, it has to restart a networkd systemd 
unit when a property changes. Multiple restarts can actually cause 
systemd to prevent the unit from further restarts. Multiple restarts is 
also not desirable in terms of the app's performance. What 
phosphor-networkd does today is to start a 5 (or 10) second timer when a 
property is set, and restart the networkd unit after the timeout, so as 
to accumulate other properties being set in this time. I believe other 
daemons are doing this as well.

One solution is to design these APIs as D-Bus methods as opposed to a 
set of D-Bus properties. We've avoided this in general because you'd 
then need to define separate getters and setters, and like mentioned 
earlier, a caller may want to set only a subset of all the properties in 
the interface.

A proposal we discussed in a meet recently is to implement a new 
xyz.openbmc_project.Object.Properties D-Bus interface with a method 
called 'Update', which accepts a dictionary of property-value pairs. A 
D-Bus object may implement this interface to be able to set multiple 
properties in one go. The type of the value would need to be a variant 
of all native D-Bus data types. Supporting container data types is 
probably going to be hard, so this may have to be restricted to 
non-container data types. The key would be a string - the property name. 
Thoughts on this proposal?

Thanks,
Deepak

RE: D-Bus interface to update multiple properties in one shot

[Tanous, Ed]

> 
> A proposal we discussed in a meet recently is to implement a new
> xyz.openbmc_project.Object.Properties D-Bus interface with a method
> called 'Update', which accepts a dictionary of property-value pairs.

I'm not really following what problem this is trying to solve.  Does the timer in networkd not work as designed?  Are we trying to remove it?  In which use cases are we seeing performance problems with multiple set calls?  Have we measured these performance impacts?

What worries me about the proposed interface is consistency.  Will we be adding the xyz.openbmc_project.Object.Properties interface as a specification to certain paths in phosphor? all paths? All object manager paths?
How will one know which paths require use of the org.freedesktop.DBus.Properties interface, and which can use the interface you've proposed?  Yes, we could do a mapper call to find out, but I'm going to negate any performance improvement we've gained.
When calling the update method, if one of the property update fails, what then?  Does the property update get partially applied, does it roll back to the old values?  I don't think there are any good catchall answers here.

This idea intrigues me, but I think in many of these cases, consistency is more important than performance.  If there's a way to keep consistency with the proposed interface _and_ get performance, then I think that this could be workable, I'm just not sure how to accomplish both.

-Ed

Re: D-Bus interface to update multiple properties in one shot

[Deepak Kodihalli]

On 07/09/18 8:14 PM, Tanous, Ed wrote:
>>
>> A proposal we discussed in a meet recently is to implement a new
>> xyz.openbmc_project.Object.Properties D-Bus interface with a method
>> called 'Update', which accepts a dictionary of property-value pairs.
> 
> I'm not really following what problem this is trying to solve.  Does the timer in networkd not work as designed?  Are we trying to remove it?  In which use cases are we seeing performance problems with multiple set calls?  Have we measured these performance impacts?

The problem I'm trying to solve is with D-Bus apps (currently 
implementing org.freedesktop.DBus.Properties) that want to take a 
specific action after all or a subset of a D-Bus properties that they 
expose have been set. The typical action in these cases is restarting a 
service (because say on each set, you update a config file). It's the 
restart that's causing performance problems - because there could be too 
many of them, if you were updating a lot of properties. I don't think 
the timer is intuitive to a user of the API, because in scenarios where 
only a subset of the properties are updated, the additional wait time to 
get served is unexpected to the user of the API.

> What worries me about the proposed interface is consistency.  Will we be adding the xyz.openbmc_project.Object.Properties interface as a specification to certain paths in phosphor? all paths? All object manager paths?

Specific (not object manager) paths. For eg I don't think we'd need to 
add this to objects implementing inventory interfaces (those are a bunch 
of properties as well). I view implementing this interface as an 
(optional) improvement over org.freedesktop.DBus.Properties, in some 
cases, but this is not a replacement for the 
org.freedesktop.DBus.Properties interface. So objects implementing this 
proposed interface must implement org.freedesktop.DBus.Properties as well.

> How will one know which paths require use of the org.freedesktop.DBus.Properties interface, and which can use the interface you've proposed?  Yes, we could do a mapper call to find out, but I'm going to negate any performance improvement we've gained.

This can be described in the documentation of the D-Bus API. For eg, how 
does one know they have to implement org.freedesktop.DBus.ObjectManager 
(this is not a mandatory interface to implement)? This is typically 
documented in the D-Bus API.

> When calling the update method, if one of the property update fails, what then?  Does the property update get partially applied, does it roll back to the old values?  I don't think there are any good catchall answers here.

Good point. I think the caller must be able to resort to using the 
org.freedesktop.DBus.Properties interface. The update method of the 
proposed interface can fail on the first set that fails, and return back 
the properties that were successfully updated.

Thanks,
Deepak

> This idea intrigues me, but I think in many of these cases, consistency is more important than performance.  If there's a way to keep consistency with the proposed interface _and_ get performance, then I think that this could be workable, I'm just not sure how to accomplish both.
> 
> -Ed
> 

Re: D-Bus interface to update multiple properties in one shot

[Deepak Kodihalli]

On 10/09/18 12:08 PM, Deepak Kodihalli wrote:
> On 07/09/18 8:14 PM, Tanous, Ed wrote:
>>>
>>> A proposal we discussed in a meet recently is to implement a new
>>> xyz.openbmc_project.Object.Properties D-Bus interface with a method
>>> called 'Update', which accepts a dictionary of property-value pairs.
>>
>> I'm not really following what problem this is trying to solve.  Does 
>> the timer in networkd not work as designed?  Are we trying to remove 
>> it?  In which use cases are we seeing performance problems with 
>> multiple set calls?  Have we measured these performance impacts?
> 
> The problem I'm trying to solve is with D-Bus apps (currently 
> implementing org.freedesktop.DBus.Properties) that want to take a 
> specific action after all or a subset of a D-Bus properties that they 
> expose have been set. The typical action in these cases is restarting a 
> service (because say on each set, you update a config file). It's the 
> restart that's causing performance problems - because there could be too 
> many of them, if you were updating a lot of properties. I don't think 
> the timer is intuitive to a user of the API, because in scenarios where 
> only a subset of the properties are updated, the additional wait time to 
> get served is unexpected to the user of the API.
> 
>> What worries me about the proposed interface is consistency.  Will we 
>> be adding the xyz.openbmc_project.Object.Properties interface as a 
>> specification to certain paths in phosphor? all paths? All object 
>> manager paths?
> 
> Specific (not object manager) paths. For eg I don't think we'd need to 
> add this to objects implementing inventory interfaces (those are a bunch 
> of properties as well). I view implementing this interface as an 
> (optional) improvement over org.freedesktop.DBus.Properties, in some 
> cases, but this is not a replacement for the 
> org.freedesktop.DBus.Properties interface. So objects implementing this 
> proposed interface must implement org.freedesktop.DBus.Properties as well.
> 
>> How will one know which paths require use of the 
>> org.freedesktop.DBus.Properties interface, and which can use the 
>> interface you've proposed?  Yes, we could do a mapper call to find 
>> out, but I'm going to negate any performance improvement we've gained.
> 
> This can be described in the documentation of the D-Bus API. For eg, how 
> does one know they have to implement org.freedesktop.DBus.ObjectManager 
> (this is not a mandatory interface to implement)? This is typically 
> documented in the D-Bus API.
> 
>> When calling the update method, if one of the property update fails, 
>> what then?  Does the property update get partially applied, does it 
>> roll back to the old values?  I don't think there are any good 
>> catchall answers here.
> 
> Good point. I think the caller must be able to resort to using the 
> org.freedesktop.DBus.Properties interface. The update method of the 
> proposed interface can fail on the first set that fails, and return back 
> the properties that were successfully updated.
> 
> Thanks,
> Deepak
> 
>> This idea intrigues me, but I think in many of these cases, 
>> consistency is more important than performance.  If there's a way to 
>> keep consistency with the proposed interface _and_ get performance, 
>> then I think that this could be workable, I'm just not sure how to 
>> accomplish both.
>>
>> -Ed
>>
> 

I've put this for review on Gerrit - 
https://gerrit.openbmc-project.xyz/#/c/openbmc/phosphor-dbus-interfaces/+/12861/.

Thanks,
Deepak