| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-06-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 | |
| 15:12:37 | sean-k-mooney | when people do it manually and get it wrong | |
| 15:12:55 | sean-k-mooney | i honestly dont mind either way to be honest | |
| 15:13:08 | stephenfin | dansmith: in general they're correct, but things break if you've to make any modification to an earlier patch | |
| 15:13:11 | sean-k-mooney | the only thing i find annoying is that gerrit does not do it the way we want | |
| 15:13:35 | dansmith | it also helps me see how far something was backported.. like if I'm looking at something on stein and it has N backport lines, I know that came from stein+N | |
| 15:14:29 | sean-k-mooney | stephenfin: maybe try using git review for backport for a while | |
| 15:14:34 | stephenfin | True, but you have that information in the cherry picks box on the right | |
| 15:14:39 | stephenfin | in the UI, that is | |
| 15:14:44 | dansmith | not in the command line tho | |
| 15:14:44 | dansmith | right | |
| 15:15:19 | stephenfin | fair point | |
| 15:15:32 | stephenfin | sean-k-mooney: in general, there's no issue. I just use 'git cherry-pick -x' | |
| 15:16:03 | stephenfin | but I've been bitten a few times now where I had a stack of stable changes and had to make a tweak to a single patch higher up | |
| 15:16:50 | stephenfin | so you've add the missing mock or the typo in your commit message or whatever else you missed | |
| 15:17:22 | stephenfin | and then you've to go update the cherry-picked lines in every backported patch plus any patch on top in each branch. not fun :) | |
| 15:17:32 | stephenfin | it'd be nice if it was optional | |
| 15:18:10 | stephenfin | as something that's nice-to-have but not really necessary, particularly if it incurs pain | |
| 15:18:34 | stephenfin | *when it incurs pain | |
| 15:21:08 | dansmith | well, my vote is that it's worthwhile bookkeeping | |
| 15:22:19 | stephenfin | Definitely worthwhile but not essential. I'd like to be able to loosen the cuffs on the occasion they start hurting | |
| 15:24:49 | elod | if i'm not mistaken, then gerrit adds the cherry-picked lines correctly, iff the original patch has merged. if the patch is not merged yet, it does not add the cherry-picked line, for logical reasons, as it might not be the one that gets merged | |
| 15:25:14 | stephenfin | elod: Ah, so that's how it makes the decision :) Can't confirm but makes sense | |
| 15:26:09 | stephenfin | Waiting for a patch series to land on stable/X before backporting to the stable/X-1 etc. is less than optional though. It causes unnecessary context switching for both submitter and reviewer, assuming the submitter even remembers to keep backporting things | |
| 15:27:00 | elod | that's true, we had the same discussion in our team :] | |
| 15:27:34 | stephenfin | Ha! I'm about to discuss it downstream too 😇 | |
| 15:27:34 | stephenfin | Ha! I'm about to discuss it downstream too 😇 | |
| 15:27:37 | dansmith | I'd much prefer to have the bookkeeping right and I think it's very unusual that backports to N releases are that urgent | |
| 15:28:25 | elod | otherwise, if a patch needs to be fixed, then all its backports need to be fixed, so that's another extra burden. so there are two sides | |
| 15:29:35 | elod | if we stick to policy and do the backporting branch-by-branch, when a patch has merged, then this is not an issue, though | |
| 15:29:39 | stephenfin | dansmith: so not including the cherry-picked from line would be very unusual too | |
| 15:29:48 | stephenfin | it'd be exceptional, not the norm | |