<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 19, 2016 at 11:07 PM, Joshua Harlow <span dir="ltr"><<a href="mailto:harlowja@fastmail.com" target="_blank">harlowja@fastmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">Matt Riedemann wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
There are a lot of specs up for review in ocata related to adding new<br>
versioned notifications for operations that we didn't have notifications<br>
on before, like CRUD operations on resources like flavors and server<br>
groups.<br>
<br>
We've got a lot of legacy notifications for server actions, like<br>
server.pause.start and server.pause.end. Those are pretty simple.<br>
<br>
The thing that has me concerned about the CRUD operation notifications<br>
on resources is the extra DB query overhead to create the payloads which<br>
might not even get sent out.<br>
<br>
For example, I was reviewing this spec about adding notifications for<br>
CRUD ops on server groups:<br>
<br>
<a href="https://review.openstack.org/#/c/375316/" rel="noreferrer" target="_blank">https://review.openstack.org/#<wbr>/c/375316/</a><br>
<br>
Looking at the code for InstanceGroup, when a member is added to or<br>
removed from the group, the hosts field implicitly changes, but to<br>
calculate the hosts field we have to get all of the instances (members)<br>
in the group and then built the list of instance.host values.<br>
<br>
This is probably less of an issue if the server group object in scope<br>
already has the hosts field set, but if it doesn't and we're<br>
constructing it just for the notification, that's extra DB and RPC<br>
overhead - and notifications might not even be setup.<br>
<br>
I was thinking about it like logging details at debug level. If I need<br>
to build some large object or get some data for debugging something<br>
that's not in scope, I'd wrap that in a conditional:<br>
<br>
if LOG.isEnabledFor(logging.DEBUG<wbr>):<br>
LOG.debug('gimme da deets: %s', self.build_da_deets())<br>
<br>
But do we have anything like that for notifications? Basically, tell me<br>
if I should even bother building payloads for notifications.<br>
<br>
</blockquote>
<br></div></div>
A valid concern IMHO, seems like we might want a isEnabledFor() on the Notifier class in <a href="https://github.com/openstack/oslo.messaging/blob/master/oslo_messaging/notify/notifier.py#L175" rel="noreferrer" target="_blank">https://github.com/openstack/o<wbr>slo.messaging/blob/master/oslo<wbr>_messaging/notify/notifier.py#<wbr>L175</a> (I am assuming here that the underlying drivers can even provide the knowledge to know that, which may or may not be a big assumption?)<br>
<br>
-Josh</blockquote><div><br></div><div>+1. I can see this being useful. In ironic, we've managed to avoid the issue so far, but notification is still in its infancy (< 1 month old) and as it grows I think we will have to deal with this too. Ironic also has a [default]notification_level config option [1]; do you think that there will be a similar config option in oslo instead?</div><div><br></div><div>--ruby</div><div><br></div><div>[1] <a href="https://github.com/openstack/ironic/blob/a67facf208289b8bef3d7a8189e707d9d0ef6b9f/etc/ironic/ironic.conf.sample#L108-L112">https://github.com/openstack/ironic/blob/a67facf208289b8bef3d7a8189e707d9d0ef6b9f/etc/ironic/ironic.conf.sample#L108-L112</a></div></div></div></div>