| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-11-13 | |||
| 13:05:40 | stephenfin | jkulik: np. I'll also add that we're generally very open to adding additional cores, particularly when those cores are focused on a particular area and have competency there, but obviously that competency needs to be demonstrated for some time first | |
| 13:06:00 | jkulik | :+1: | |
| 13:06:30 | sean-k-mooney | well keep in mind the job of a core is primarly to review code not write it | |
| 13:07:30 | sean-k-mooney | so while stephenfin is correct we would be open to adding cores the copendncy we woudl use to make that determination is there review output not there code output | |
| 13:08:07 | jkulik | review output in Nova in general, I guess, not only the vmwareapi driver then? As I guess, that one doesn't see too many patches. | |
| 13:09:17 | sean-k-mooney | sortof both. in general in the past we have expect a general knowladge of the entire codebase but most core have areas or specialisation where they spend most of there time | |
| 13:10:16 | stephenfin | That would be an interesting discussion to have as a community. As you say, that area doesn't see too many patches and there is very little expertise in that area among existing cores. Ensuring we had decent review throughput would be an interesting thing to figure out | |
| 13:10:45 | sean-k-mooney | well part of the expction would be that they would enforece the same feature design crita as we do for the rest of nova | |
| 13:11:06 | sean-k-mooney | in the past vmware have added funcitonlity in the out of tree drive and then tried to upstream it | |
| 13:11:34 | sean-k-mooney | or made changes that normaly woudl requrie spec or blueprints that got little or no discussion | |
| 13:12:08 | sean-k-mooney | so if we were to add cores spcialised in vmware i woudl expoect the process to addign vmware features to be the saem as libvirt/ironic | |
| 13:12:30 | jkulik | hm ... yeah, I'd expect upstream to work like upstream not like downstream :D | |
| 13:12:57 | sean-k-mooney | the problem we have had historically with driver that have out of tree vairiant | |
| 13:13:21 | sean-k-mooney | is they and just enouch funcitonality in the intree driver to enabel them to create the feature they want to have in teh out of tree one | |
| 13:14:03 | jkulik | hm ... having a lot of patches downstream, I can see the point. the VMware driver manages a whole cluster, so things might work quite differently than in the reest of the drivers / Nova in general | |
| 13:14:19 | sean-k-mooney | i was plannign to bring this up next cycle but once the vmware/hyperv dirers were removed i was gong to propose removoing the ablit y to have out of tree dirvers entirly | |
| 13:15:04 | sean-k-mooney | jkulik: we dont offically support out of tree driers in nova but we dont explictly block it either | |
| 13:15:36 | jkulik | I mean, we're having a fork basically, so our patches are not necessarily "out of tree" - they are on top of tree :D | |
| 13:16:00 | stephenfin | yeah, that's how cloudbase were handling things with the hyper-v driver | |
| 13:16:57 | stephenfin | through a combination of (I suspect) lack of upstream review throughput but also lack of investment in the time needed to drive consensus on larger design changes | |
| 13:17:37 | jkulik | yeah, same here. whenever we try to upstream common code (as a basis or start to upstream more), those reviews hang around for a long time. | |
| 13:17:50 | stephenfin | the zVM driver takes a different tack, where the driver is essentially a shim over an external library where all the "real" functionality lives | |
| 13:18:05 | jkulik | since there's noone invovled with the vmware driver upstream, that's fully understandable, but doesn't help in reducing the technical debt either | |
| 13:18:42 | stephenfin | jkulik: then that would be on us. If someone were to start maintaining a CI and the codebase it would be up to us to ensure things aren't hanging around for months | |
| 13:20:32 | jkulik | point being no CI anymore for nearly 2 years. got it. | |
| 13:20:45 | sean-k-mooney | ya so zvm is interesting because i also have not see it run as a third party ci | |
| 13:21:05 | sean-k-mooney | but we did nto mark it as experimental so we need to give them time to correct taht | |
| 13:21:17 | sean-k-mooney | the IBM PowerKVM CI does not test zvm | |
| 13:21:26 | sean-k-mooney | its thesting livbirt on power | |
| 13:22:53 | sean-k-mooney | that siad it may only run wehn you make chagnes to the zvm dirver | |
| 13:23:09 | sean-k-mooney | so we need to submit a test patch and determin if its still functional | |
| 13:25:09 | jkulik | Does it make sense to submit monthly patches just to check the different CIs still running? | |
| 13:25:43 | stephenfin | CIs can be configured to run periodic jobs. That would probably be a better approach | |
| 13:26:00 | stephenfin | and publish the URL where those reports can be found | |
| 13:50:47 | tkajinam | o/ May I ask for reviews to merge https://review.opendev.org/c/openstack/nova/+/900538 to make sqlalchemy-2.x job voting again ? | |
| 14:22:21 | zigo | Also, would be cool to have this one merged: https://review.opendev.org/c/openstack/oslo.cache/+/900158 (ie: oslo.cache leaving sockets to memcached in CLOSE_WAIT state...) | |
| 14:26:48 | opendevreview | Danylo Vodopianov proposed openstack/nova master: Packed virtqueue support was added. https://review.opendev.org/c/openstack/nova/+/876075 | |
| 14:32:02 | sean-k-mooney | zigo: wrong channel | |
| 14:32:29 | zigo | ok :) | |
| 14:32:35 | sean-k-mooney | im not sure about hte while true: | |
| 14:32:43 | zigo | I am. | |
| 14:32:51 | sean-k-mooney | that might never exit | |
| 14:32:53 | zigo | There's a break ... | |
| 14:33:02 | sean-k-mooney | there is only if you get an indexError | |
| 14:33:16 | zigo | Which is going to happen when all connections have been closed. | |
| 14:33:31 | sean-k-mooney | no | |
| 14:33:39 | zigo | queue.pop() will return nothing... | |
| 14:33:49 | zigo | I mean, will do an IndexError no? | |
| 14:33:50 | sean-k-mooney | it returns None by default no? | |
| 14:34:02 | sean-k-mooney | which will cause an AtributeError when we defernce the connection | |
| 14:34:05 | zigo | tkajinam wrote it, he may tell. | |
| 14:34:26 | zigo | while ( conn = self.queue.pop().connection ) != None: | |
| 14:34:26 | zigo | At first, I wrote: | |
| 14:35:05 | sean-k-mooney | https://docs.python.org/3/library/collections.html#collections.deque.pop | |
| 14:35:22 | sean-k-mooney | ok pop does raise indexerror if its empty | |
| 14:35:50 | sean-k-mooney | i was expeciting it to reutrn a default like get on a dict | |
| 14:37:05 | sean-k-mooney | zigo: i still dont partically like useign excptions as contolflow but i see why this is working | |
| 14:38:02 | zigo | I was surprised at first too. Probably this only deserves some added code comments ... | |
| 14:38:16 | sean-k-mooney | zigo: i still feel like it might be better to have a timeout but i see why you want it to wait | |
| 14:38:32 | zigo | A timeout ?!? | |
| 14:38:39 | zigo | You mean wait until the CLOSE_WAIT times out ? | |
| 14:38:57 | zigo | That is a crazy idea. One of our server doesn't even have source port available because of this. | |
| 14:39:08 | zigo | Also, we reach the memcached max open sockets very fast. | |
| 14:39:12 | sean-k-mooney | no | |
| 14:39:26 | sean-k-mooney | i mean perhasp we shoudl not try to clsoe the connections indefintly | |
| 14:40:02 | zigo | If we don't, then there's some remaining sockets in CLOSE_WAIT state. | |
| 14:40:03 | tkajinam | the logic loops over all connections in the queue but it does not retry for a single one | |
| 14:40:16 | sean-k-mooney | oh your right | |
| 14:40:19 | tkajinam | in case a connection closure fails then it leaves that one and process the next | |
| 14:40:24 | sean-k-mooney | if we cant close it its already poped | |
| 14:40:31 | tkajinam | yeah | |
| 14:40:32 | sean-k-mooney | so we will at most try each connection once | |
| 14:40:36 | sean-k-mooney | ok yep | |
| 14:40:45 | sean-k-mooney | so ya i agree the patch is correct then | |
| 14:41:29 | zigo | tkajinam: Are you ok if I add some more comments in your patch? | |
| 14:41:56 | tkajinam | you can though we have number of similar implementations in the other places in that single file | |
| 14:42:15 | tkajinam | anyway this is not the right channel and we can continue it in #openstack-oslo | |
| 14:42:55 | sean-k-mooney | i +1'd it for what its worth | |
| 14:43:07 | sean-k-mooney | and i +2'd the voting job changes | |
| 14:43:20 | tkajinam | sean-k-mooney, thanks ! | |
| 14:48:30 | sean-k-mooney | while true: and excptions as contol flow are redflags for me | |
| 14:48:54 | sean-k-mooney | zigo: so if i see either i try to take a closer look and ensure the will exit in all cases | |
| 14:49:18 | sean-k-mooney | unless there i s a comment explinaing ny this shoudl run forever or simialr | |
| 14:49:43 | zigo | Agreed, it's not very natural. | |
| 14:50:06 | zigo | Though there's a lot of valid cases in this world. | |
| 14:50:29 | zigo | Example: the wait for event loop in every single X-Window app ... :) | |
| 14:54:07 | sean-k-mooney | i alwasy implemnt those in terms of a var that is set to true but can be set to false by the app | |
| 14:54:09 | sean-k-mooney | but yes | |
| 15:06:10 | tkajinam | zigo, as I said earlier the same snippet is used multiple times in the same file. If you add a comment then it should be added to the all places. If you fix it then you should fix all. | |
| 15:06:56 | dvo-plv | hello, hello | |
| 15:07:23 | dvo-plv | I wouldl ike to suggest this spec for review https://review.opendev.org/c/openstack/nova-specs/+/895924 | |
| 15:07:29 | dvo-plv | If you will have a time | |
| 15:07:30 | tkajinam | zigo, hmm. ignore it. it might be the only place where that hack is used for loop | |
| 17:15:21 | opendevreview | Stephen Finucane proposed openstack/nova master: docs: Further tweaks to the CPU models document https://review.opendev.org/c/openstack/nova/+/784066 | |
| 17:22:43 | opendevreview | Robert Franzke proposed openstack/nova stable/zed: fix: server rebuild not working during zed upgrade https://review.opendev.org/c/openstack/nova/+/900811 | |
| 18:49:43 | opendevreview | Elod Illes proposed openstack/nova stable/2023.2: Fix rebuild compute RPC API exception for rolling-upgrades https://review.opendev.org/c/openstack/nova/+/900338 | |
| 18:49:44 | opendevreview | Elod Illes proposed openstack/nova stable/2023.2: Adding server actions tests to grenade-multinode https://review.opendev.org/c/openstack/nova/+/900339 | |
| 19:23:36 | opendevreview | melanie witt proposed openstack/nova stable/yoga: Decorate only Flavor.get_* methods that execute queries https://review.opendev.org/c/openstack/nova/+/900821 | |
| 19:25:15 | opendevreview | melanie witt proposed openstack/nova stable/xena: Decorate only Flavor.get_* methods that execute queries https://review.opendev.org/c/openstack/nova/+/900822 | |
| 19:30:05 | opendevreview | melanie witt proposed openstack/nova stable/yoga: Decorate only Flavor.get_* methods that execute queries https://review.opendev.org/c/openstack/nova/+/900821 | |