Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-25
17:20:21 kashyap ...
17:20:46 kashyap But yeah, that timeout of 'libvirt-guests' looks spurious enough, might as well start over.
17:21:05 sean-k-mooney kashyap: would that not cause filesystem curruption if you did not suspend them on rebooting the host
17:21:06 kashyap (Also, not sure if that paused instance's QEMU process went 'defunct')
17:21:37 efried following up re os-vif and python-novaclient releases: Libs are required to do one release per milestone. os-vif was last released at m1, so we can expect the release team to propose that one. python-novaclient was released a couple weeks ago, so we're probably good on that one.
17:21:41 sean-k-mooney kashyap: im guessing the qemu moniotr process stoped processing messages form libvirt
17:22:47 sean-k-mooney efried: ok there is one think i would like to fix soonish but im only starting on it today
17:22:48 kashyap Yeah, but that doesn't tell us why. It could be any no. of reasons
17:23:33 sean-k-mooney kashyap: yep its proably quicker to kill it and spin up a clean vm
17:23:45 sean-k-mooney if mriedem hits it again we can take another look
17:24:22 kashyap sean-k-mooney: Yeah, on FS corruption, possibly "enterprise distros" would enable it
17:24:56 sean-k-mooney it look like ubuntu just enables it by defualt to be safe by defualt
17:31:23 kashyap sean-k-mooney: RHEL doesn't either, BTW. And one can configure what action 'libvirt-guests' can take on host shutdown
17:31:45 sean-k-mooney ok
17:32:01 sean-k-mooney well that is not related to the issue mriedem was having
17:32:17 sean-k-mooney the issue he was having was that the vm hung
17:32:49 sean-k-mooney and then the linux-guests scipt also hugn on shutdown for the same reason
17:33:09 sean-k-mooney ther eis a timout in the service file if i rememebr correctly it waits for up to 2 minutes
17:33:28 sean-k-mooney and it continue with the system shutdown if it takes longer then that
17:34:06 kashyap I wasn't saying it is related. On your FS corruption: no, it is admin / higher-level tool's responsibility to ensure your guests will quiesce its FS.
17:35:16 kashyap (And yes, there is a timeout: check SHUTDOWN_TIMEOUT in /etc/sysconfig/libvirt-guests)
17:35:17 sean-k-mooney ack
17:35:24 kashyap Default is 5 minutes.
17:35:56 sean-k-mooney ya i have seen it when i have rebooted system in the console output in the past i just noticed it had one but never really look that closely
17:39:23 melwitt dansmith, mriedem: would like to have your review on a change to remove the "last context manager" from the CellDatabases fixture https://review.opendev.org/672604. this came up again while I was working on adding a func test to Kevin_Zheng's multi-cell nova-manage db archive_deleted_rows patch https://review.opendev.org/507486, which has been of high priority interest downstream lately
17:39:30 openstackgerrit Eric Fried proposed openstack/nova master: WIP: Process [compute] in $NOVA_CPU_CONF in nova-next https://review.opendev.org/672800
17:42:42 sean-k-mooney melwitt: by the way has anyone reviewed the unified limits spec for the api subteam? not sure who that would be
17:44:34 efried sean-k-mooney: Isn't gmann "the api subteam"?
17:45:42 sean-k-mooney efried: i guess so? i wasnt sure who was on it. but i didnt want too see that sepc slip through the cracks if they were not about to review it
17:46:01 efried Agree.
17:46:15 melwitt sean-k-mooney: no, not yet. people I usually ask about api stuff are alex_xu, gmann
17:46:16 efried I think I added gmann to that spec for that reason, but not sure if he looked.
17:46:16 sean-k-mooney similarly with the image encryption spec.
17:46:57 efried I'm sort of delegating, like "encouraging" the spec owners to track down whoever is needed.
17:48:09 sean-k-mooney oh the provider yaml spec merged
17:48:31 sean-k-mooney cool i should read the final version
17:49:51 sean-k-mooney johnthetubaguy: melwitt: if ye feel like reviewing a spec that is close https://review.opendev.org/#/c/608696/ im happy to do a little reaching out on behalf of the spec owner :)
18:14:31 dansmith melwitt: okay, I'm generally pretty wary of changing that stuff (or even trying to load enough context to review that). I'm not sure I'll get to that point before I Ieave tomorrow, but...ack :)
18:16:40 openstackgerrit Merged openstack/nova master: Remove 'nova.virt.driver.ComputeDriver.estimate_instance_overhead' https://review.opendev.org/672106
18:18:34 melwitt dansmith: ok, thanks for letting me know. it was tough for me to load the context myself, so I understand. I wanted to ideally have you review since the patch involves ripping out the stuff that you had to add with the _cell_lock
18:20:26 melwitt I think it makes the fixture much simpler, but definitely want to run it by you in case I missed something
18:22:13 dansmith yeah I'm just afraid of it breaking something subtle which we don't find for a couple months and then think we need to fix it by changing the real code when in fact the fixture is too relaxed or something
18:22:26 dansmith but that's just because of how hard it was to get it right in the first place, of course
18:27:00 melwitt oh, you mean something to do with racing tests appearing like real bugs when there's really just an issue with the fixture? yeah, I can understand that concern. as far as I can tell, my proposed patch removes all changing of global state, so I'd think there won't be an issue. but those are famous last words, I know
18:29:44 melwitt if we're too afraid to change the fixture, then we will hopefully be able to accept the multi-cell nova-manage patch's func test not being full coverage because of the faking that the CellDatabases fixture does. my primary objective is to get the multi-cell nova-manage db archive_deleted_rows done
18:31:33 dansmith okay, I'm not sure why that is harder than other cell iteration things we do in tests, but I'd be much more inclined to accept more mockery (since that's really a trivial operation) vs. blocking that on rearchitecting the fixture. But, I haven't looked enough into why that's a problem to say really
18:31:33 melwitt and while working on that, its func test was not failing when it should have been (bug in a patchset), and I found it wasn't failing properly because of the "last context manager" faking in the fixture
18:31:40 dansmith maybe I can try to do that before tomorrow at least
18:34:30 melwitt dansmith: tl;dr is the func test is written correctly and is good, but it did _not_ catch a bug in the proposed multi-cell archive impl because of the faking in the fixture. the fixture auto-targets untargeted database access to the last targeted database or the default database. the former hid the bug in the impl because the fixture auto-targeted something that was not targeted in real life and needed to be targeted in real life
18:34:57 melwitt I hope that makes sense
18:35:02 mriedem i'd also rather figure out why the func tests on the archive patch don't work rather than block on redoing the fixture, but i don't know what the issues were,
18:35:04 mriedem having said that,
18:35:16 mriedem i have a func test in my cross-cell resize series that does db archive on all 3 cells in the test https://review.opendev.org/#/c/651650/22/nova/tests/functional/test_cross_cell_migrate.py
18:35:20 dansmith but the real code sends untargeted stuff to the default db in the config, which is why the fixture does
18:35:36 melwitt mriedem: I did figure it out, it was because of the auto-targeting by "last targeted database" if untargeted
18:36:04 dansmith mriedem: yeah, even still, I'd be happy with just a unit test to make sure that archive is calling archive on all the cell mappings.. it
18:36:09 dansmith is such a trivial op I don't really know that we need much more than that,
18:36:17 mriedem ok i guess i mean "i'd rather figure out an easier way to make the new tests work with the existing fixture"
18:36:24 melwitt dansmith: but the fixture also sends untargeted stuff to the "last targeted db" first. I think it should only send untargeted stuff to the default db
18:36:24 dansmith and we run archive in tempest jobs, which should hit cell0 and cell1 if we make it run all cells
18:36:25 mriedem also,
18:36:29 melwitt (which is what my patch is doing)
18:36:48 mriedem i was going to say - nova-next runs archive_deleted_rows, we can and should make it run on all cells
18:36:54 mriedem which will hit cell0 and cell1 as dansmith said
18:37:03 mriedem so....then we'd have real integration test coverage
18:37:04 dansmith melwitt: everything in the compute node is untargeted though, which is why I don't really know how you can change that globally and have it match the real world
18:37:09 dansmith but.. I haven't read it so I dunno
18:37:11 mriedem of the cli, which is better than the functional stuff anyway
18:37:18 dansmith mriedem: agreed
18:38:03 melwitt dansmith: I think maybe things are getting confused. in the fixture we have two ways of targeting untargeted stuff. one is sending it to the default db (good) and one is sending it to the last db that was targeted (bad IMHO)
18:38:38 mriedem i have run afoul of that latter behavior
18:38:40 dansmith I understand
18:38:41 mriedem agree it's not fun
18:38:53 mriedem but i think i've found ways around that in my multi-cell func testing
18:39:16 melwitt so y'all actually like the last targeted db thing? I'm open to that, just didn't think anyone would think they wanted to keep it
18:39:25 mriedem this one https://review.opendev.org/#/c/641179/
18:39:36 mriedem i remember i was hitting weirdness because of that 'last targeted context' thing as well
18:39:43 mriedem i'm not saying i like it
18:39:52 mriedem but i also don't like redoing the whole thing per se
18:39:56 mriedem when there are maybe other ways
18:40:05 melwitt I'm +1 on the real integration testing, that's fine by me. but I just thought it would make the fixture a lot simpler and less big hiding to remove that bit about "last targeted db"
18:40:12 mriedem it's like touching the old quotas code - i can, but don't want to if i can help it
18:40:40 mriedem w/o looking deep into your change idk
18:40:45 mriedem i wouldn't abandon it,
18:40:50 mriedem but i wouldn't block the other thing on it either
18:40:57 mriedem i'd get the integration testing in nova-next working
18:41:09 melwitt that's fair. it's less like redoing and more like "delete all the self._last_ctxt_mgr" but yeah, when you get around to it, take a look and see if you hate it
18:41:09 mriedem which should be like, 1 line
18:42:22 melwitt don't get me wrong, I'm totally fine with that. as long as it gets tested, I'm happy. I was honed in on trying to make the func test work 100%
18:43:50 melwitt and thought people might be happy to see all the _last_ctxt_mgr stuff deleted from the fixture, no more global state changing, much simpler
18:45:06 melwitt I'll rebase the multi-cell archive patch on top of a different change to add --all-cells to nova-next
19:04:30 mriedem efried: ha http://lists.openstack.org/pipermail/openstack-discuss/2019-July/008037.html
19:25:29 openstackgerrit Merged openstack/nova master: api-ref: touch up the os-services docs https://review.opendev.org/672571
19:33:45 artom That actually went pretty well.
19:45:30 openstackgerrit Matt Riedemann proposed openstack/nova-specs master: Support delete_on_termination in volume attach api https://review.opendev.org/612949
19:45:39 mriedem i cleaned this up ^ it's pretty straight-forward
20:02:28 efried mriedem: approved that. Seems like an easy win.
20:03:19 mriedem ack, i'm cleaning up https://review.opendev.org/#/c/667894/ now
20:04:55 efried mriedem: make all the names match up too if you please
20:05:40 efried mriedem: the bp is at https://blueprints.launchpad.net/nova/+spec/add-user-id-field-to-the-migrations-table

Earlier   Later