Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-07-10
19:50:50 dansmith man that's a lot of rechecks
19:51:11 efried melwitt: I had previously -2'd https://review.openstack.org/#/c/521041/ which is the patch that actually puts the code in the way of the compute manager. But perhaps we want to move that -2 to the bottom as you say, since the first two patches don't do anything without the top one.
19:52:25 melwitt efried: k. at a glance, it looked like things we could merge as progress so it would help to -W or -2 to show that it needs to wait
19:52:53 efried melwitt: Yes, we could merge the bottom two without hurting anything.
19:53:26 efried melwitt: I think some people might object to merging what's essentially "dead code".
19:53:51 efried melwitt: Me, I'd rather see it merged so we've got a shorter path when we're ready to pull the trigger.
19:54:09 efried But no strong feelings either way.
19:54:21 dansmith I think we should wait
19:54:46 dansmith merging refactors early in a set or something are useful, but if it's really dead code, we might as well wait, IMHO
19:55:48 melwitt I don't have a strong feeling about it. I think it'd be okay to merge some progress but waiting is also fine
19:56:59 melwitt mriedem: we have a proof-of-concept patch for the rbd erasure coding config option as of today, fyi https://review.openstack.org/581055
19:57:23 melwitt jmlowe: is this something we can see working in the ceph job results? using the erasure coding? http://logs.openstack.org/55/581055/2/check/legacy-tempest-dsvm-full-devstack-plugin-ceph/d6331b3/
19:58:54 jmlowe you're thinking functional test?
19:59:44 melwitt jmlowe: no, I mean I was wondering if there's anything ceph would log (in the ceph job I linked) that shows passing the data_pool did something. just curious if it's something we could see
20:01:18 jmlowe I don't think so, let me poke around a little more, you can check the data pool of a rbd device with the ceph api but it's supposed to be relatively transparent aside from disk usage increasing in some other pool
20:02:01 jmlowe or is that what you are thinking, write a gig and check the disk usage of the pools?
20:02:57 melwitt okay, that's cool. no, I wasn't thinking ahead that complex, just thought it'd be handy if something got reflected in the ceph job run for free
20:04:55 melwitt for one thing, I doubt the job is running luminous so it wouldn't do anything anyway
20:07:26 melwitt oh, it actually is. cool. http://logs.openstack.org/55/581055/2/check/legacy-tempest-dsvm-full-devstack-plugin-ceph/d6331b3/logs/ceph/ceph-mgr.x.txt.gz#_2018-07-10_01_16_55_913130
20:08:12 jmlowe yeah, it is running luminous so it doesn't blow up
20:08:57 melwitt jmlowe: what do you mean, what would make it blow up if not luminous?
20:09:29 dansmith passing the flag that turns it on right?
20:09:39 dansmith because EC isn't supported pre-limunous?
20:09:39 jmlowe I'm pretty sure it's a generic abstraction, so you can do stupid ceph tricks like set the data pool to some other replicated pool, that wouldn't do anything useful except save you the extra work of setting up an erasure coded pool in a testing environment
20:09:44 dansmith luminous
20:10:21 melwitt well, I thought jmlowe had said that passing data_pool=None pre-luminous would make sure it doesn't hurt anything
20:10:26 jmlowe guessing if you used the EOL jewel python binding it wouldn't have the kwarg for data_pool
20:10:49 melwitt oh, I see. so we need to guard this behavior with a check for ceph version
20:11:09 jmlowe yeah, nobody should be using jewel by the time this lands
20:11:16 melwitt or are we safe assuming has to be >= luminous at this point?
20:11:21 melwitt okay
20:11:55 jmlowe you have to do stupid rpm tricks with centos if you wanted to use it with queens or later
20:12:37 melwitt okay, good to know
20:12:46 jmlowe I think the queens repo forces install of luminous repo for centos, was an annoyance when I was going to mimic
20:21:15 mriedem dansmith: i just went through Kevin_Zheng's 2nd in series live migration abort queued patch and only thing that kind of bothers me is the duplicate validation in the rpc api here https://review.openstack.org/#/c/568542/15/nova/compute/rpcapi.py - i've left an alternative and looking for a 2nd opinion on that
20:23:11 dansmith mriedem: your link in that comment doesn't seem to point to anything relevant
20:24:50 dansmith presumably you're looking at something that is checking service version
20:25:16 dansmith which is cool, but if the service_version says things are okay, but they still have their rpc api manually pinned, they could conflict, so failing somehow is probably appropriate
20:28:51 dansmith oh sorry, are you talking about the migration status?
20:29:01 mriedem yeah
20:29:15 dansmith sorry, I totally focused on the version checks
20:29:25 mriedem the api already checks if the migration status is running and if not it bombs out (today)
20:29:37 mriedem then calls the rpc api which if the compute is old, makes the same check
20:29:44 dansmith I don't think I understand why we need to check the status of the migration again
20:29:54 mriedem see my comment
20:30:28 mriedem i think the scenario is you are attempting to abort a queued live migration but the compute is old
20:30:49 dansmith yeah I see it, but.. this code runs in the api itself, and doesn't actually know the version of the compute it's going to talk to
20:30:52 mriedem his check for 'compute too old' is the can_send_version
20:31:06 dansmith that's not compute-aware
20:31:58 mriedem so his version check on the nova-compute service needs to happen in the api change if the user is trying to abort a !running migration and the host is old
20:32:02 mriedem s/host/service/
20:32:09 dansmith can_send_version() is looking at the api service's rpc pin, which might be computed from service version, or manually set
20:32:23 dansmith mriedem: right
20:32:28 mriedem ok, he's just following similar checks in compute rpc api for things like tagged attachments and such
20:32:49 dansmith mriedem: and by the time we get to rpcapi, we don't have any new information we didn't have a couple frames up on the stack, so no need to do it again
20:32:57 openstackgerrit Matt Rabe proposed openstack/nova master: Add destination MSP IP address to PowerVM migrate data https://review.openstack.org/581463
20:33:07 dansmith yeah, those are for things that haven't already checked service version for the actual compute and are relying on the pin I think
20:33:20 dansmith which is a way to do it, but you don't need to do it in both places
20:33:58 mriedem so if we check the nova-compute service version, we don't need the can_send_version (or any) changes to the compute rpcapi at all right?
20:34:08 dansmith well, no,
20:34:09 openstackgerrit Chris Dent proposed openstack/nova master: Remove unused variable in migration https://review.openstack.org/581464
20:34:16 dansmith which is why I was saying earlier
20:34:52 dansmith you can check the service version to know whether all computes or the compute supports the !running cancel, but you don't know if the rpc pin is manually set low,
20:34:53 dansmith so you need to do the can_send_version check in rpcapi
20:35:09 openstackgerrit Matt Rabe proposed openstack/nova master: Add destination MSP IP address to PowerVM migrate data https://review.openstack.org/579676
20:35:19 dansmith which might be a reason to just do it there only, and not also up in api and just catch the exception
20:35:50 dansmith let rpcapi tell you if the thing you're asking for is doable right now, and if not tell the user they can't have it
20:35:55 dansmith since rpcapi can check both things
20:36:11 dansmith don't need to check the service version itself for this I don't think
20:36:16 mriedem yeah ok - i was wondering why he didn't just *move* the status check to rpcapi instead of duplicate it
20:36:20 dansmith yeah
20:36:30 mriedem you want to leave a comment in that change?
20:36:38 dansmith not really, but I will :P
20:36:42 mriedem thanks
20:55:31 mriedem this is weird https://github.com/openstack/nova/blob/8469fa70dafa83cb068538679100bede7679edc3/nova/objects/service.py#L323
20:57:42 cdent mriedem: that is odd
20:58:09 mriedem https://review.openstack.org/#/c/161822/5/nova/objects/service.py
20:58:20 openstackgerrit Rick Bartra proposed openstack/nova master: Add granularity to os-services API https://review.openstack.org/578553
20:59:13 mriedem i have no idea why that was added there,
20:59:17 mriedem but lots of people were approving of the chnage
20:59:19 mriedem *change
20:59:51 mriedem aha https://review.openstack.org/#/c/161822/2/nova/db/sqlalchemy/api.py@425
20:59:57 mriedem the db api used to behave that way
21:00:04 mriedem so it was mirrored in the object
21:01:13 openstackgerrit Eric Fried proposed openstack/nova master: Tighten up ReportClient use of generation https://review.openstack.org/556669
21:01:27 mriedem apparently the service startup code relies on that to determine if it needs to create the service record
21:01:30 mriedem rather than just handling the notfound
21:11:05 openstackgerrit Eric Fried proposed openstack/nova master: Check provider generation and retry on conflict https://review.openstack.org/556669
21:37:23 openstackgerrit Ken'ichi Ohmichi proposed openstack/nova master: Avoid BadRequest error log on volume attachment https://review.openstack.org/581453
21:45:51 mriedem dansmith: question in the numa aware vswitch hardware change here https://review.openstack.org/#/c/564448/14/nova/tests/unit/virt/test_hardware.py@3343
21:46:42 dansmith mriedem: see my comment on that in an earlier PS?
21:46:55 mriedem nope
21:47:05 dansmith mriedem: I think the deal is, the physnet may be available on the host, but the operator hasn't said that it's particularly affined to some node
21:47:21 dansmith I stumbled on that as well, but worked out the reasoning from the comments and the tests
21:48:06 mriedem but the point is,
21:48:18 dansmith I dunno if that would just be "the operator doesn't care enough to configure the thing" or if there's some reason why it may not be affined, like it's too dynamic or something
21:48:28 mriedem as a user i can say i want to be affined to baz and if the operator hasn't reported baz, we'll still do the build even though you might not be affined to baz
21:48:47 mriedem i'm thinking of it like inventory - is that the wrong way to think about this?
21:48:56 dansmith no, that comment says that the user has requested network baz, and the compute node will do it,
21:49:02 mriedem i see stephenfin has also dropped the patch that does any RT claims code

Earlier   Later