Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-04-10
15:43:22 dansmith kashyap: and of course, you should be removing test.nested
15:47:38 edleafe dansmith: yeah, I'll push a new rev of that spec shortly
15:47:45 dansmith edleafe: ack thanks
15:47:55 lyarwood mriedem: yeah we can still race on our side, then again so could c-api when it's creating the attachment
15:48:08 lyarwood *attachments
15:48:34 mriedem so i can't remember what we said at the ptg the bdm unique constraint would be on, since it can't be volume_id and instance_uuid, since that would break multiattach
15:48:41 mriedem oh, nvm,
15:48:45 mriedem that would be ok
15:49:01 dansmith what if people want multiple attachments to the same vm and instance??
15:49:14 dansmith s/??/?/
15:49:37 lyarwood to the same volume and instance you mean?
15:49:50 dansmith heh yeah
15:49:51 lyarwood like during LM
15:50:05 dansmith or just for some sort of fictitious multipathing sort of thing
15:50:15 dansmith like they want multiple VFs on the same physnet today
15:50:39 mriedem via the cinder api, you can create multiple volume attachments to the same instance and volume
15:50:45 mriedem nova uses that for migrations
15:50:46 lyarwood we wouldn't model that using attachments, that's all within a single attachment and the connection_info it provides
15:51:10 mriedem only one attachment should be 'active' at any time
15:51:15 mriedem like what we're doing with the neutron port binding stuff
15:51:42 mriedem active = the attachment has a host connector and connection to the backend storage
15:52:10 dansmith lyarwood: I dunno, if we have multiple paths to the host from the volume provider, we'd need different attachments because of differing addresses right?
15:53:13 dansmith I'm just playing devil's advocate here to make sure we don't regret a decision later
15:53:20 dansmith removing a constraint is easy I guess
15:53:32 mriedem i don't even have a decision/solution for this race problem right now
15:53:33 dansmith although I'm not sure if mriedem is saying current live migration behavior would break there
15:53:37 lyarwood dansmith: hehe yeah I understand
15:54:10 mriedem the solution to fix the race with attaching the same volume to the same instance concurrently is a unique constraint on the bdms table over the volume_id and instance_uuid columns
15:54:17 mriedem but that doesn't fix lyarwood's new bug
15:55:11 mriedem you can't put a unique constraint on just the volume_id column since that would break multiattach
15:55:35 mriedem you almost need a conditional constraint, where volume_id must be unique if multiattach=False
15:55:46 mriedem but there is no such thing as a conditional unique constraint is there?
15:56:11 mriedem and jaypibbles ran off
15:56:29 dansmith mriedem: I think you need an active=$id column to do that
15:56:39 dansmith that's why we have deleted=$id I think
15:56:47 mriedem https://en.wikipedia.org/wiki/Check_constraint
15:57:38 dansmith hmm
15:57:45 dansmith I wonder what the performance of that is
15:57:50 mriedem i've always seen these in sqla-migrate but never played with one
15:58:00 mriedem zzzeek_: how terrible are check constraints?
15:58:20 dansmith wait,
15:58:20 zzzeek_ mriedem: most mysql / mariadb variants ignore them
15:58:25 lyarwood http://docs.sqlalchemy.org/en/latest/core/constraints.html#check-constraint
15:58:28 lyarwood Note that some databases do not actively support check constraints such as MySQL.
15:58:30 zzzeek_ mriedem: which is why you never see thme :)
15:58:30 dansmith you have to do your own uniqueness checking in the contraint then
15:58:33 lyarwood ^ yeah what zzzeek_ said
15:58:34 mriedem gdi
15:58:49 zzzeek_ lyarwood: mariadb 10.2 does now. oddly enough this creates more problems :)
15:59:35 dansmith mriedem: I'm sure DB2 supports them and is just hanging out by the punch bowl waiting for someone to care
15:59:47 mriedem i <3 DB2
15:59:52 dansmith I know you do
16:00:00 mriedem i heard ms azure rolled out a dbaas service and i noticed it didn't include db2
16:00:03 mriedem i was hurt
16:00:14 dansmith and shocked, I'm sure
16:00:22 mriedem it does include pg
16:02:07 mriedem ok so if we had an 'active' column on the bdms table, we could set that to true when we do something like set the connection_info on it
16:02:11 mriedem which means it's attached
16:02:29 mriedem but still,
16:02:40 mriedem you could have >1 bdm on the same volume which are both 'active'
16:02:58 mriedem if that volume is multiattach=true
16:04:57 mriedem wonder if there is something that can be done on the cinder side, i.e. a rule saying, you can't have >1 attachment record to the same volume for different instances if the volume is multiattach=false
16:05:15 mriedem or if that's already the rule they have in place
16:14:46 mriedem lee bugged out, but i might have a fix on the cinder side
16:14:51 smcginnis mriedem: I think we can't due to things like migration.
16:14:52 mriedem glory hallelujah
16:15:13 mriedem smcginnis: i'll poke you with the patch when it's up, and i'll hope lee can apply and see if it solves his issue
16:15:25 smcginnis mriedem: OK, sounds like a plan.
16:19:31 openstackgerrit Lee Yarwood proposed openstack/nova master: rbd: flatten images when creating/unshelving an instance https://review.openstack.org/457886
16:21:25 melwitt mriedem: thanks for adding the neutron stuff to the forum ideas etherpad
16:21:32 mriedem np
16:22:37 mriedem lyarwood: does that also fix bug 1732428?
16:22:37 openstack bug 1732428 in OpenStack Compute (nova) "Unshelving a VM breaks instance metadata when using qcow2 backed images" [Medium,In progress] https://launchpad.net/bugs/1732428 - Assigned to Matt Riedemann (mriedem)
16:24:18 lyarwood mriedem: no, flatten is specific to the rbd imagebackend
16:24:49 openstackgerrit Eric Berglund proposed openstack/nova master: PowerVM: Add proc_units_factor conf option https://review.openstack.org/554688
16:26:16 jgriffith lyarwood: I added a note to https://bugs.launchpad.net/cinder/+bug/1762687
16:26:16 openstack Launchpad bug 1762687 in Cinder "Concurrent requests to attach the same non-multiattach volume to multiple instances can succeed" [High,New]
16:26:28 lyarwood thanks ./me looks
16:26:33 jgriffith I think the race is the condition check in _reserve_volume on the cidner side
16:26:37 jgriffith mriedem: ^^
16:27:23 mriedem jgriffith: just about got something here
16:27:46 jgriffith mriedem: oh... so I guess that means I was wrong?
16:27:55 mriedem haven't read the comment yet,
16:27:58 mriedem just fixing tests
16:28:01 jgriffith Oh... LOL
16:28:12 jgriffith mriedem: so what you're saying is "there's a chance" :)
16:28:22 smcginnis :)
16:28:28 jgriffith if this were slack I'd insert stupid gif here
16:30:12 smcginnis I think that's the whole reason why folks like slack over irc. :)
16:31:11 melwitt dansmith: this is the patch we talked briefly about on friday at the ptg about flattening rbd images if not CONF.use_cow_images. I had asked the room if there was any usefulness in someone configuring that way and you had said some people would to get better performance https://review.openstack.org/#/c/457886
16:31:20 mriedem lyarwood: can you test this out? https://review.openstack.org/560074
16:31:27 lyarwood mriedem: sure can
16:33:17 mriedem dansmith: melwitt: just fyi, i'll be out for a few hours this afternoon
16:33:56 melwitt k
16:33:57 dansmith melwitt: okay, was there more to that question?
16:36:18 melwitt dansmith: lyarwood rebased it a little while ago and it reminded me that I had been meaning to ask if you could review it. I added the bit about using the CONF.use_cow_images config option as a toggle for flattening
16:36:28 dansmith okay
16:40:59 lyarwood mriedem: that appears to be enough but I'm just spamming requests from the cli again
16:43:23 mriedem lyarwood: ok i'm updating it per jgriffith's comment
16:43:59 jgriffith lyarwood: so just the refresh was enough?

Earlier   Later