Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2023-09-11
11:47:19 sean-k-mooney im going to cloen oslo.log and pip install -e it into the env
11:47:25 sean-k-mooney and i guess do a biset effectivly
11:48:51 frickler so this whole issue is 6 months old and got successfully ignored? cool
11:49:42 sean-k-mooney no idea really. its only causing two test to fail
11:49:51 sean-k-mooney so i dont think its actully breaking real code
11:50:15 sean-k-mooney however im not parcarlly happy with just modifying the test without understadnign why this is breakign
11:50:27 sean-k-mooney as it really not obvious why this is happeining
11:50:58 sean-k-mooney and looking at the patch delta i woudl not expect this type of change between 5.2.0 and 5.3.0
11:53:25 elodilles stephenfin: do you have any idea what could be a solution for this oslo.log issue? ^^^
11:56:34 sean-k-mooney so its broken by 94b9dc3
11:56:40 frickler sean-k-mooney: 5.2.0 is a bugfix release only done on stable/2023.1, 5.1.0 is the initial release for 2023.1
11:56:51 sean-k-mooney https://github.com/openstack/oslo.log/commit/94b9dc32ec1f52a582adbd97fe2847f7c87d6c17
11:56:52 elodilles yepp
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 ?

Earlier   Later