| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-10 | |||
| 12:26:09 | sean-k-mooney | yes | |
| 12:26:25 | sean-k-mooney | we have a config option to limit the max builds on a host | |
| 12:26:45 | sean-k-mooney | one of the suggestions was to add one for snapshot | |
| 12:27:05 | sean-k-mooney | aarents: just looking at the eventlet docs | |
| 12:27:07 | sean-k-mooney | https://eventlet.net/doc/threading.html#eventlet.tpool.execute | |
| 12:27:21 | sean-k-mooney | so it looks like this will still be a blocking call | |
| 12:27:32 | sean-k-mooney | but it yeild the current greenlet ? | |
| 12:28:04 | sean-k-mooney | e.g. will that internally invoke the function on a native thread | |
| 12:28:07 | sean-k-mooney | yeild | |
| 12:28:19 | sean-k-mooney | then call thread.join at some later point | |
| 12:30:18 | aarents | I understand more like that: coroutine delegate to native thread (block just waiting native thread finish, but in the maintime yeild to other coroutine always) | |
| 12:32:35 | sean-k-mooney | aarents: if that is how it works then it would be ok but the docs are not clear | |
| 12:32:58 | sean-k-mooney | oh i missed the prelude | |
| 12:32:59 | sean-k-mooney | The simplest thing to do with tpool is to execute() a function with it. The function will be run in a random thread in the pool, while the calling coroutine blocks on its completion: | |
| 12:34:30 | aarents | yep | |
| 12:34:38 | aarents | sean-k-mooney: according the nova doc you share, I'm precisely in case "Yielding the thread in long-running tasks" but my issue is: I can't put smart greenthread.sleep(0) call because IO are in glance.client | |
| 12:35:08 | sean-k-mooney | aarents: no that is for internal cases | |
| 12:35:12 | sean-k-mooney | not external io | |
| 12:35:22 | aarents | k | |
| 12:35:53 | sean-k-mooney | if we are computing something expensive we are ment to addd sleeps to yeild | |
| 12:36:22 | aarents | yes, aggree | |
| 12:42:50 | sean-k-mooney | aarents: ok left my toughts in the patch. i would proably be ok with that change if we had the config option to limit the number in parralle | |
| 12:43:27 | stephenfin | bauzas: Last ask for the day, I promise, but could you also take a shot at https://review.opendev.org/#/c/728505/ ? It's a re-approval, if that helps | |
| 12:43:58 | aarents | sean-k-mooney: thanks | |
| 12:44:05 | stephenfin | If you want, will trade for review of your spec :-P | |
| 12:52:32 | openstackgerrit | Merged openstack/nova-specs master: spec update: virtual-persistent-memory https://review.opendev.org/726284 | |
| 13:05:55 | bauzas | stephenfin: heh no worries | |
| 13:28:22 | aarents | sean-k-mooney: dansmith thks, I replied https://review.opendev.org/#/c/734776/1 | |
| 13:32:15 | sean-k-mooney | aarents: i was thinking more like 4 but 10 might be fine too | |
| 13:32:29 | sean-k-mooney | what is the default build limit | |
| 13:32:45 | sean-k-mooney | ah that is 10 | |
| 13:33:03 | sean-k-mooney | ya so 10 would be fine | |
| 13:34:15 | aarents | ok | |
| 13:38:46 | dansmith | 10 seems like a lot to me | |
| 13:39:08 | dansmith | well, depends on where the bottleneck is I guess | |
| 13:39:25 | sean-k-mooney | ya that is why i was thinking 4 but we consider 10 ok for max_concurrent_builds | |
| 13:39:42 | sean-k-mooney | maybe it should default to the value of max_concurrent_builds if its not set | |
| 13:39:46 | sean-k-mooney | to keep them the same | |
| 13:40:13 | sean-k-mooney | we have done that with some of the periodic intervals | |
| 13:46:04 | aarents | sean-k-mooney currently we live without limit by defaut, the new idea is to not exhaust thread pool, in that case 10 is fine but we can align with other parameter why not | |
| 13:46:56 | sean-k-mooney | aarents: well we live without a limit becasue its all in one thread | |
| 13:47:24 | aarents | k | |
| 13:48:05 | sean-k-mooney | update the patch and either have the default depend on the other config value or select a defalut and we can review that then | |
| 13:48:16 | openstackgerrit | Dan Smith proposed openstack/nova master: DNM: Try to make a glance multistore job https://review.opendev.org/734184 | |
| 13:50:15 | aarents | sean-k-mooney: ok | |
| 13:57:00 | sean-k-mooney | if we see Lazy-loading 'pci_devices' on Instance ... mesages on the compute node during spwan is that a bad thing? | |
| 13:57:27 | sean-k-mooney | or more is that expected or should we have all the info in the build request? | |
| 14:17:26 | dansmith | sean-k-mooney: it means something tried to read that field on the instance.. if we're doing that based on something in the request, then it's probably good to lazy-load it, so we're not always passing it regardless of whether or not pci is used | |
| 14:17:43 | dansmith | if we're always looking at that regardless, then the lazy load is just a perf hit and we should pre-join it | |
| 14:18:36 | sean-k-mooney | this was just before the xml was generated for an instance with no pci requetss or numa policy set | |
| 14:19:11 | sean-k-mooney | i might take a look at why its doing that in a bit | |
| 14:19:18 | sean-k-mooney | im currently testing file backed memroy | |
| 14:19:47 | dansmith | in that case, we could either be smarter about when we look at that field to avoid the lazy-load, or we should just pre-join it | |
| 14:25:56 | sean-k-mooney | alex_xu: https://review.opendev.org/#/c/734658/4/doc/source/admin/file-backed-memory.rst@70 i added info about the numa and hugepage behavior | |
| 14:26:48 | sean-k-mooney | numa more or less works although the numa toplogy filter will be looking at the host ram when considering numa affinity not the file backed memory | |
| 14:26:59 | sean-k-mooney | i think i a werid way that is actully the correct thing to do | |
| 14:27:18 | sean-k-mooney | as it will not allow a guests virtual numa node to be large then a hosts numa node | |
| 14:28:47 | sean-k-mooney | libvirt is also passing the correct numa affinity hints to qemu for the files too | |
| 14:29:40 | sean-k-mooney | so the things we could fix are hugepages, and how the local_GB is reported. both of which could be done via existing config options | |
| 14:31:39 | sean-k-mooney | so we can just have one more pathc to document it. in the long run we might want to adress this diffrently. we dont need a code change to do the write thing but i think we should warn when we detect you have not set the config options correctly as the current patch does | |
| 14:40:10 | openstackgerrit | Alexandre Arents proposed openstack/nova master: Snapshot: offload glance upload in a native thread https://review.opendev.org/734776 | |
| 15:02:18 | stephenfin | Can anyone say why we include "cherry-picked from" lines in stable? They seem like a waste of time, particularly when you've to make changes to the backport | |
| 15:02:40 | stephenfin | and Gerrit already does the linking based on change ID | |
| 15:04:35 | sean-k-mooney | convention | |
| 15:04:43 | sean-k-mooney | but other then that its not really neeed | |
| 15:04:54 | sean-k-mooney | also if you do it via gerrit it does it differrently | |
| 15:05:03 | sean-k-mooney | e.g. it only add the first one | |
| 15:05:12 | sean-k-mooney | unless you do the backport on by one i think | |
| 15:05:14 | stephenfin | Looks like it's to work around a Gerrit bug that has since been fixed https://docs.openstack.org/project-team-guide/stable-branches.html | |
| 15:05:27 | stephenfin | cherry-pick -x option includes ‘cherry-picked from …’ line in the commit message which is required to avoid Gerrit bug | |
| 15:05:27 | stephenfin | cherry-pick -x option includes ‘cherry-picked from …’ line in the commit message which is required to avoid Gerrit bug | |
| 15:05:35 | stephenfin | which points to https://code.google.com/p/gerrit/issues/detail?id=1107 | |
| 15:05:52 | sean-k-mooney | its not required by any of our tooling | |
| 15:05:58 | stephenfin | nope | |
| 15:06:00 | sean-k-mooney | upstream or downstrema | |
| 15:06:08 | stephenfin | I propose we stop doing it. It's annoying and often wrong | |
| 15:06:14 | stephenfin | Will bring up in the next team meeting | |
| 15:06:19 | sean-k-mooney | so use -X | |
| 15:06:29 | sean-k-mooney | instead of -x | |
| 15:08:38 | openstackgerrit | Merged openstack/nova-specs master: Encrypted Emulated Virtual TPM https://review.opendev.org/728505 | |
| 15:09:06 | stephenfin | sean-k-mooney: What would -X do? the man page says something about merge strategy | |
| 15:09:33 | sean-k-mooney | git-review -x add the cherry picked line -X does not | |
| 15:10:13 | sean-k-mooney | oh sorry its the other way around | |
| 15:10:20 | sean-k-mooney | -X adds it and -x does not | |
| 15:10:33 | stephenfin | I don't think the -X is related, actually | |
| 15:10:38 | stephenfin | Reading the docs | |
| 15:10:50 | sean-k-mooney | so if you backporting using git review | |
| 15:10:51 | stephenfin | you won't get the line unless you specify -x | |
| 15:10:57 | sean-k-mooney | the workflow is checkout stable/whatever | |
| 15:11:04 | sean-k-mooney | then do git review -X | |
| 15:11:09 | stephenfin | ahhhh | |
| 15:11:12 | sean-k-mooney | to add the cherry picked lins | |
| 15:11:14 | stephenfin | I just use git-cherrypick | |
| 15:11:26 | sean-k-mooney | but if you do -x it does not add them | |
| 15:11:34 | sean-k-mooney | ya i was talkingbaout the arge for git review | |
| 15:11:39 | dansmith | I guess I'm confused.. why is it not helpful? When I'm browsing history at the command line, it's helpful.. otherwise I have to go into gerrit by change-id to find the siblings | |
| 15:11:40 | sean-k-mooney | not git cherry-pick | |
| 15:12:08 | sean-k-mooney | dansmith: im assuming stephne it ac case where it was wrong | |
| 15:12:24 | dansmith | when is it ever wrong? I can't think of a case. | |
| 15:12:31 | melwitt | most of the time, I like the cherry picked lines because it lets me easily see if the changes were picked from N branch to N - 1 branch. but agreed it's a pain when you have to update one | |