| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2026-08-31 | |||
| 08:58:44 | gibi | it might or might not mean that the patches are not rebased yet | |
| 08:59:04 | sean-k-mooney | they have not been rebvased but have been rechecked after the merge | |
| 08:59:45 | opendevreview | Clif Houck proposed openstack/nova master: Parallelize per-node resource updates https://review.opendev.org/c/openstack/nova/+/980679 | |
| 08:59:48 | gibi | does the check pipeline rebases the patch before running the tests? I don't think so | |
| 08:59:58 | sean-k-mooney | i clicked the rebase button in the ui we can see | |
| 09:00:03 | gibi | cool | |
| 09:00:19 | sean-k-mooney | gibi: am it does a speculitive merge with master yes | |
| 09:00:33 | sean-k-mooney | so it wont rebase but it should have the same effect | |
| 09:00:52 | gibi | even the check pipeline? not just the gate? | |
| 09:01:11 | sean-k-mooney | correct | |
| 09:01:31 | gibi | the the locking fix did not fully solved the issue reported in that bug | |
| 09:01:51 | sean-k-mooney | ya so the fix | |
| 09:02:16 | sean-k-mooney | is that we shoudl add database locked to the list of excation that oslo.deb treads as a db deadlock excption | |
| 09:02:26 | sean-k-mooney | then our existing decorators will handel that properly | |
| 09:03:07 | gibi | I'm not sure we want to change the production code due to functional test DB limitations | |
| 09:03:14 | sean-k-mooney | my guess is unless we explcitly flush the db or add a small delay it might take a second for sqlachemy to finsih unlocking the db after the write | |
| 09:03:28 | gibi | could be | |
| 09:03:44 | gibi | sqlite also has some config to retry internall for hits | |
| 09:03:47 | sean-k-mooney | well sqlite is used in production when you use bifrost or standalone ironic | |
| 09:03:51 | gibi | /hits/this/ | |
| 09:04:24 | gibi | they are broken then as our locking is only for functional test | |
| 09:04:44 | sean-k-mooney | well yes but our lockign is for nova :) | |
| 09:04:55 | gibi | ahh so they not using nova with sqlite | |
| 09:04:57 | sean-k-mooney | i mean ironic proper not ironic virt driver | |
| 09:05:16 | gibi | but then I'm not sure how this is relevant :) | |
| 09:05:25 | sean-k-mooney | ya so if you use biforst to manage yoru hardware inventory it uses or can use sqlite as the db | |
| 09:05:51 | sean-k-mooney | the same way that ironic suprot there own jsonrpc implation instead of rabbit for a lighter weight deployment if you want that | |
| 09:06:23 | sean-k-mooney | oh just that the oslo.db change should benift them too | |
| 09:06:26 | gibi | OK, so you say we change oslo.db's list of exceptions to retry on | |
| 09:06:37 | gibi | :) | |
| 09:07:07 | gibi | I still cannot really justify that from nova perspective and the ironic case is something I cannot reproduce | |
| 09:07:08 | sean-k-mooney | i think that would be good in the long term yes but i think we shoudl be able to work aroudn this in the futnctionl tests | |
| 09:07:55 | sean-k-mooney | its just somethign i looked into when sqlite was segfaulting in cybrog | |
| 09:08:44 | sean-k-mooney | as a cleaner way then jsut hte locking | |
| 09:10:08 | sean-k-mooney | but ya if there are other tuneable or we can specficly tell sqlachemy to flush/release the db or something that might be better | |
| 09:10:39 | gibi | https://sqlite.org/c3ref/busy_timeout.html | |
| 09:10:50 | gibi | this could be an SQLite native retry | |
| 09:11:24 | gibi | https://sqlite.org/c3ref/busy_timeout.html | |
| 09:11:33 | gibi | https://sqlite.org/pragma.html#pragma_busy_timeout | |
| 09:13:41 | sean-k-mooney | """The SQLITE_LOCKED result code indicates that a write operation could not continue because of a conflict within the same database connection or a conflict with a different database connection that uses a shared cache. """ | |
| 09:14:20 | sean-k-mooney | so the vs """The SQLITE_BUSY result code indicates that the database file could not be written (or in some cases read) because of concurrent activity by some other database connection, usually a database connection in a separate process. """ | |
| 09:14:30 | opendevreview | Balazs Gibizer proposed openstack/nova master: [functional]Do not patch req-id generation https://review.opendev.org/c/openstack/nova/+/1003024 | |
| 09:14:45 | sean-k-mooney | we are not using processes and we have a per db lock | |
| 09:15:08 | sean-k-mooney | so im no t sure the busy tunabels will help | |
| 09:15:14 | gibi | we use threads | |
| 09:15:18 | sean-k-mooney | are we using the cache file | |
| 09:15:36 | sean-k-mooney | i wonder if we are sharign the cache betwen dbs? | |
| 09:17:18 | gibi | I think it is a separate file per DB but I can probably check by injecting a long see into our tests and see the dir structure | |
| 09:17:35 | sean-k-mooney | ack | |
| 09:20:56 | sean-k-mooney | i think we create a seprate WAL,cache and db fiel per database each in there own temp dir for the test | |
| 09:21:30 | sean-k-mooney | but maybe only the db and wal file are separate | |
| 09:22:48 | gibi | https://paste.openstack.org/show/b8jN0kKZZWRZVUQ11xDP/ I don't see cache files | |
| 09:23:39 | sean-k-mooney | ya it looks like wal mode replaces it | |
| 09:23:49 | sean-k-mooney | and shared_cache mdoe is the legacy version | |
| 09:23:51 | sean-k-mooney | https://sqlite.org/wal.html | |
| 09:24:21 | gibi | we do set WAL | |
| 09:24:30 | sean-k-mooney | There is an additional quasi-persistent "-wal" file and "-shm" shared memory file associated with each database, | |
| 09:24:39 | sean-k-mooney | i was confusing it with the shared memory file | |
| 09:24:49 | gibi | but I don't see the WAL file either :) | |
| 09:25:11 | gibi | maybe it is deleted when the connection closed | |
| 09:27:06 | gibi | yepp | |
| 09:27:10 | gibi | they are there | |
| 09:27:14 | gibi | temporarily | |
| 09:27:18 | gibi | nova-test-api-5dclxobp.sqlite-wal | |
| 09:27:24 | gibi | nova-test-api-5dclxobp.sqlite-shm | |
| 09:27:35 | gibi | but that also means it is a unique file per DB | |
| 09:28:08 | gibi | now if two thread like nova-api and nova-conductor connects to the same api DB I assume they are sharing these | |
| 09:28:31 | sean-k-mooney | ack well if you find something that helps let me knwo https://sqlite.org/pragma.html#pragma_locking_mode might be intersting | |
| 09:28:52 | sean-k-mooney | gibi: they could but i thinkt hat is intended to be supproted | |
| 09:29:02 | gibi | anyhow I will push a patch with a PRAGMA busy_timeout = milliseconds; | |
| 09:29:12 | gibi | that is small and easy | |
| 09:29:24 | gibi | and functional test specific | |
| 09:32:02 | gibi | hm, maybe not. The WAL PRAGMA is persistent for the DB, but the busy_timeout is per connection so I cannot just add it to the same place :/ | |
| 09:32:18 | gibi | looking at oslo.db retry :) | |
| 09:36:51 | sean-k-mooney | the other thing you could try is set the lockign mode to exclusivne and back to normal in our currnet lock code https://sqlite.org/pragma.html#pragma_locking_mode but im not sure if that will really help or not | |
| 09:37:50 | sean-k-mooney | i feel like that hsoudl not be required but ya im goign to go get coffee and step away for a bit | |
| 09:40:19 | gibi | the hardness of this that I have no reproducer so whatever I try I have no proof it actually helps other than merging it and letting is run in the gate | |
| 09:41:12 | gibi | so trying random things is expensive | |
| 09:42:04 | gibi | the oslo.db retry code is pluged in a horrible way. I'm not even sure we use any retry at all by default | |
| 09:43:02 | gibi | we have decorators @oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True) in some places | |
| 09:43:09 | gibi | so we do retry in some cases | |
| 09:43:11 | gibi | not all | |
| 09:43:28 | gibi | so even if I change what to retry on I would need to decorate everything | |
| 09:43:31 | gibi | that is uggly | |
| 09:48:51 | gibi | OK, looking at differently. We have the write lock per DB file. That should serialize all the writes. If I assume it works well, then the only thing that can happen is that the SQLite DB lock is not released when our functional fixture lock is released (as you said at the start) so we could try to flush, close, whatever the connection to help with that DB level lock | |
| 10:12:35 | opendevreview | Merged openstack/nova master: tests: Remove errant CONF mocks https://review.opendev.org/c/openstack/nova/+/1002852 | |
| 10:15:46 | tkajinam | could anyone give 2nd +2 to https://review.opendev.org/c/openstack/nova/+/995370 and https://review.opendev.org/c/openstack/nova/+/995370 to reduce deprecation warnings ? | |
| 10:52:36 | gibi | tkajinam: thanks I +Ad the https://review.opendev.org/c/openstack/nova/+/995370 but you linked that patch twice above so I assuem you have one more patch to look at | |
| 11:03:31 | tkajinam | oh sorry I've bee struggling to make my copy-paste work correctly | |
| 11:03:35 | tkajinam | gibi, https://review.opendev.org/c/openstack/nova/+/1002937 this is the 2nd one | |
| 11:03:47 | tkajinam | gibi, and thanks a lot ! | |
| 11:05:16 | tkajinam | oh wait I found a few remaining warnings | |
| 11:05:39 | tkajinam | will address these in follow-up | |
| 11:20:00 | opendevreview | Takashi Kajinami proposed openstack/nova master: ovo: Drop deprecated item_clas argument https://review.opendev.org/c/openstack/nova/+/1002937 | |
| 11:20:26 | opendevreview | Takashi Kajinami proposed openstack/nova master: ovo: Drop deprecated item_cls argument https://review.opendev.org/c/openstack/nova/+/1002937 | |
| 11:34:13 | gibi | thanks | |
| 12:25:52 | opendevreview | Merged openstack/nova master: Replace deprecated remotable_classmethod https://review.opendev.org/c/openstack/nova/+/995370 | |
| 12:41:11 | opendevreview | Takashi Kajinami proposed openstack/nova master: ovo: Drop deprecated item_cls argument https://review.opendev.org/c/openstack/nova/+/1002937 | |
| 13:39:55 | clif | sean-k-mooney: is there anything I can do to help that perf patch merge? | |
| 14:11:59 | sean-k-mooney | did it fail after the rebase? im on pto today so not really paying attaentio, gibi is lookign at some stablity impovment | |