| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-09-11 | |||
| 11:57:08 | frickler | so 5.1.0..5.3.0 gives a better idea of what happened in master. though iiuc the issue is in 5.0.0..5.1.0 | |
| 11:58:29 | frickler | 94b9dc3 is 5.0.1, ack | |
| 11:59:20 | sean-k-mooney | so with that calls to LOG.error are internally using os.write | |
| 11:59:32 | sean-k-mooney | presumably its realted to the lock its taking | |
| 11:59:44 | sean-k-mooney | here https://github.com/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17#diff-2c76d41c287653560e6e84f39ce877b4f9ba33c7b36db17194e7435cd54adfb0R41 | |
| 12:00:58 | sean-k-mooney | its this call on release | |
| 12:01:04 | sean-k-mooney | https://github.com/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17#diff-2c76d41c287653560e6e84f39ce877b4f9ba33c7b36db17194e7435cd54adfb0R108 | |
| 12:01:09 | sean-k-mooney | line 108 | |
| 12:01:19 | sean-k-mooney | that is causing teh test failure sepcifically | |
| 12:01:50 | sean-k-mooney | the next commit maks this conditionaly | |
| 12:01:52 | sean-k-mooney | https://github.com/openstack/oslo.log/commit/de615d9370681a2834cebe88acfa81b919da340c | |
| 12:02:12 | sean-k-mooney | so i guess there are two ways to fix this | |
| 12:02:25 | sean-k-mooney | i could proably disable this fix in this test | |
| 12:02:35 | sean-k-mooney | or i might be able to mock the lock | |
| 12:03:32 | sean-k-mooney | the issue is we have a StandardLogging fixture in our base test clase which handles all or most of the loging config | |
| 12:03:50 | sean-k-mooney | i can also mock the LOG var on the moduel i guess | |
| 12:04:28 | sean-k-mooney | we cocationally do that so i would just replace LOG with a mock explictly | |
| 12:05:13 | elodilles | so this issue only comes up in nova's tests, but could not cause any problem outside the test code, you mean? | |
| 12:07:03 | sean-k-mooney | its not breaking any real code | |
| 12:07:25 | sean-k-mooney | its just breaking the test case as we are mocking os.write and testign a function that uses it directly | |
| 12:07:40 | sean-k-mooney | and oslo.log is not ment ot driectly call os.write | |
| 12:07:47 | sean-k-mooney | or at least id did not previously | |
| 12:08:13 | sean-k-mooney | the "eventlet logging fix" patch add a loc that internally calls os.write | |
| 12:10:14 | elodilles | hmmmm, i see | |
| 12:11:32 | sean-k-mooney | self.useFixture(fixtures.MonkeyPatch("nova.privsep.utils.LOG", mock.Mock())) | |
| 12:11:36 | sean-k-mooney | that shoudl be the fix | |
| 12:11:55 | sean-k-mooney | ya that works | |
| 12:12:04 | sean-k-mooney | ill push that up as a patch | |
| 12:27:06 | elodilles | sean-k-mooney: thanks! \o/ | |
| 13:24:02 | opendevreview | sean mooney proposed openstack/nova master: adapt to oslo.log changes https://review.opendev.org/c/openstack/nova/+/894538 | |
| 13:24:36 | sean-k-mooney | frickler: elodilles: bauzas: ^ that should fix that specific issue | |
| 13:24:56 | bauzas | just saw the patch | |
| 13:25:50 | sean-k-mooney | there may be more generic ways to adress this but i dont really want do do anything more invaisve or global right now | |
| 13:26:22 | bauzas | sean-k-mooney: I wish oslo.log would have had an upgrades relnote | |
| 13:26:50 | sean-k-mooney | well they were not awre i guss that we were sentiive to things like calls to os.write | |
| 13:26:57 | sean-k-mooney | it does not rbeak runtime code | |
| 13:27:09 | sean-k-mooney | with that said i need to test this with our functional tests as well | |
| 13:27:15 | sean-k-mooney | so im going to do that now | |
| 13:28:54 | bauzas | cool | |
| 13:53:33 | opendevreview | Merged openstack/python-novaclient stable/2023.2: Update TOX_CONSTRAINTS_FILE for stable/2023.2 https://review.opendev.org/c/openstack/python-novaclient/+/894076 | |
| 14:22:02 | sean-k-mooney | bauzas: so the functional test also work find with 5.3.0 | |
| 14:22:16 | sean-k-mooney | so it really was just thowse two speicifc unit tests that were impacted | |
| 14:50:41 | bauzas | I voted +2 | |
| 14:51:24 | bauzas | folks (esp. cores), I have a lot of changes that need to be merged before RC1 : https://etherpad.opendev.org/p/nova-bobcat-rc-potential | |
| 14:52:19 | bauzas | at least : https://review.opendev.org/c/openstack/nova/+/893742 https://review.opendev.org/c/openstack/nova/+/893744 https://review.opendev.org/c/openstack/nova/+/893749 | |
| 14:53:27 | bauzas | dansmith: sean-k-mooney: gmann: ^ | |
| 14:56:07 | dansmith | bauzas: yep I saw, I'll work through the etherpad when I'm done with my current thing | |
| 14:56:19 | bauzas | ++ | |
| 15:10:05 | sean-k-mooney | stephenfin: can you review https://review.opendev.org/c/openstack/nova/+/894538 | |
| 15:10:13 | stephenfin | sure | |
| 15:11:10 | sean-k-mooney | bauzas: ill start look at those now to | |
| 15:11:32 | stephenfin | sean-k-mooney: is the print intentional? Do we need it? | |
| 15:11:33 | sean-k-mooney | did we really have no api microversion this cycle | |
| 15:11:39 | sean-k-mooney | ok i tought we did | |
| 15:12:02 | bauzas | sean-k-mooney: no, we haven't, neither any RPC change | |
| 15:12:17 | bauzas | it was a very smooth release in terms of upgrade | |
| 15:12:59 | sean-k-mooney | this is both good and bad for the first slurp release | |
| 15:13:53 | sean-k-mooney | stephenfin: no it was not good catch | |
| 15:14:17 | stephenfin | cool, can +2/+W once it's respin | |
| 15:14:20 | stephenfin | *respun | |
| 15:14:26 | bauzas | sean-k-mooney: I thought your print was needed | |
| 15:14:39 | sean-k-mooney | i can add a log for it | |
| 15:14:44 | sean-k-mooney | that was me just debuging things | |
| 15:15:02 | sean-k-mooney | the way we use pbr does not work with the debugger in vscode | |
| 15:15:05 | bauzas | cool, just respin then | |
| 15:15:17 | bauzas | hah | |
| 15:15:37 | bauzas | I personnally rather pdb with an entrypoint I add | |
| 15:15:48 | bauzas | but I understand your ask for walking thru | |
| 15:16:12 | opendevreview | sean mooney proposed openstack/nova master: adapt to oslo.log changes https://review.opendev.org/c/openstack/nova/+/894538 | |
| 15:17:45 | stephenfin | thanks | |
| 15:26:47 | sean-k-mooney | bauzas: i mentioned this to you before but can we also try to land https://review.opendev.org/c/openstack/nova/+/860829 today | |
| 15:33:03 | bauzas | sean-k-mooney: this is just changing the way we query, right? | |
| 15:33:39 | bauzas | my only concern is how much it would impact the SQL query plan | |
| 15:33:53 | sean-k-mooney | we need to do it anyway | |
| 15:33:57 | bauzas | and what would be the impact in terms of performance | |
| 15:34:14 | bauzas | sean-k-mooney: because of SQLA 3, right? | |
| 15:34:34 | bauzas | S/3/2 | |
| 15:34:39 | sean-k-mooney | for 2 yes | |
| 15:34:52 | sean-k-mooney | we are changing the way we describe the join | |
| 15:35:00 | sean-k-mooney | but it should not matiraly chagne the queary | |
| 15:35:04 | bauzas | that, I understood | |
| 15:35:30 | bauzas | ideally, if the execution plan isn't changing, that's a no-brainer | |
| 15:35:40 | bauzas | but I don't know this | |
| 15:35:55 | sean-k-mooney | its been a while since i read the docs but im not expecting it to mofy the executed SQL at all | |
| 15:37:01 | sean-k-mooney | i read https://docs.sqlalchemy.org/en/14/orm/backref.html and stephens blog before reviewing https://that.guru/blog/sqlalchemy-relationships-without-foreign-keys/ | |
| 15:37:32 | bauzas | I hope you understand my concern : if we merge anything so far in the cycle that would impact the query time on some large and frequently called records like instance or instance info cache, operators would jump on our throats | |
| 15:38:04 | bauzas | stephenfin: around ? | |
| 15:38:06 | sean-k-mooney | bauzas: i do but i dont think that is a reason not to merge this | |
| 15:38:20 | sean-k-mooney | no ne just left to head into the city | |
| 15:38:27 | sean-k-mooney | he will be around tomorrow | |
| 15:38:29 | bauzas | kk | |
| 15:38:50 | sean-k-mooney | we can chat about it in the team meeting tomrowow if you like | |
| 15:39:02 | bauzas | sean-k-mooney: and I assume you want to address this in this cycle so that the SQLA 2 bump is considered 100% complete for Bobcat ? | |
| 15:39:10 | sean-k-mooney | yes | |
| 15:39:30 | sean-k-mooney | i want that to be one of our cycle highlights for what its worth | |
| 15:39:48 | bauzas | in the prelude if you want | |
| 15:40:09 | bauzas | cycle highlights are tend to be marketing-readable | |
| 15:40:18 | sean-k-mooney | well i would be happy even if its in the normaly release notes since all of the above means we finsihed it :) | |
| 15:40:44 | bauzas | I mean, don't get me wrong | |
| 15:40:48 | bauzas | this looks a quickwin | |