| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-09-16 | |||
| 11:34:16 | sean-k-mooney | aspiers: the only real feedback i would give is the images are a little hard to read without clicking on them. the commandline ones more then anything else | |
| 11:34:31 | sean-k-mooney | the content looks good to me | |
| 11:42:28 | openstackgerrit | ya.wang proposed openstack/nova master: Fix typor of cpu model when check CPU compatibility https://review.opendev.org/682267 | |
| 12:26:41 | bauzas | FWIW, I'm a bit on and off today, working back on the placement audit command | |
| 12:43:44 | efried | o/ nova! | |
| 12:43:50 | efried | ready for a fun-filled day of rechecks?! | |
| 12:45:08 | sean-k-mooney | at least we are down to 2 series | |
| 12:46:40 | sean-k-mooney | there is 1 patch need for vpmem, + https://review.opendev.org/#/q/topic:bp/cpu-resources+(status:open+OR+status:merged) + 2 pathes for forbiden aggreates i think | |
| 12:46:45 | efried | I'm starting to think there may actually be something wrong. | |
| 12:46:53 | efried | forbidden aggs is in. | |
| 12:47:01 | sean-k-mooney | oh ok | |
| 12:47:07 | sean-k-mooney | with the pmem stuff | |
| 12:47:07 | efried | And i think there's one for numalm, but not an important one | |
| 12:47:11 | efried | pmem and cpu-resources | |
| 12:47:14 | efried | are the big ones | |
| 12:47:32 | sean-k-mooney | i was ingoring the funcitonal tests for numalm | |
| 12:47:37 | efried | right | |
| 12:47:52 | sean-k-mooney | the actully featre has lannded just tests are leaft | |
| 12:48:17 | efried | 13 rechecks on the top cpu-resources patch | |
| 12:48:49 | efried | cascading effect of -2s in the gate from a prior patch failing. | |
| 12:48:51 | efried | mostly | |
| 12:49:05 | sean-k-mooney | ya that was what i was about to say | |
| 12:49:25 | efried | I think we have a race in select_destinations though | |
| 12:49:38 | efried | been seeing quite a few like this https://storage.gra1.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_778/674895/42/check/openstack-tox-py36/7787cd5/testr_results.html.gz | |
| 12:51:56 | sean-k-mooney | maybe or that test is just not deterministic | |
| 12:52:15 | efried | yeah, either way | |
| 12:52:19 | efried | it's not just that one test | |
| 12:52:32 | efried | I saw several similar failures with that same select_destinations unequal | |
| 12:52:48 | efried | I didn't dig all the way into them (lack of appropriate tooling on my phone) | |
| 12:52:51 | sean-k-mooney | all in the fallback tests | |
| 12:52:53 | efried | but thinking maybe I should at this point. | |
| 12:53:06 | efried | no, I remember seeing one in a pre-existing test (I think) | |
| 12:53:15 | efried | i.e. regression | |
| 12:53:39 | sean-k-mooney | we might not be mocking some global state in the tests? | |
| 12:54:06 | sean-k-mooney | that said the fallback code is very new so there might be something there | |
| 12:56:35 | stephenfin | efried: I'm thinking we should stop rechecking the two few patches for now | |
| 12:56:40 | stephenfin | *top | |
| 12:56:45 | efried | whyzat? | |
| 12:56:58 | efried | oo, I have a local repro | |
| 12:57:04 | efried | running the whole test class | |
| 12:57:10 | efried | nova.tests.unit.scheduler.test_scheduler.SchedulerManagerTestCase | |
| 12:57:15 | efried | sean-k-mooney: ^ | |
| 12:57:39 | sean-k-mooney | ok then it is likely either a shared state issue or a bug in the code | |
| 12:57:42 | stephenfin | On account of having to recheck again if anything lower gets in. Better just get those lower ones in | |
| 12:57:55 | sean-k-mooney | but if that repoduces it locally that makes it eaiser to figure out | |
| 12:58:11 | stephenfin | Hmm, I've seen that as well, actually | |
| 12:58:28 | stephenfin | Okay, that sounds like a bug. That's testing the retry logic | |
| 12:59:05 | efried | stephenfin: if it's not affecting queue times -- which it probably isn't at this point -- then it's better to have the top ones kicked out by merge-failed -2s than to have to wait for them to pass the check pipeline again once the lower ones are in, imho | |
| 12:59:34 | stephenfin | that's a fair point | |
| 12:59:35 | sean-k-mooney | efried: im just about to grab lunch but ill dig into it when i get back if ye have not figured it out by then | |
| 12:59:35 | efried | stephenfin, sean-k-mooney: I definitely remember seeing the failure on a patch lower in the series at some point. | |
| 12:59:48 | efried | I don't remember which one now | |
| 12:59:53 | efried | where was that fallback introduced, again? | |
| 13:00:24 | stephenfin | efried: https://review.opendev.org/#/c/671801/ | |
| 13:00:50 | sean-k-mooney | ya thats the patch that updated those tests too | |
| 13:00:59 | stephenfin | intermittently failing tests are the _best_ | |
| 13:01:09 | sean-k-mooney | brb | |
| 13:05:46 | shilpasd | Eric: finally after so many rechecks isoalte agg patches are merged, tnx for your extended support | |
| 13:06:13 | shilpasd | Gibi: tnx for followup patch, review on isolate agg patches | |
| 13:06:43 | efried | shilpasd: Your work on this feature is really appreciated. Please pass our compliments on to your colleagues as well. | |
| 13:08:25 | shilpasd | dansmith: stephenfin: takashin: thanks for review | |
| 13:08:42 | shilpasd | efried: sure thanks | |
| 13:13:16 | efried | stephenfin: First thing that jumps out is test_select_destination_with_pcpu_fallback_disabled, which is a test, is calling test_select_destination_with_4_3_client, which is also a test. I.e. both are going to run "simultaneously", one being a subset of the other. | |
| 13:13:27 | efried | ...which should be fine, as long as neither is doing anything global | |
| 13:14:43 | stephenfin | they don't _look_ like they're doing anything global | |
| 13:18:01 | kashyap | print domain_caps[arch][machine_type]._os.loader.enums | |
| 13:18:06 | kashyap | Oops | |
| 13:18:38 | kashyap | (Disregard that, please) | |
| 13:21:28 | kashyap | efried: Mired in parsing some XML bits and testing :D | |
| 13:21:38 | efried | suuuure | |
| 13:21:40 | kashyap | Not my day today... | |
| 13:23:02 | efried | stephenfin: I agree. I'm looking through the other tests in that class; the problem could just as easily be caused by one of *them* doing something global. | |
| 13:44:33 | mriedem | gibi: a few comments in this bug fix of yours https://review.opendev.org/#/c/666857/ | |
| 13:44:37 | stephenfin | efried: something is messing with flags | |
| 13:44:59 | efried | stephenfin: Because CONF is global mebbe? | |
| 13:45:08 | stephenfin | quite possibly, yeah | |
| 13:45:45 | stephenfin | 'CONF.workarounds.disable_fallback_pcpu_query' is intermittently 'False' despite me setting it to 'True' in the test | |
| 13:48:32 | efried | stephenfin: I can reproduce reliably locally when I skip test discovery, and reliably *not* reproduce when I let discovery happen (which runs the tests slower). So definitely racy. | |
| 13:54:30 | efried | stephenfin: Yeah, self.flags is just CONF.set_override | |
| 13:54:58 | stephenfin | and CONF is global | |
| 13:55:26 | efried | stephenfin: use oslo_config.fixture.Config | |
| 13:55:30 | stephenfin | This is hardly the first time we've got positive and negative tests for something config opt-driven though, is it? | |
| 13:56:08 | sean-k-mooney | well | |
| 13:56:24 | sean-k-mooney | you wont have two tests running at the same time in the same process | |
| 13:56:37 | sean-k-mooney | so its fine to set things with set flags | |
| 13:56:49 | sean-k-mooney | as long as you alreay set the correct state up in the test | |
| 13:56:51 | efried | sean-k-mooney: same process yes, same thread no, right? | |
| 13:56:58 | sean-k-mooney | no | |
| 13:57:09 | artom | Wait, has oslo_config.fixture.Config finally fixed the global CONF problem? I guess the fixture is not per-test, so no. | |
| 13:57:11 | sean-k-mooney | tox runes tests in multiple processes not threads | |
| 13:57:25 | stephenfin | well, stestr | |
| 13:57:57 | efried | artom: ...no, it looks like it still by default uses global CONF. So much for that idea. | |
| 13:58:10 | sean-k-mooney | yes point is each worker is tis own process that runs its own python interpreter | |
| 13:58:19 | sean-k-mooney | and only 1 test is running in each worker at a time | |
| 13:58:34 | sean-k-mooney | so test only share global state with other test that ran in that worker | |
| 13:58:41 | stephenfin | we're using ConfFixture in our base test class in nova/test.py | |
| 13:58:43 | sean-k-mooney | but only one test is running at a time | |
| 14:00:12 | stephenfin | then wtf is happening here? :-D | |
| 14:00:47 | sean-k-mooney | im just back form lunch so i can start runing it in a debugger | |
| 14:01:02 | sean-k-mooney | ye have narrorwed it down to the config option yes? | |