Earlier  
Posted Nick Remark
#openstack-nova - 2020-06-10
10:47:49 bauzas in other words, document your code :)
10:48:03 stephenfin Hmm, I _could_ but it does feel like stating the obvious
10:48:03 sean-k-mooney bauzas: so im ok with b if iall it is is removing :<Type>
10:48:19 bauzas sean-k-mooney: me too, just my point
10:48:28 bauzas again, it's just a static check AFAIK
10:48:30 sean-k-mooney i dont think that need to be in a comit or called out anywhere in the chagne
10:48:45 sean-k-mooney bauzas: well its not even a staitc check
10:48:51 sean-k-mooney the py2 jobs will fail
10:49:00 sean-k-mooney it wont parse correctly
10:49:02 stephenfin We'd saying if you use Py3 syntax and your commit message has "Closes-Bug" in it, add this boilerplate
10:49:32 sean-k-mooney stephenfin: i would really like to avoid that
10:49:38 stephenfin I mean, we don't say "this bugfix has o.vo implications so you're not allowed backport it" in a commit
10:49:40 sean-k-mooney we coudld but i dont see the point
10:50:11 sean-k-mooney stephenfin: bauzas we could update the contibutor guide or somehting to call it out i guess
10:50:11 bauzas sean-k-mooney: the point is that I'm afraid stable core reviewers could miss such tiny bits that would break
10:50:23 sean-k-mooney bauzas: but it would not pass ci
10:50:31 sean-k-mooney so it wont get merged at least
10:50:32 bauzas fortunately, in this specific case, agreed, CI won't run
10:50:37 stephenfin Fortunately, as sean-k-mooney points out, this will raise a SyntaxError on python 2.7
10:50:38 sean-k-mooney but they might not know why
10:50:56 stephenfin heh, yeah 0:)
10:51:00 bauzas this
10:51:10 stephenfin how a contributor guide change?
10:51:10 bauzas honestly, I don't want to bikeshed
10:51:12 stephenfin *about
10:51:29 bauzas you know what ? I'll just approve the change
10:51:31 stephenfin Do we have a backporting section in that?
10:51:44 bauzas and then people who backport should just see my comment in the review
10:51:50 stephenfin \o/ WFM too :)
10:52:00 bauzas if they don't, then I'll kill them
10:52:06 bauzas seriously
10:52:50 bauzas stephenfin: but for a second, we unnecesseraly added complexity to the existing code by adding a linter check
10:53:14 bauzas I could have -1d it by saying 'sorry dude, unrelated to the change'
10:53:37 bauzas stephenfin: so, please refrain yourself to add more than what's necessary ;) :D
10:53:46 stephenfin ooooorrr, we're reducing complexity by removing the need to worry about types
10:53:53 sean-k-mooney bauzas: i could argure the opiste
10:54:07 stephenfin let the compute do the work for you
10:54:09 sean-k-mooney if you dont add it and added it in a seperate change that is adding uneesisary overhead in revew
10:54:11 stephenfin *computer
10:54:25 stephenfin agreed
10:54:39 stephenfin if I wasn't changing the function signature, this would be unrelated
10:54:47 stephenfin but because I am, this is a fair change
10:54:54 sean-k-mooney anyway stephen are you going to chekc the hugepage/file backed memeory thing or will i doit
10:55:23 stephenfin sean-k-mooney: If you've time, go for it. I'm going to lunch shortly as I'm in meetings most of this afternoon :(
10:55:28 stephenfin If not, I'll aim to do it before EOD
10:55:46 sean-k-mooney well i want to understand this so that i can figure out how this relates to numa in placmenet
10:56:03 sean-k-mooney so ill try and doit shortly
10:57:06 sean-k-mooney file backed memroy will have to be reported on the ROOT RP
10:57:18 sean-k-mooney since it has no real numa affintiy
10:59:44 gibi I think we can trust the stable cores to see if a code is not py2 compatible
11:00:22 sean-k-mooney the fact that we are not using six anymore will liekly cause more issue then type hints
11:00:34 gibi also if there is such code that valid for py2 and py3 but behaves differently, then during the master review we have to make sure we add enough unit test coverage that will break when the patch is blindy backported to py2
11:00:54 sean-k-mooney yep
11:01:15 gibi in this particular case the interpreter catches the problem
11:01:47 bauzas gibi: sean-k-mooney: what I'm saying is that we somehow opted into thinking more about what we write and what we merge on master now that the ship has sailed
11:01:53 bauzas that's it.
11:01:58 sean-k-mooney list vs iterator/view will be one of the main one for like dict.keys()
11:02:10 stephenfin and unicode/bytes
11:02:20 bauzas previously, we had no brain knobs to do as we were supporting boh
11:02:22 sean-k-mooney in that case its not as bad
11:02:22 bauzas both*
11:02:29 sean-k-mooney as we are sticter in py3
11:02:36 sean-k-mooney but yes
11:02:36 gibi bauzas: sure, we should be aware, but at the same time we should not limit ourselves to write better code on master
11:02:47 bauzas I don't disagree
11:03:15 bauzas I'm just saying we need to ponderate the will of writing nice code with the pragmatism of running such code on different stable releases
11:03:54 bauzas and from someone who sometimes has to backport things down to Newton, I'm thinking about my job for the next 3 years
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

Earlier   Later