| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-10 | |||
| 11:04:13 | bauzas | if I can ease myself, I'll certainly do | |
| 11:06:32 | sean-k-mooney | stephenfin: so looking at http://paste.openstack.org/show/794558/ | |
| 11:06:40 | sean-k-mooney | stephenfin: i think we are good on the numa front | |
| 11:09:08 | sean-k-mooney | stephenfin: this is the relevent virsh output http://paste.openstack.org/show/794560/ | |
| 11:09:40 | sean-k-mooney | so libvirt sees the hugepage i have allocated but we are not reporting any hugepages in the host numa_toplogy blob | |
| 11:11:50 | sean-k-mooney | gibi: by the way inital_ram_allocation_ratio still default to 1.5 :( i think we just changed it to 1 in ooo | |
| 11:12:29 | sean-k-mooney | we said we chagned it to 1.0 in the ptg but we did not | |
| 11:12:44 | gibi | sean-k-mooney: yeah, we mentioned 1.0 but I don't remember the context | |
| 11:13:03 | gibi | I do know that the product I helping downstream set it 1.0 directly since Icehose or Mitaka | |
| 11:13:19 | sean-k-mooney | ya and windriver did too | |
| 11:13:55 | sean-k-mooney | maybe we should actully chage the upstream default to 1 but we would have to be very vocal and proably add an upgrade check if we did | |
| 11:15:01 | sean-k-mooney | 16 for our cpus is also nuts as a defalt. anything over about 4 tends to cause issue when the system is loaded | |
| 11:15:58 | gibi | sean-k-mooney: agree about vocalization, but I don't think we should spend too much effort changing this default. | |
| 11:15:59 | sean-k-mooney | chaning initalil_allcoation_raitio actully might be safe as it only matter for new installs | |
| 11:16:40 | sean-k-mooney | gibi: well i think we change it downstream in our product, you change it in yours and windriver and i think mirantis did in theres | |
| 11:16:56 | sean-k-mooney | so if everyone changes the default its proably a bad default | |
| 11:17:23 | sean-k-mooney | gibi: but ya we dont need to spend time on it now | |
| 11:17:35 | sean-k-mooney | i jsut wanted to correct myself | |
| 11:17:54 | gibi | ack, thanks for the correction | |
| 11:42:18 | aarents | Hi nova, | |
| 11:42:45 | aarents | bauzas: gibi FYI, As per new element given during PTG regarding my issue https://bugs.launchpad.net/nova/+bug/1874032 , I confirm that offload snapshot upload in a native thread fix it | |
| 11:42:45 | openstack | Launchpad bug 1874032 in OpenStack Compute (nova) "nova-compute become stuck when doing IO on busy file system" [Wishlist,In progress] - Assigned to Alexandre arents (aarents) | |
| 11:42:50 | aarents | (i'm no more able to get nova-compute hang if file system is busy) | |
| 11:45:45 | sean-k-mooney | stephenfin: actully im going to restart libvirt and test that agian | |
| 11:48:34 | aarents | so I propose some thing here: https://review.opendev.org/#/c/734776/1 dansmith was planning to think about it during PTG | |
| 12:15:02 | sean-k-mooney | aarents: right but didnt we agree not to do that and instead just use a new sepmaphore | |
| 12:17:28 | sean-k-mooney | we have https://github.com/openstack/nova/blob/e954a0569785c941ccb89d31c7b4b847b15b5fc5/doc/source/reference/threading.rst which documents nova threading model | |
| 12:18:04 | sean-k-mooney | there are a few cases where we have to use explcit treading but we avoid it unless absolutly required | |
| 12:25:55 | aarents | sean-k-mooney: thanks for the helpfull link, by new semaphore you mean you mean something limiting max concurrent snapshot ? | |
| 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 | |