| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-05-02 | |||
| 18:21:44 | jroll | mgagne: what version are you on? | |
| 18:21:49 | mriedem | mitaka | |
| 18:21:56 | mgagne | jroll: the best and greatest Mitaka ;) | |
| 18:22:14 | mriedem | mgagne: do you see a fair number of reschedules? | |
| 18:22:51 | jroll | ah yeah. I suspect in master, a bunch of concurrent boots are just fine | |
| 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 | |