17:00:23 <gibi> #startmeeting nova notification
17:00:24 <openstack> Meeting started Tue Jul 18 17:00:23 2017 UTC and is due to finish in 60 minutes.  The chair is gibi. Information about MeetBot at http://wiki.debian.org/MeetBot.
17:00:26 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
17:00:28 <openstack> The meeting name has been set to 'nova_notification'
17:00:32 <mriedem> o/
17:00:33 <gibi> hi!
17:01:02 <gibi> mriedem: thanks for the countdown on #openstack-nova that made me realize that I need to chair this now
17:01:26 <mriedem> np
17:01:36 <gibi> let's get started with the update_at bugfix
17:01:41 <gibi> #link https://review.openstack.org/#/c/475276/
17:01:55 <gibi> I had some time to play with the problem
17:02:17 <gibi> and it seems that the missing update_at is more a like a legacy behavior than a bug in the notification
17:02:30 <gibi> #link https://review.openstack.org/#/c/475276/13/doc/notification_samples/instance-update.json
17:02:39 <gibi> here is my summary ^^
17:02:59 <gibi> and a separate bug report about the issue #link https://bugs.launchpad.net/nova/+bug/1704928
17:03:00 <openstack> Launchpad bug 1704928 in OpenStack Compute (nova) "updated_at field is set on the instance only after it is scheduled" [Undecided,New]
17:03:20 <gibi> I digged the problem a bit but I failed to found the root cause
17:03:29 <gibi> I commented my findings to the bug report
17:04:01 <mriedem> hmm ok
17:04:39 <gibi> sooo, I'm +1 on Takashi's original patch
17:04:45 <mriedem> so i think takashi is saying,
17:04:53 <mriedem> if the instance.updated_at field was set, it would be in there,
17:05:02 <mriedem> so it must not be set on the instance object at the time we send that notification
17:05:04 <mriedem> for whatever reason
17:05:09 <gibi> yes
17:05:26 <gibi> what I found is that at the first two instance.update it is not set
17:05:34 <gibi> but at the third and later ones it is set
17:05:57 <gibi> this is why Takashi's patch adds the override here https://review.openstack.org/#/c/475276/13/nova/tests/functional/notification_sample_tests/test_instance.py@263
17:06:02 <gibi> in the sample test
17:06:11 <gibi> at the third step
17:08:22 <mriedem> i also can't find where we ever set instance.updated_at
17:08:49 <gibi> mriedem: me neither
17:09:03 <mriedem> it probably happens in oslo.db
17:09:09 <mriedem> as part of the timestamp mixin
17:09:15 <gibi> mriedem: I tracked it down to the instance.save()
17:09:27 <gibi> mriedem: the into the model_query but I lost it there
17:09:36 <mriedem> yeah it would go into oslo.db
17:09:54 <mriedem> updated_at = Column(DateTime, onupdate=lambda: timeutils.utcnow())
17:10:05 <mriedem> so onupdate trigger for the record
17:10:09 <mriedem> should update the updated_at
17:10:16 <mriedem> so anytime an instance.save results in an db record update
17:10:18 <mriedem> the updated_at should be updated
17:11:20 <gibi> mriedem: my test shows that it did not happen
17:11:45 <gibi> mriedem: only only happen at first when the host and the node of the instance updated
17:12:30 <gibi> hm, before that only state and task_state is updated but that did not result in an updated_at update
17:12:45 <mriedem> right those changes should set updated_at
17:14:08 <mriedem> anyway we don't have to figure this out here
17:14:48 <gibi> I could look into it more during the week, let's see if I found something or not
17:15:09 <gibi> mriedem: do you agree to merge Takashi's original patch without fixing this bug?
17:15:41 <mriedem> ummmmmmmm
17:15:47 <mriedem> is this a court of law?
17:16:14 <gibi> mriedem: it is more like asking for oppinion :)
17:16:24 <mriedem> i'd like to understand the updated_at bug a bit more first
17:16:37 <gibi> mriedem: OK, then I will try to make progress there
17:17:10 <gibi> let's move on
17:17:43 <gibi> from the searchlight patch we only miss the tag in the instance.create
17:18:00 <gibi> I saw that you made comments there
17:18:16 <mriedem> yeah, that needs some work
17:18:29 <gibi> I hope Kevin_Zheng can fix that in time
17:18:33 <mriedem> my main concern is that the instance doesn't have the tags field set when it gets to the compute for the instance.create notification,
17:18:37 <mriedem> which would result in a lazy-load
17:19:04 <mriedem> i have https://review.openstack.org/#/c/484170/ to fix that but it needs to resolve some weird db api interaction that's causing tests to randomly fail
17:19:05 <gibi> mriedem: yeah, I saw your optimizing patch that make tags available
17:19:32 <mriedem> there is a temporary workaround we can do in the notifications change too if my optimization doesn't get done in time,
17:19:34 <gibi> mriedem: but I had no time to look at the db failures
17:19:49 <mriedem> which is just set instance.tags = instance_tags in conductor schedule_and_build_instances before casting to compute
17:20:27 <mriedem> either way we'll get it done
17:20:30 <mriedem> i'm not too worried about it
17:20:40 <gibi> OK
17:20:49 <gibi> Kevin_Zheng: if you need help, just ping me
17:20:55 <gibi> moving on
17:21:41 <gibi> you had the good point optimizing out some BDM db loading by reusing already in scope BDMs
17:21:50 <gibi> I started a patch series to do that optimization
17:22:04 <gibi> #link https://review.openstack.org/#/c/483324/
17:22:23 <gibi> I still need to create additional patches to that series
17:22:36 <gibi> to handle the special notifications like volume_swap
17:22:48 <gibi> but you can see the generic idea in the current patches already
17:22:57 <mriedem> yeah
17:24:06 <gibi> I guess it is OK if that will not get review bandwidth and therefore deferred to Queens
17:24:53 <mriedem> since loading up bdms for notifications is disabled by default,
17:24:57 <mriedem> i think it's ok if this happens in queens
17:25:31 <gibi> yes it is off by default
17:25:48 <gibi> OK, moving to my last point
17:26:18 <gibi> you asked about the status of https://blueprints.launchpad.net/nova/+spec/json-schema-for-versioned-notifications
17:26:42 <gibi> and I replied that we made no progress and I don't even know if we will have the bandwith in Queens
17:27:22 <mriedem> yeah i deferred the bp to queens
17:27:49 <gibi> I guees if nobody shows up to help with that then it will not be approved for Queens and that is OK
17:27:57 <mriedem> i think whether or not someone picks it up depends on if some consumer of the versioned notifications, like searchlight, actually needs this
17:28:16 <mriedem> otherwise we're spending time on something no one is using
17:28:43 <gibi> mriedem: I agree
17:29:11 <gibi> and that was my last point
17:29:31 <gibi> do you have anything else to talk about?
17:29:36 <mriedem> nope
17:29:59 <gibi> then let's close this
17:30:06 <gibi> thanks for the discussion!
17:30:15 <mriedem> sure, ttyl
17:30:22 <gibi> #endmeeting