| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-09-21 | |||
| 14:36:29 | gibi | dansmith: I'm looking at the scatter_gather code and I don't think I spotted the part that prevents us to return a generator as a result. | |
| 14:36:51 | gibi | dansmith: I even replaced the list() call with a generator expression in your first patch and that seems to still pass the tests | |
| 14:39:41 | dansmith | gibi: hmm, I initially was returning a generator from it and was hitting something in there that was trying to count the result | |
| 14:40:17 | dansmith | gibi: but I guess maybe I was generating an error in one of the threads and that threw me off | |
| 14:40:27 | dansmith | let me try to change it again locally now that things are all working and see | |
| 14:41:06 | gibi | the only length calulation is here https://github.com/openstack/nova/blob/193729b93a11ff54da99386e881270209797f020/nova/context.py#L439 but that is only checking the number of results not the lenght of each result | |
| 14:42:26 | dansmith | yeah | |
| 14:42:35 | dansmith | hmm, yeah, that seems to work | |
| 14:43:04 | dansmith | I dunno what I was hitting before, but I initially had this so it was generators all the way down, convinced myself that wouldn't work across that thread boundary and then wrote that docstring about it | |
| 14:43:21 | dansmith | so... I dunno, but that was super early, so maybe something else was going on | |
| 14:43:26 | dansmith | I shall change it | |
| 14:45:27 | stephenfin | sdague: Any tips on how to find changes that have not been reviewed by anyone? 'NOT label:Code-Review<=2 age:5d' no longer seems to work | |
| 14:46:01 | stephenfin | This is in relation to Gerrit and your 'dashboard query changes since upgrade' mail (which I've yet to receive :() | |
| 14:46:03 | johnthetubaguy | I think sdague refreshed the openstack dashboard url for the new gerrit version | |
| 14:46:03 | mriedem | dansmith: you won me over on the sort keys/dirs defaults thing in https://review.openstack.org/#/c/504983/ | |
| 14:46:10 | mriedem | i'd be cool with dropping that handling | |
| 14:46:24 | stephenfin | johnthetubaguy: Aye, but I don't think that change is in there. | |
| 14:46:27 | gibi | dansmith: cool | |
| 14:46:47 | johnthetubaguy | stephenfin: ah, fair enough | |
| 14:47:04 | dansmith | mriedem: why not just leave what I have? | |
| 14:47:16 | dansmith | mriedem: you can't pass dirs without keys, and can't pass dirs that don't match keys in terms of length | |
| 14:47:19 | mriedem | you can, it's just kind of dead code as you said yesterday | |
| 14:47:29 | dansmith | yeah, but now I have tests for it | |
| 14:47:34 | mriedem | :/ | |
| 14:47:53 | mriedem | up to you, i'm +2 either way | |
| 14:48:01 | mriedem | are you going to redo this series to make do_query return a generator? | |
| 14:48:24 | dansmith | yeah, I'm doing it now | |
| 14:48:33 | dansmith | I wish I hadjust done it at the end because of the conflict it makes, | |
| 14:48:37 | dansmith | but I'm in the middle now anyway | |
| 14:49:32 | openstackgerrit | Dan Smith proposed openstack/nova master: Add base implementation for efficient cross-cell instance listing https://review.openstack.org/504983 | |
| 14:49:33 | openstackgerrit | Dan Smith proposed openstack/nova master: Make instance_list honor global query limit https://review.openstack.org/504984 | |
| 14:49:33 | openstackgerrit | Dan Smith proposed openstack/nova master: Add db.instance_get_by_sort_filters() https://review.openstack.org/504985 | |
| 14:49:34 | openstackgerrit | Dan Smith proposed openstack/nova master: Support pagination in instance_list https://review.openstack.org/504986 | |
| 14:49:34 | openstackgerrit | Dan Smith proposed openstack/nova master: Add fault-filling into instance_get_all_by_filters_sort() https://review.openstack.org/505391 | |
| 14:49:35 | openstackgerrit | Dan Smith proposed openstack/nova master: Add tests to validate instance_list handles faults correctly https://review.openstack.org/505392 | |
| 14:49:35 | openstackgerrit | Dan Smith proposed openstack/nova master: Add get_instance_objects_sorted() https://review.openstack.org/505417 | |
| 14:49:36 | openstackgerrit | Dan Smith proposed openstack/nova master: Copy some tests to a cellsv1 mixin https://review.openstack.org/505442 | |
| 14:49:36 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix a pagination logic bug in test_bug_1689692 https://review.openstack.org/505661 | |
| 14:49:37 | openstackgerrit | Dan Smith proposed openstack/nova master: Use improved instance_list module in compute API https://review.openstack.org/505418 | |
| 14:49:37 | openstackgerrit | Dan Smith proposed openstack/nova master: Remove legacy fault-loading routines https://review.openstack.org/505456 | |
| 14:49:47 | dansmith | mriedem: gibi: ^ | |
| 14:49:58 | gibi | dansmith: looking... | |
| 14:50:55 | dansmith | gibi: I'm still trying to figure out why that weird raise is .. weird.. but I can stack whatever the fix is on top | |
| 14:53:45 | gibi | dansmith: OK. thanks for the explanation. As you guessed I haven't tried to move the raise in the except block myself | |
| 14:55:27 | gibi | dansmith: It seems you forget to update the code comments when you change the list to the generator expression in https://review.openstack.org/#/c/504983 | |
| 14:55:32 | mriedem | andreykurilin: hey i assume you like to do some scale and performance testing since you work on rally, right? | |
| 14:56:54 | dansmith | gibi: I updated the docstring... did I miss a reference? | |
| 14:57:38 | gibi | dansmith: https://review.openstack.org/#/c/504983/6/nova/compute/instance_list.py | |
| 14:57:43 | dansmith | what in the | |
| 14:57:48 | gibi | dansmith: L69 and L93 | |
| 14:57:58 | dansmith | craaap | |
| 14:58:06 | dansmith | I must have dumped it during a rebase | |
| 14:58:09 | dansmith | urgh | |
| 14:58:12 | dansmith | I _did_ update ;) | |
| 14:58:20 | gibi | I believe you :) | |
| 14:58:23 | openstackgerrit | Elod Illes proposed openstack/nova master: Add instance.interface_attach notification https://review.openstack.org/503089 | |
| 14:58:50 | dansmith | gibi: from my history: https://pastebin.com/u7nUYi38 | |
| 14:58:51 | dansmith | :P | |
| 14:59:20 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Spec: Use keystoneauth1 Adapter for endpoints https://review.openstack.org/500190 | |
| 14:59:31 | efried | mriedem edmondsw ^ | |
| 14:59:52 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Fix hyperlinks in document https://review.openstack.org/506101 | |
| 14:59:54 | edmondsw | efried ack | |
| 15:01:06 | openstackgerrit | Takashi NATSUME proposed openstack/nova master: Fix hyperlinks in document https://review.openstack.org/506101 | |
| 15:01:53 | openstackgerrit | Dan Smith proposed openstack/nova master: Add base implementation for efficient cross-cell instance listing https://review.openstack.org/504983 | |
| 15:01:54 | openstackgerrit | Dan Smith proposed openstack/nova master: Make instance_list honor global query limit https://review.openstack.org/504984 | |
| 15:01:54 | openstackgerrit | Dan Smith proposed openstack/nova master: Add db.instance_get_by_sort_filters() https://review.openstack.org/504985 | |
| 15:01:55 | openstackgerrit | Dan Smith proposed openstack/nova master: Support pagination in instance_list https://review.openstack.org/504986 | |
| 15:01:55 | openstackgerrit | Dan Smith proposed openstack/nova master: Add fault-filling into instance_get_all_by_filters_sort() https://review.openstack.org/505391 | |
| 15:01:56 | openstackgerrit | Dan Smith proposed openstack/nova master: Add tests to validate instance_list handles faults correctly https://review.openstack.org/505392 | |
| 15:01:56 | openstackgerrit | Dan Smith proposed openstack/nova master: Add get_instance_objects_sorted() https://review.openstack.org/505417 | |
| 15:01:57 | openstackgerrit | Dan Smith proposed openstack/nova master: Copy some tests to a cellsv1 mixin https://review.openstack.org/505442 | |
| 15:01:57 | openstackgerrit | Dan Smith proposed openstack/nova master: Fix a pagination logic bug in test_bug_1689692 https://review.openstack.org/505661 | |
| 15:01:58 | openstackgerrit | Dan Smith proposed openstack/nova master: Use improved instance_list module in compute API https://review.openstack.org/505418 | |
| 15:01:58 | openstackgerrit | Dan Smith proposed openstack/nova master: Remove legacy fault-loading routines https://review.openstack.org/505456 | |
| 15:02:08 | openstackgerrit | Matt Riedemann proposed openstack/nova master: api-ref: fix default sort key when listing servers https://review.openstack.org/506227 | |
| 15:02:09 | andreykurilin | mriedem: hi! basically, I like to do great things, so rally is my choice :) but yeah, scale and performance testing are somehow related :) | |
| 15:03:22 | gibi | dansmith: I don't need proofs I have faith :) | |
| 15:03:46 | dansmith | gibi: I need proof for my own sanity :) | |
| 15:03:55 | gibi | :) | |
| 15:04:25 | gibi | yeah, sanity is important | |
| 15:05:42 | mriedem | andreykurilin: ok some of us got talking at the PTG about how we need some scale testing done on nova, especially with multiple cells, and thought you might be interested in doing that, | |
| 15:06:01 | mriedem | especially since godaddy is using cells v1 and presumably has to investigate migrating to cells v2 at some point | |
| 15:06:40 | mriedem | andreykurilin: more specifically, dansmith is working on a series of changes to make listing instances across cells more efficient, | |
| 15:06:48 | mriedem | it would be nice if we could get benchmarks before/after | |
| 15:07:39 | mriedem | i'd also be interested in general in how long it takes to create instances in ocata vs pike, since pike is now doing quotas differently and doing resource claims in the scheduler rather than the computes | |
| 15:08:28 | mriedem | i could do this a bit sloppy with devstack and using the fake virt driver, but that doesn't give me multiple cells | |
| 15:10:06 | andreykurilin | mriedem: so basically, it is quite easy to add scenarios for cells v1/v2 (and generate the load), but the problem is in where to launch it. I'll talk to some folks to try find the place for such research | |
| 15:15:51 | mriedem | bauzas: a few comments inline https://review.openstack.org/#/c/506092/ | |
| 15:16:06 | mriedem | andreykurilin: great, thanks | |
| 15:16:26 | mriedem | we've been trying to get feedback from large cells v1 users for awhile now and so far we don't get much feedback | |
| 15:18:26 | efried | sdague You want me to do anything about https://review.openstack.org/#/c/488137/17/nova/utils.py@1309 ? (There or in a fup?) | |
| 15:22:19 | bauzas | mriedem: k, cool, reading | |
| 15:24:50 | bauzas | mriedem: just a question for just one comment, I'm not sure why we need to disable the source host | |
| 15:25:05 | bauzas | mriedem: because you'd like to make sure you end up in some specific host? | |
| 15:30:26 | dansmith | edleafe: I dunno why, but gerrit is being confusing about the stack of actual code patches for your selection/alternates bit | |
| 15:30:36 | dansmith | edleafe: is this the bottom? https://review.openstack.org/#/c/486215/7 | |
| 15:33:47 | mriedem | bauzas: that's what was reported in the bug for the recreate, | |
| 15:33:54 | mriedem | but i guess that's not really necessary to reproduce the issue | |
| 15:34:07 | mriedem | because the scheduler will compare 1 host to 2 instances and fail | |
| 15:34:14 | mriedem | bauzas: yeah so nevermind that part | |