Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-05-02
18:22:55 mgagne afaik, no. we run only one scheduler as cache is done in-memory and not shared
18:23:17 mgagne we used to see a lot of rescheduling with FilterScheduler IIRC
18:23:36 mriedem well, CachingScheduler is FilterScheduler
18:23:41 mriedem except with the cache
18:23:48 mgagne jroll: remember that OOM issue you had with scheduler with ironic? ;)
18:23:58 jroll heh
18:24:06 jroll that was loading instances and such
18:24:28 mgagne scheduler taking a gazillion bytes of ram
18:24:36 mgagne jroll: yea, great stuff
18:24:38 jroll idk, I'd like to see some benchmarking on master before we make decisions about CachingScheduler being useful on master
18:25:15 mriedem i've asked for benchmarking between placement + filterscheduler and cachingscheduler since at least ocata/pike at every summit
18:25:16 mgagne jroll: being useful is not really the issue, it's more that a migration path is not ready yet for those using CachingScheduler
18:25:25 mriedem mgagne: i'm working on that CLI right now
18:25:52 jroll right, if we backport that CLI we should be all good
18:25:54 mriedem and i did remember when replying to the ML that newton and ocata computes will write allocations to placement,
18:26:03 mriedem it was only in pike when we disabled that because the filter scheduler creates the allocations now
18:26:12 mgagne mriedem: tyvm for working on it. IMO, if it's not merged, it's not done. hence my concern with removing stuff that could break my use case.
18:26:42 mriedem mgagne: sure, but also, if you're on mitaka, you have plenty of time for this to land before you're ready to roll to it
18:27:05 mriedem i.e. the cachingscheduler and ironic_host_manager removal
18:27:28 mgagne mriedem: I'm more on a defensive stance with that regard, can never be too careful
18:28:02 mriedem i understand :)
18:28:47 mgagne on principal, I can't agree with someone suggesting a new feature/fix not yet merged to replace something that is going to be removed.
18:30:59 mgagne or principle?
18:31:10 artom The latter
18:35:20 openstackgerrit Dan Smith proposed openstack/nova master: Add CellMapping.get_by_project_id() query method https://review.openstack.org/509002
18:35:22 openstackgerrit Dan Smith proposed openstack/nova master: Make get_instance_objects_sorted() be smart about cells https://review.openstack.org/509003
18:35:23 dansmith tssurya: ^
18:35:46 tssurya dansmith: ack, and thanks
18:35:56 dansmith I think we may want to consider a flag to let you choose between "always scatter to all cells" and "only hit cells for which you have instances"
18:36:21 dansmith for your case, the latter will be a big improvement, but for people that spread across cells, doing the extra mapping lookup will be a waste
18:36:31 tssurya dansmith: sounds good, yea makes sense
18:37:03 mgagne artom: :)
18:37:28 tssurya but are you talking about doing this in the listing context alone ? or a generic flag for the scatter-gather utility
18:37:55 dansmith scatter-gather can't do it generically,
18:37:59 dansmith because it only applies to instances
18:38:08 dansmith so it would just be instance listing that would honor that
18:38:43 jroll how ready for primetime is cells v2 in ocata?
18:38:57 tssurya yea I mean scatter-gather already has the selected cells querying option, what I mean was this "only hit cells for which you have instances" sounds really appealing in a lot of places
18:39:05 tssurya like quota calculating while booting
18:39:09 tssurya calculation*
18:39:19 tssurya meant*
18:39:46 dansmith tssurya: sure, it makes sense to do it in those sorts of places, I just think it has to be baked into each one that is tenant-scoped
18:39:46 tssurya so was wondering if there was a way to flag this more generically
18:39:59 tssurya dansmith: hmm, yea
18:40:22 dansmith tssurya: anywhere we can do tenant-scoped lookups should look at that flag, if that's what you mean
18:40:29 dansmith jroll: not very
18:40:41 jroll whomp whomp
18:40:44 tssurya dansmith: yes,
18:41:58 melwitt jroll: multi cell is not a thing in ocata, I'd say
18:42:16 jroll melwitt: yeah, that was my actual question, thanks
18:43:03 melwitt and, affinity scheduling doesn't work with multi cell yet, there's a bug (I have a patch up to fix it) https://review.openstack.org/540258
18:43:35 melwitt I know that's something y'all need
18:45:02 jroll tis, thanks for that
19:02:33 melwitt hm, seeing an issue where a BFV instance being deleted fails to detach the volume because attachment_id = None http://logs.openstack.org/01/565601/2/check/tempest-full/f1004d9/controller/logs/screen-n-cpu.txt.gz#_May_02_03_50_00_958050
19:03:55 cfriesen anyone know offhand what's causing this failure when running tox against newton? http://paste.openstack.org/show/720243/
19:04:50 melwitt did you do a tox -e<whatevs> -r first?
19:04:57 melwitt usually have to do that when you switch branches
19:05:39 cfriesen will try that, thanks
19:06:47 melwitt usually you'll have different deps versions required per branch and that's where those errors come from. if the env has a module version missing an attribute that another branch needs
19:11:21 melwitt so for the BFV volume detach, it's using the old attach API, that means bdm.attachment_id was not set or None
19:11:56 melwitt and then the old os-detach API is called with attachment_id = null and that yields a 400 from cinder
19:13:33 melwitt now to figure out why attachment_id isn't set for a BFV instance created from a snapshot
19:22:04 mriedem melwitt: i know exactly why
19:22:08 arvindn05 efried: thanks for the review :). For the comment on the commit message, i just squash my individual commits to create the single commit that gerrit requires..hence the Added unit and functional tests
19:22:20 mriedem melwitt: https://review.openstack.org/#/c/541420/
19:22:44 melwitt yesss, I was hoping there was already a patch for dat
19:22:49 mriedem melwitt: either way, when detaching we should know if we're doing new style detach (attachment_id is not None) or old style detach
19:23:05 efried arvindn05: You mean in your sandbox you had multiple commits, one that did the code and one that did the test, and you squashed them together before proposing them to gerrit?
19:23:19 arvindn05 yup
19:23:36 efried arvindn05: Yeah, that doesn't change my opinion :) There's no need for a commit message on a change that clearly has unit and functional tests to say that it has unit and functional tests.
19:23:39 melwitt mriedem: right. we're doing old style because attachment_id is None but, attachment_id sent over to cinder in the old style os-detach action call is also None which gets a 400 response
19:23:45 efried arvindn05: But you notice I'm not blocking on it either.
19:24:16 melwitt so old style detach fails
19:24:34 mriedem we don't send an attachment_id to os-detach in cinder
19:24:40 mriedem it's not a parameter for that api
19:24:49 mriedem well, i don't think it is
19:24:52 melwitt it's in the log here http://logs.openstack.org/01/565601/2/check/tempest-full/f1004d9/controller/logs/screen-n-cpu.txt.gz#_May_02_03_50_00_958050
19:24:54 mriedem for new style detach, we'd call attachment_delete
19:25:03 mriedem log schmog, that's fake news
19:25:05 melwitt it's the only thing we're sending
19:25:06 arvindn05 efried: cool....i dont like the gerrit workflow...more used to github which allows individual commits and lets the maintainer decide on squash vs rebase vs commit as is
19:25:21 arvindn05 but going with the flow :)
19:25:36 melwitt WHAT SAY YOU
19:26:17 efried arvindn05: You're welcome (even encouraged) to split up reviews in gerrit, though generally if along test/impl lines the test should come first.
19:26:44 efried arvindn05: Me, I've never used github, so no basis for comparison there.
19:26:46 mriedem oh yeah i guess os-detach does take an attachment_id parameter, which is optional
19:27:14 melwitt it's weird that of all the params, that's the only thing we're sending. seems wrong
19:27:15 mriedem melwitt: we can't detach the volume on a volume-backed instance, is that what you're trying to do?
19:27:31 mriedem *root volume
19:28:34 melwitt mriedem: maybe. this is a delete_on_termination=True instance created from a snapshot (the tempest test) and when we delete the instance, it tries to detach the volume and delete it. and it fails to detach it
19:28:46 mriedem this schema check looks wrong
19:28:47 mriedem InvalidInput: Invalid input received: Invalid input for field/attribute attachment_id. Value: None. None is not of type 'string'
19:28:57 melwitt a BFV instance, that is
19:29:04 mriedem this schema doesn't say attachment_id is required https://github.com/openstack/cinder/blob/master/cinder/api/schemas/volume_actions.py#L73
19:29:08 mriedem and it allows null
19:29:16 mriedem oh no sorry
19:29:23 mriedem that says you don't have to put a body in it
19:30:15 melwitt so what's the correct flow for deleting a BFV instance with delete_on_termination=True? has to detach and then delete the volume, the volume can't be deleted if it's in-use
19:31:31 arvindn05 efried: in gerrit, subsequent changes are always amends for a single review which means on the git side, you do lose history. If you wanted to say revert changes back to commit #2, we would need to download the code from gerrit for PS#2...anyway just my rant from using both github and gerrit :)
19:32:23 efried arvindn05: You can do commits in series, which means you use a different change-id for the second commit but set the first commit as its parent.

Earlier   Later