| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-06-01 | |||
| 10:15:07 | frickler | bauzas: ha, I think that's the issue, we have "threads=1" for nova-api, but =10 for metadata. hopefully that should be the issue already | |
| 10:15:20 | frickler | thx for the pointers | |
| 10:16:52 | jkulik | on that topic^ how does one deal with longrunning rpc-calls done from the API and the number of processes in uWSGI? It should block a whole process right? Thus making a DoS possible by doing a lot of volume-attachments in parallel (which does an RPC which holds a lock on the Instance) | |
| 10:18:47 | lyarwood | Only the initial part of the attachment is a call, the main part is a cast that should yield | |
| 10:19:02 | lyarwood | https://docs.openstack.org/nova/latest/reference/attach-volume.html | |
| 10:19:44 | lyarwood | I haven't had enough coffee to fully answer the impact of threads=1 on that flow however ;) | |
| 10:20:22 | jkulik | if there's an attachment running already, this takes the Instance lock. then all the other calls from nova-api to nova-compute for reserving the block-device-mapping entry (says for k8s workloads attaching > 10 volumes) will wait on the lock | |
| 10:21:16 | jkulik | so would it be recommended to run nova-api with a high number of threads to counter that? | |
| 10:21:24 | lyarwood | right that instance lock will block the remaining attachments | |
| 10:21:41 | lyarwood | that isn't going to help | |
| 10:21:59 | lyarwood | so you can run additional nova-api processes to avoid the lock on that side | |
| 10:22:10 | lyarwood | but nova-compute also takes an instance lock per attachment | |
| 10:22:20 | lyarwood | and another connect_volume lock within os-brick per compute node | |
| 10:22:42 | jkulik | yes, I'm not worried about nova-compute. I'm worried about some k8s projects starving out other users of nova-api | |
| 10:23:19 | lyarwood | I'm not sure if the instance lock actually blocks anything else from being accepted tbh | |
| 10:23:27 | lyarwood | but I get what you're saying | |
| 10:23:58 | lyarwood | we've not had any reports of n-api rejecting requests with k8s calling into it tbh but it could be possible | |
| 10:25:02 | jkulik | we're still on eventlet with a low number of process and trying to go to uWSGI, it looks like we have to run a huge number of processes to accommodate for that | |
| 10:25:14 | jkulik | or play around with threads=100 or something | |
| 10:26:39 | lyarwood | would you mind writing this up as a bug https://launchpad.net/nova/+bug ? | |
| 10:30:40 | jkulik | if you think that helps, sure. I just wanted to get input on how to run Nova tbh. | |
| 10:31:45 | lyarwood | It's mostly so others in NA can chime in later today with thoughts | |
| 10:32:46 | jkulik | ok. I'll write the problem statement down | |
| 10:34:51 | lyarwood | thanks | |
| 10:41:36 | sean-k-mooney | i generally try to avoid theads like "X is old and broken and Y is new an shiny" but a recent internal one pointed me to https://github.com/tummychow/git-absorb | |
| 10:41:54 | sean-k-mooney | which looks quite interesting | |
| 10:43:22 | sean-k-mooney | has anyone used it? im not sure how well it would work in pratice | |
| 10:43:24 | kashyap | sean-k-mooney: That project is missing a 1-line summary at the top; instead of a long elevator pitch | |
| 10:43:47 | kashyap | It says it is a port of "Facebook's "hg absorb"; which I don't know what it is | |
| 10:43:49 | sean-k-mooney | well its porting a feature of mecurial to git | |
| 10:43:55 | kashyap | "a feature"? | |
| 10:44:01 | sean-k-mooney | yes | |
| 10:44:09 | kashyap | That's what I mean by a simple 1-line summary of the feature :) | |
| 10:44:13 | gibi | sean-k-mooney: "will automatically identify which commits are safe to modify, and which staged changes belong to each of those commits" this sounds dangerous | |
| 10:44:17 | bauzas | sean-k-mooney: well, I trust in Gerrit for this | |
| 10:44:18 | kashyap | Yeah | |
| 10:44:23 | sean-k-mooney | so what it does is allow you to write one commit at the end of a patch chain that fixes all the review feedback | |
| 10:44:36 | bauzas | and I trust reviewers | |
| 10:44:45 | sean-k-mooney | then it will automaticly decomose it into may commits and aplly them as fixup via an interactive rebase | |
| 10:44:57 | kashyap | That sounds quite invasive to me | |
| 10:44:59 | bauzas | if you create a fixup patch, then the CI wouldn't like it | |
| 10:45:08 | gibi | I mean sometimes I cannot figure out correctly what local change goes to which commit in my branch so I don't trust the machine to do that :D | |
| 10:45:15 | sean-k-mooney | oh ya im not sure this is smart to use | |
| 10:45:35 | sean-k-mooney | but it was an interesting idea | |
| 10:45:35 | bauzas | I'd prefer to just continue to have a master branch that works | |
| 10:45:58 | stephenfin | bauzas: I think you're misunderstanding what this does | |
| 10:46:02 | sean-k-mooney | it basically takes each hunk in the current commit and trys to apply it to the oldest commit that does not cause a marge conflict | |
| 10:46:05 | gibi | I have a nice long commit chain already for pps in nova so I can actually test its smartness | |
| 10:46:20 | bauzas | stephenfin: explain me, then | |
| 10:46:55 | stephenfin | you've got e.g. a 15 patch series and changes are required in a few of them | |
| 10:47:14 | bauzas | yeah that's what i understood | |
| 10:47:16 | stephenfin | in the GH pull request workflow, you'd typically fix them all in one go and commit a single "fix all the comments" patch | |
| 10:47:29 | bauzas | yeah, hence my "I love Gerrit" for this | |
| 10:47:51 | stephenfin | this will decompose that single patch and merge the things back into whatever one of the previous 15 patches they belong to | |
| 10:48:05 | kashyap | stephenfin: I see the use-case here, though. But it requires great care when using it to quadruple-check things | |
| 10:48:29 | kashyap | It sounds very nice on paper :) | |
| 10:48:31 | stephenfin | yeah, it's much less useful for Gerrit where you're forced to think in terms of individual commits and use rebase extensively | |
| 10:48:47 | sean-k-mooney | kashyap: yep that is why by default it create the delta as patch that you then can merge with an interactive rebase | |
| 10:48:47 | lyarwood | yeah it's working around the bork'd nature of the PR workflow | |
| 10:48:59 | sean-k-mooney | yep | |
| 10:48:59 | bauzas | stephenfin: okay, then why it would be nice for our, then ? | |
| 10:49:14 | stephenfin | it wouldn't really, but that doesn't make it a bad tool | |
| 10:49:16 | lyarwood | git stash ftw | |
| 10:49:22 | bauzas | stephenfin: we can already provide a new patch per change | |
| 10:49:30 | sean-k-mooney | yep it kidof reminded me of https://docs.openstack.org/infra/git-restack/ | |
| 10:49:38 | stephenfin | and I don't see what impact this would have on a working master branch | |
| 10:49:46 | sean-k-mooney | although a different approch | |
| 10:49:48 | lyarwood | it's useful when you need to fix HEAD~$something up but at pointing at HEAD | |
| 10:49:58 | lyarwood | but are* | |
| 10:49:59 | stephenfin | lyarwood: yup, agree RE: borked PR workflow | |
| 10:50:04 | bauzas | honestly, I don't see *why* I'd need this new tool | |
| 10:50:09 | jkulik | in our GH workflow, we still rebase the changes into every commit even for multi-commit PRs. no tool necessary. git commit --fixup and git rebase -i | |
| 10:50:16 | sean-k-mooney | bauzas: im not saying you do | |
| 10:50:29 | sean-k-mooney | bauzas: just wondering if people had used it | |
| 10:50:32 | bauzas | stephenfin: I don't like it because it means that it's OK to have a patch having bugs | |
| 10:50:46 | bauzas | if the next patch fixes them | |
| 10:50:53 | gibi | lyarwood: I tend to prepare commits top of HEAD and then do an interactive rebase to meld them into the proper origin commit they belong | |
| 10:50:55 | stephenfin | jkulik: yup, which works for a change to a single patch. Trickier if you have changes that affect multiple patches | |
| 10:50:58 | bauzas | of course, you *can* squash both | |
| 10:51:17 | stephenfin | bauzas: sounds like your issue is with the PR workflow rather than this tool :) | |
| 10:51:20 | bauzas | but heh, you can *not* squash, and that's why I dislike | |
| 10:51:23 | lyarwood | gibi: yeah that's another way, I just find stash a little quicker for small things | |
| 10:51:23 | sean-k-mooney | bauzas: that not what the tool is enabling at all though so that is kind of irrelevent | |
| 10:51:25 | gibi | lyarwood: I guess instead of commits I could do stash | |
| 10:51:28 | bauzas | stephenfin: correct | |
| 10:51:30 | stephenfin | or rather the CI systems built on this workflow | |
| 10:51:44 | sean-k-mooney | gibi: i dont like stash because i have lost work that way | |
| 10:51:49 | bauzas | stephenfin: and that's why i said "I prefer the Gerrit workflow for this" | |
| 10:51:51 | gibi | sean-k-mooney: ditto | |
| 10:52:11 | stephenfin | Ah, okay. Given we were talking about the tool, I thought you were comparing the _tool_ to Gerrit | |
| 10:52:17 | jkulik | lyarwood: here's the bug you requested https://bugs.launchpad.net/nova/+bug/1930406 | |
| 10:52:17 | opendevmeet | Launchpad bug 1930406 in OpenStack Compute (nova) "parallel volume-attachment requests might starve out nova-api for others" [Undecided,New] | |
| 10:52:19 | stephenfin | which doesn't really make sense | |
| 10:52:19 | sean-k-mooney | i much prefer to either do an interactive rebase and fix inline or put patches on the end an move them | |
| 10:52:25 | lyarwood | jkulik: thanks | |
| 10:52:32 | bauzas | sean-k-mooney: gibi: well, I use git reflog in this case | |
| 10:52:43 | gibi | for the git absorb thingy, if it can do a smart split of the local changes then it might help me with the commit creation what I do manually with git add -p | |
| 10:52:47 | sean-k-mooney | bauzas: i dont think that works with stash | |
| 10:52:48 | lyarwood | ah yes the get out of jail free card that is reflog | |
| 10:52:59 | sean-k-mooney | if it does good to know | |