Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-11
15:53:45 sean-k-mooney but ya that is proably more likely the reason for the delta in performance
15:54:31 mriedem sean-k-mooney: of course
15:54:40 Zara it's the other way around, though; the member role is the slow one and the admin role is the fast one
15:55:02 mriedem sounds like it's time to do some profiling
15:57:26 mriedem tssurya: can't we have some functional test on https://review.opendev.org/#/c/645611/ ?
15:58:46 mriedem efried: don't we have some rules about removing things from a runway slot if they've been in merge conflict for over a week and the author is not responsive?
15:59:08 mriedem "The code author must be ready for quick iteration on the patch reviews and responding to review comments. The author will be consulted before moving the blueprint to a runway to make sure the next 2 weeks are suitable for quick iteration on the reviews. If the next 2 weeks are not suitable, the next blueprint in line will be tried and the deferred blueprint will maintain its place in line."
15:59:10 efried mriedem: if not, we should.
15:59:21 mriedem ok i'm going to remove the sev stuff
15:59:25 efried mriedem: I've poked aspiers a couple of times about the merge conflict, so yeah.
15:59:25 tssurya mriedem: hmm I guess we could, I'll add some using your fake driver suggestion
15:59:32 efried thanks
15:59:37 sean-k-mooney mriedem: i think in general if the authour is not responcive that is enough to kick it
15:59:44 sean-k-mooney assuming it has -1 form a core
16:00:21 sean-k-mooney e.g. it need to be reworked but no active engagment form the owner
16:00:21 tssurya and I guess I have to bump microversion
16:15:29 openstackgerrit Matt Riedemann proposed openstack/nova master: WIP: Convert nova-next to a zuul v3 job https://review.opendev.org/670196
16:21:14 mriedem artom: pro tip, if rebasing https://review.opendev.org/#/c/667177/ is not necessary, don't do it
16:21:55 sean-k-mooney mriedem: gerrit need a diff diff setting
16:22:56 sean-k-mooney e.g. generate teh diff for the old patch against old base + new patch against new base and display the diff of those two diff per file
16:23:36 sean-k-mooney aka the eye ball diff we currently do but automated
16:24:31 melwitt yeah, been wanting that to exist for years. does git-review support something like that locally? I keep forgetting
16:25:31 sean-k-mooney not that i know of but it could
16:25:49 sean-k-mooney a 4 way diff is not that uncommon a thing
16:26:10 Zara (thanks for the suggestions so far btw; I disappeared to stare at the output of `nova list --debug` :) )
16:27:43 dansmith git review does have a thing
16:31:41 artom mriedem, I know, not sure what happened
16:32:33 artom I guess I was careless
16:33:03 artom I must have `git rebase -i` to edit that middle patch, and forgotten that I'd updated the remote?
16:33:31 mriedem git review -R
16:33:56 artom Aha, noted
16:42:44 openstackgerrit Dustin Cowles proposed openstack/nova master: Use SDK instead of ironicclient for node.list https://review.opendev.org/656027
16:42:44 openstackgerrit Dustin Cowles proposed openstack/nova master: Introduces SDK to IronicDriver and uses for node.get https://review.opendev.org/642899
16:42:45 openstackgerrit Dustin Cowles proposed openstack/nova master: Use SDK instead of ironicclient for setting instance id https://review.opendev.org/659690
16:42:45 openstackgerrit Dustin Cowles proposed openstack/nova master: Use SDK instead of ironicclient for validating instance and node https://review.opendev.org/656028
16:42:46 openstackgerrit Dustin Cowles proposed openstack/nova master: Use SDK instead of ironicclient for getting network metadata from node https://review.opendev.org/670213
16:42:46 openstackgerrit Dustin Cowles proposed openstack/nova master: Use SDK instead of ironicclient for add/remove instance info from node https://review.opendev.org/659691
16:45:59 efried rebase = false
16:45:59 efried [gitreview]
16:45:59 efried mriedem: artom: or in .gitconfig, so you don't have to remember -R each time:
16:46:29 melwitt mriedem: do you know if the add/remove security group to/from instance API is a candidate for deprecation? was just looking at this abandoned patch after noticing the bug in launchpad https://review.opendev.org/535510
16:46:56 mriedem efried: i don't personally want it in config since i make the decision each time
16:48:51 mriedem melwitt: i want to say it wasn't in https://specs.openstack.org/openstack/nova-specs/specs/newton/implemented/deprecate-api-proxies.html because it's an operation on the server which is not deprecated
16:48:54 mriedem even if it's a proxy to neutron
16:49:10 mriedem but we're not very consistent about that b/c the floating IP things in https://specs.openstack.org/openstack/nova-specs/specs/pike/implemented/deprecate-multinic-proxy-api.html are similar
16:49:49 melwitt mriedem: yeah, was thinking similar. at first I was comparing to attach interface but that's different bc you can't do that without nova. whereas it looks like you could do the add/remove secgroup to port without nova
16:49:51 mriedem addFloatingIP and removeFloatingIP would be in the same group as add/removeSecurityGroup on a server
16:51:23 melwitt ok, yeah. just wanted to ask bc the patch was abandoned on that basis (the potential for deprecation). otherwise, someone said they had tested the patch and it worked to fix the concurrent update problem
18:04:54 mriedem efried: i'm +2 on gibi's big heal_allocations change now https://review.opendev.org/#/c/637955/35
18:05:03 efried oh goodie
18:05:13 mriedem i'm sure you can just rubber stamp it with no effort
18:05:27 efried I'm sure
18:05:37 efried been waiting for you to quit hammering at it before looking again
18:05:42 efried I'm sure it's nigh unrecognizable now.
18:05:49 mriedem not since you last looked on
18:05:50 mriedem *no
18:06:01 mriedem more tests,
18:06:09 mriedem and the rollback stuff was changed
18:06:33 mriedem he first tries to update ports before allocations and if the port updates fail or allocation update fails the port updates are rolled back
18:07:42 efried okay, all of that sounds familiar.
18:07:48 efried I'll hit it now.
18:18:21 mriedem i'm working on a fup patch for my nits
18:21:42 openstackgerrit Matt Riedemann proposed openstack/nova master: Follow up for "nova-manage: heal port allocations" https://review.opendev.org/670361
18:35:17 openstackgerrit Merged openstack/nova master: Add host and hypervisor_hostname flag to create server https://review.opendev.org/645520
19:50:45 efried mriedem: gonna need your help understanding a thing here...
19:51:22 efried is this a patch-style update or a replace?
19:51:22 efried neutron.update_port(uuid, body=body)
19:51:53 efried i.e. does it only change the fields that are present in the body?
20:01:38 mriedem artom: sean-k-mooney: dansmith: https://review.opendev.org/#/c/667177/10
20:01:50 mriedem efried: replace
20:02:00 artom zomgies
20:02:15 mriedem efried: are you worried about overwriting something in the binding profile during the rollback from the time we originally got the port?
20:02:47 efried mriedem: Actually I'm worried about blowing away the whole content of the port *except* the binding:profile['allocation'] when we're trying to set that guy.
20:03:04 mriedem efried: wait,
20:03:28 mriedem efried: link me to the line
20:03:36 efried mriedem: yes, sec.
20:03:41 mriedem but we do port binding updates like this in the neutronv2/api code as well
20:04:50 efried mriedem: https://review.opendev.org/#/c/637955/35/nova/cmd/manage.py@1896 and continuation https://review.opendev.org/#/c/637955/35/nova/cmd/manage.py@1926
20:05:19 melwitt I didn't think it's like placement where anything not in the payload will get "erased" like a replace
20:05:55 efried well, there's a bug somewhere then. Because either we blow away everything but the allocation on the initial pass, or we do nothing on the rollback.
20:06:25 efried but mriedem yes indeed, I see port updates in neutronv2/api.py which are similar in spirit.
20:07:49 melwitt https://developer.openstack.org/api-ref/network/v2/?expanded=update-port-detail#update-port
20:08:17 efried melwitt: I read that; are you seeing something in there that implies "partial update"?
20:08:26 efried (I admit I didn't scrutinize it)
20:08:40 mriedem efried: agree that https://review.opendev.org/#/c/637955/35/nova/cmd/manage.py@1896 looks like trouble
20:08:50 mriedem https://developer.openstack.org/api-ref/network/v2/index.html?expanded=update-port-detail#revisions
20:08:56 mriedem neutron does have revisions and an If-Match header
20:09:03 mriedem like generations in placement and etags
20:09:18 mriedem anyway, that's more about what i thought you were going to bring up
20:10:10 efried I guess I would sort of hope you're not effing out of band with whatever ports you're trying to heal
20:10:53 efried but yeah, it would be nice to handle generations too.
20:11:03 mriedem looking at where this is used https://github.com/openstack/nova/blob/ff0feed25d56c8ccd2298d5b5b82e636880fa986/nova/network/neutronv2/api.py#L92
20:11:12 mriedem it looks like we only ever modify the full binding:profile and then send back our updates
20:11:15 mriedem not just pieces of it
20:12:04 efried so like, update_port knows to replace the thing for the top-level key you're sending, but doesn't go deeper?
20:12:51 mriedem i would assume that the neutron API takes the value for whatever key in the PUT /ports/{port_id} request and replaces the value, not parts of the value
20:12:52 efried in which case the second one (rollback) is fine, but the first one is broken?
20:12:58 mriedem it would assume so
20:13:25 efried "whatever key in the PUT request" -- the key that's in there is 'port'.
20:13:35 efried you mean the one under 'port'?

Earlier   Later