| -@gerrit:opendev.org- Clark Boylan proposed: | 00:28 | |
| - [zuul/zuul] 872044: Switch to sqlalchemy 2.0 https://review.opendev.org/c/zuul/zuul/+/872044 | ||
| - [zuul/zuul] 872226: Test with MariaDB instead of MySQL https://review.opendev.org/c/zuul/zuul/+/872226 | ||
| - [zuul/zuul] 872363: Don't nest alembic transactions https://review.opendev.org/c/zuul/zuul/+/872363 | ||
| -@gerrit:opendev.org- Clark Boylan proposed: [zuul/zuul] 872364: Switch our local testing docker-compose to mysql 8.0 https://review.opendev.org/c/zuul/zuul/+/872364 | 00:29 | |
| @clarkb:matrix.org | I think maybe 872363 will fix sqla 2.0. This of course assumes that there aren't other transaction issues | 00:29 |
|---|---|---|
| @clarkb:matrix.org | I suspect that Zuul's use of sessions with a commit() call on context manager exit largely solve that problem for us though. Also I Think there would be warnings about this if we got it wrong. The alembic sitaution was more nuanced so no warning | 00:32 |
| -@gerrit:opendev.org- Ian Wienand proposed: | 00:37 | |
| - [zuul/zuul-jobs] 872258: build-docker-image: fix change prefix https://review.opendev.org/c/zuul/zuul-jobs/+/872258 | ||
| - [zuul/zuul-jobs] 872365: zuul-jobs-test-registry-docker-* : update to jammy nodes https://review.opendev.org/c/zuul/zuul-jobs/+/872365 | ||
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/nodepool] 870430: Add a cache for image lists https://review.opendev.org/c/zuul/nodepool/+/870430 | 00:43 | |
| @iwienand:matrix.org | sigh, i'm guessing this broke yesterday-ish ... https://zuul.opendev.org/t/zuul/builds?job_name=zuul-jobs-tox-linters&project=zuul/zuul-jobs | 00:46 |
| @iwienand:matrix.org | ``` | 00:47 |
| linters: 9174 W commands[3]> python -m ansiblelint --version [tox/tox_env/api.py:428] | ||
| Unable to parse ansible cli version: ansible 2.10.17 | ||
| ... | ||
| Keep in mind that only 2.12 or newer are supported. | ||
| ``` | ||
| @jim:acmegating.com | Clark: it seems like there is a small possibility that two components could attempt to creatio the initial schema with that change, right? (presumably the second will fail and either recover or exit, so i don't expect data loss or corruption, just a non-graceful start) | 00:52 |
| @jim:acmegating.com | * Clark: it seems like there is a small possibility that two components could attempt to create the initial schema with that change, right? (presumably the second will fail and either recover or exit, so i don't expect data loss or corruption, just a non-graceful start) | 00:52 |
| @jim:acmegating.com | Clark: (because we fetch the current version in one transaction, then decide what to do with it in a second one) | 00:53 |
| @clarkb:matrix.org | corvus: yes that sounds right. In our testing environment this is a non issue but I guess if you turned on two schedulers at the same time and tey needed to do a migration you could hit this. Definitely worth thinking about a bit more. | 00:54 |
| @jim:acmegating.com | the migration should be fine, it's actually initialization that would hit this | 00:54 |
| @clarkb:matrix.org | We may need a separate table we can lock with to avoid the EXCLUSIVE lock requirement to create the version table? Of course that may lead to the same problem | 00:54 |
| @clarkb:matrix.org | fwiw this issue may have existed with older mysql and mariadb before since we don't hit the lock conflict? | 00:55 |
| @clarkb:matrix.org | they could both read the version and determine it is insufficient then proceed one after the other in sequence? | 00:55 |
| @jim:acmegating.com | Clark: is there a way we can put these in one transaction? | 00:55 |
| @jim:acmegating.com | that's what the code was intending (and used to) do | 00:56 |
| @clarkb:matrix.org | I don't know | 00:56 |
| @clarkb:matrix.org | I tried putting the get_current_revisions in a subtransaction and that didn't work | 00:56 |
| @clarkb:matrix.org | but maybe I did that wrong (so we'd have a parent transaction, then subtransactions for get_current_revisions and the alembic stamp/migrate) | 00:56 |
| @jim:acmegating.com | is alembic now explicitly creating new transactions within the stamp or upgrade methods? | 00:57 |
| @clarkb:matrix.org | corvus: it always did https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/sql/alembic/env.py#L72-L73 | 00:57 |
| @jim:acmegating.com | er what calls that? | 00:58 |
| @clarkb:matrix.org | corvus: invoking the alembic methods here: https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/sql/sqlconnection.py#L329-L331 I think | 00:59 |
| @clarkb:matrix.org | But ya thats a separate transaction not part of the one we started previously and has a different connection id so the locking gets wedged | 00:59 |
| @jim:acmegating.com | Clark: then maybe we should just call run_migrations directly | 00:59 |
| @clarkb:matrix.org | But also this only seems to affect mysql 8.0 and not 5.7 or mariadb | 01:00 |
| @clarkb:matrix.org | corvus: hrm and let alembic sort it all out internally? | 01:00 |
| @clarkb:matrix.org | I think if we do that then we'd always have to do migrations rather than the sqla create which might be slower in some cases? | 01:00 |
| @clarkb:matrix.org | I think that is why we're doing what we do here. We're trying to avoid going through migrations one after another when we can just create what we need today and set the version appropriately. But ya that should be safer. | 01:01 |
| @jim:acmegating.com | well, i mean, you're saying that upgrade calls run_migrations_online, and that method basically just sets up a connection/context/transactions then calls run_migrations. so if we just call run_migrations inside our transaction, problem solved, right? | 01:01 |
| @jim:acmegating.com | or is the error occurring with stamp? | 01:02 |
| @clarkb:matrix.org | oh sorr I thought you meant just call `alembic.command.upgrade(config, revision, tag=tag)` and don't worry about optimizing | 01:03 |
| @jim:acmegating.com | oh definitely not suggesting that | 01:03 |
| @clarkb:matrix.org | doing it the other way around might work but there is some magic going on with the target_metadata | 01:03 |
| @clarkb:matrix.org | and possibly other things | 01:03 |
| @clarkb:matrix.org | I don't know how safe it is to call alembic that way and whether or not there is a good public interface for running migrations with an already established connection and transaction | 01:04 |
| @clarkb:matrix.org | but yes that is theoretically possible | 01:04 |
| @clarkb:matrix.org | ya I guess we just construct one of these https://alembic.sqlalchemy.org/en/latest/api/runtime.html#the-migration-context (which we are already doing) and call configure on it and then proceed as env.py does? | 01:07 |
| @jim:acmegating.com | i think this is worth looking into. i am pretty sure the condition i'm describing can actually be hit in production. | 01:07 |
| @jim:acmegating.com | and we're already creating a context above, just needs a few more args | 01:08 |
| @clarkb:matrix.org | ya the issue would be a race where some initial zuul runtime db data is deleted by another scheduler showing up? | 01:08 |
| @jim:acmegating.com | i don't think anything is going to be deleted, i just thing the second one is going to crash | 01:08 |
| @jim:acmegating.com | it may actually crash today too, but in a slightly different way | 01:09 |
| @clarkb:matrix.org | hrm ya. And ya I think the transaction there may not be strong enough considering we don't have locking errors? or that may just be a change in msql 8 behavior | 01:09 |
| @clarkb:matrix.org | I think go ahead and -1 it with the concern and I'll see if I can modify _migrate to bypass env.py | 01:10 |
| @clarkb:matrix.org | though I'm not sure how to express the stamp vs migrate | 01:10 |
| @clarkb:matrix.org | both seem to pass through env.py (the test case I used to debug this was stamping not migrating) | 01:10 |
| @jim:acmegating.com | working this out a bit more -- i think even today it would crash, it's just that today it will crash when it tries to obtain the write lock at the start of the stamp call, and with the change it would crash after trying to update the data. | 01:12 |
| @clarkb:matrix.org | https://github.com/sqlalchemy/alembic/blob/rel_1_9_2/alembic/command.py#L671 is where stamp goes through env.py I think | 01:13 |
| @jim:acmegating.com | Clark: actually, look at the calling context a bit more...: https://opendev.org/zuul/zuul/src/branch/master/zuul/driver/sql/sqlconnection.py#L342 | 01:13 |
| @clarkb:matrix.org | so ya its confusing that the thing that doesn't run migrations (stamp) calls the run migrations function. | 01:13 |
| @jim:acmegating.com | we wrap the whole thing in a zk lock. i think we're actually okay. :) | 01:13 |
| @clarkb:matrix.org | oh ha | 01:14 |
| @clarkb:matrix.org | we solved the db locking by using a different db | 01:14 |
| @jim:acmegating.com | yep. | 01:14 |
| @clarkb:matrix.org | that might be worth a comment in _migrate though | 01:14 |
| @clarkb:matrix.org | "this relies on an external locking mechanism etc" | 01:14 |
| @jim:acmegating.com | yeah, i'm not opposed to that. :) | 01:15 |
| @clarkb:matrix.org | ok I can make that update, but first dinner | 01:15 |
| @clarkb:matrix.org | and maybe sleep too and get that up in the morning | 01:16 |
| @jim:acmegating.com | thanks! | 01:17 |
| -@gerrit:opendev.org- James E. Blair https://matrix.to/#/@jim:acmegating.com proposed: [zuul/zuul] 872368: Allow default-ansible-version to be an int https://review.opendev.org/c/zuul/zuul/+/872368 | 01:22 | |
| @iwienand:matrix.org | > <@iwienand:matrix.org> sigh, i'm guessing this broke yesterday-ish ... https://zuul.opendev.org/t/zuul/builds?job_name=zuul-jobs-tox-linters&project=zuul/zuul-jobs | 01:44 |
| one is tox 4.4.3 and one is 4.4.4 | ||
| @iwienand:matrix.org | i don't think it's tox (for once). it happens to me locally too. ansible-lint --version fails | 02:01 |
| @iwienand:matrix.org | afaics they install the same packages | 02:12 |
| @iwienand:matrix.org | https://paste.opendev.org/show/bCi9y98FYZX8h9KWZLz2/ | 02:12 |
| @iwienand:matrix.org | fail -> https://zuul.opendev.org/t/zuul/build/89548c8389434649a4c4193fbd3ce87b/console | 02:12 |
| @iwienand:matrix.org | pass -> https://zuul.opendev.org/t/zuul/build/5c93155120f244e9890a860591d4d08a/console | 02:13 |
| @iwienand:matrix.org | ... i think it's https://github.com/ansible/ansible-compat/releases/tag/v3.0.0 ... | 02:24 |
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/zuul-jobs] 872371: linters-requirements : update Ansible to 2.12 https://review.opendev.org/c/zuul/zuul-jobs/+/872371 | 02:47 | |
| -@gerrit:opendev.org- Ian Wienand proposed: | 05:14 | |
| - [zuul/zuul-jobs] 872371: linters-requirements : update Ansible to 2.12 https://review.opendev.org/c/zuul/zuul-jobs/+/872371 | ||
| - [zuul/zuul-jobs] 872365: zuul-jobs-test-registry-docker-* : update to jammy nodes https://review.opendev.org/c/zuul/zuul-jobs/+/872365 | ||
| - [zuul/zuul-jobs] 872258: build-docker-image: fix change prefix https://review.opendev.org/c/zuul/zuul-jobs/+/872258 | ||
| -@gerrit:opendev.org- Ian Wienand proposed: [zuul/zuul-jobs] 872375: container-roles-jobs: Update tests to jammy nodes https://review.opendev.org/c/zuul/zuul-jobs/+/872375 | 05:27 | |
| -@gerrit:opendev.org- Viktor Nikolovski proposed: [zuul/nodepool] 872318: Quota support for Openshift/OpenshiftPod drivers https://review.opendev.org/c/zuul/nodepool/+/872318 | 07:39 | |
| -@gerrit:opendev.org- Marvin Becker proposed wip: [zuul/nodepool] 867971: Add additional pod specs to openshift driver https://review.opendev.org/c/zuul/nodepool/+/867971 | 07:42 | |
| -@gerrit:opendev.org- Marvin Becker proposed wip: [zuul/nodepool] 871060: Add volumes to openshift pod workers https://review.opendev.org/c/zuul/nodepool/+/871060 | 07:43 | |
| -@gerrit:opendev.org- Viktor Nikolovski proposed: [zuul/nodepool] 872318: Quota support for Openshift/OpenshiftPod drivers https://review.opendev.org/c/zuul/nodepool/+/872318 | 09:57 | |
| -@gerrit:opendev.org- Per Wiklund proposed: [zuul/nodepool] 871102: Introduce driver for openshift virtualmachines https://review.opendev.org/c/zuul/nodepool/+/871102 | 10:19 | |
| -@gerrit:opendev.org- Marvin Becker marked as active: | 10:22 | |
| - [zuul/nodepool] 871060: Add volumes to openshift pod workers https://review.opendev.org/c/zuul/nodepool/+/871060 | ||
| - [zuul/nodepool] 867971: Add additional pod specs to openshift driver https://review.opendev.org/c/zuul/nodepool/+/867971 | ||
| -@gerrit:opendev.org- Per Wiklund proposed: [zuul/nodepool] 871102: Introduce driver for openshift virtualmachines https://review.opendev.org/c/zuul/nodepool/+/871102 | 10:42 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 794854: Test zuul-client console-stream https://review.opendev.org/c/zuul/zuul/+/794854 | 11:58 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul-client] 759838: Add change-status subcommand https://review.opendev.org/c/zuul/zuul-client/+/759838 | 12:06 | |
| -@gerrit:opendev.org- Simon Westphahl proposed: [zuul/zuul] 872405: Prevent files cache ltime from going backward https://review.opendev.org/c/zuul/zuul/+/872405 | 12:19 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul-client] 771853: Add show running-jobs subcommand https://review.opendev.org/c/zuul/zuul-client/+/771853 | 12:44 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul-client] 835319: Add dequeue-all subcommand https://review.opendev.org/c/zuul/zuul-client/+/835319 | 13:06 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 794854: Test zuul-client console-stream https://review.opendev.org/c/zuul/zuul/+/794854 | 13:07 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 794854: Test zuul-client console-stream https://review.opendev.org/c/zuul/zuul/+/794854 | 13:14 | |
| -@gerrit:opendev.org- Viktor Nikolovski proposed: [zuul/nodepool] 872318: Quota support for Openshift/OpenshiftPod drivers https://review.opendev.org/c/zuul/nodepool/+/872318 | 14:33 | |
| @buktop:matrix.org | Maybe a stupid question but i was curious if there is a way to run somehow those tests locally? | 14:34 |
| @clarkb:matrix.org | Yes, the nox python jobs are more straightforward than the openshift functional job though. Running this script should set up zookeeper and zookeeper ssl auth: https://opendev.org/zuul/nodepool/src/branch/master/tools/test-setup-docker.sh then you run `nox -s tests` | 14:43 |
| @buktop:matrix.org | great, Thank you | 14:45 |
| @clarkb:matrix.org | The openshift functional job deploys an actual openshift 3 cluster and configures zookeeper to talk to it. I'm less familiar with that process, but I think if you have a CentOS 7 machine you can run openshift 3 and point the nodepool to it. | 14:45 |
| @buktop:matrix.org | At the moment my struggle was only with the nox tests, but will keep a note from this in case someone else needs it later. | 14:48 |
| @clarkb:matrix.org | * The openshift functional job deploys an actual openshift 3 cluster, zookeeper, and configures nodepool to talk to that openshift. I'm less familiar with that process, but I think if you have a CentOS 7 machine you can run openshift 3 and point the nodepool to it. | 14:48 |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 794854: Test zuul-client console-stream https://review.opendev.org/c/zuul/zuul/+/794854 | 14:54 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 794854: Test zuul-client console-stream https://review.opendev.org/c/zuul/zuul/+/794854 | 14:55 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 794854: Test zuul-client console-stream https://review.opendev.org/c/zuul/zuul/+/794854 | 15:26 | |
| -@gerrit:opendev.org- Matthieu Huin https://matrix.to/#/@mhuin:matrix.org proposed: [zuul/zuul] 810955: GUI: Add tenant dropdown to top menu https://review.opendev.org/c/zuul/zuul/+/810955 | 15:44 | |
| -@gerrit:opendev.org- Clark Boylan proposed: | 17:19 | |
| - [zuul/zuul] 872363: Don't nest alembic transactions https://review.opendev.org/c/zuul/zuul/+/872363 | ||
| - [zuul/zuul] 872044: Switch to sqlalchemy 2.0 https://review.opendev.org/c/zuul/zuul/+/872044 | ||
| - [zuul/zuul] 872226: Test with MariaDB instead of MySQL https://review.opendev.org/c/zuul/zuul/+/872226 | ||
| @clarkb:matrix.org | corvus: ^ now with a comment | 17:19 |
| @jim:acmegating.com | Clark: thanks! | 17:26 |
| -@gerrit:opendev.org- Clark Boylan proposed: [zuul/zuul] 872433: Drop python warnings from test suite https://review.opendev.org/c/zuul/zuul/+/872433 | 17:31 | |
| -@gerrit:opendev.org- Zuul merged on behalf of Simon Westphahl: [zuul/zuul] 871814: Initialize missing pipeline change list attribute https://review.opendev.org/c/zuul/zuul/+/871814 | 17:36 | |
| @jpew:matrix.org | Are any Zuul people going to be at FOSDEM? | 22:15 |
| @jim:acmegating.com | ttx mhu sometimes go; pinging them for input; i won't be able to attend this year :( | 22:59 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!