| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-18 | |||
| 16:57:51 | zzzeek | jaypipes: so, my assertion is, even if it does deadlock, MySQLs deadlocks are immediate and there should be a lot less of them compared to failed compare-and-swaps right now | |
| 16:58:16 | zzzeek | jaypipes: obviosuly for galera multi master everything goes out the window :) | |
| 16:58:46 | jaypipes | not really. | |
| 16:58:50 | zzzeek | jaypipes: but this is mostly a question of, when MySQL emits UPDATE and we dont use ORDER BY (which is goofy), is it going to scan the rows from the index in the same way each time | |
| 16:59:37 | jaypipes | zzzeek: are you asking if innodb is going to lock record 1, *then* record 2, regardless of if the WHERE statement says IN(1, 2) or IN(2, 1)? | |
| 16:59:45 | zzzeek | jaypipes: yes | |
| 16:59:49 | jaypipes | neither. | |
| 17:00:17 | zzzeek | jaypipes: not exactly , the Q is, will it lock 1 and 2 in the *same* order as another UPDATE statement that refers to the same keys | |
| 17:00:29 | zzzeek | i dont care what the order is, just that it is deterministic | |
| 17:00:39 | jaypipes | there will always be a single lock (a gap lock of (inclusive_low=1, inclusive_high=2) | |
| 17:00:53 | jaypipes | it's not 2 locks. | |
| 17:01:10 | jaypipes | it's one lock, evaluated atomically. | |
| 17:01:20 | zzzeek | jaypipes: OK then what does https://dev.mysql.com/doc/refman/8.0/en/innodb-locking.html#innodb-gap-locks say: "Gap locking is not needed for statements that lock rows using a unique index to search for a unique row. " | |
| 17:01:31 | zzzeek | jaypipes: I tried this without using the PK, and it gap locks | |
| 17:01:36 | zzzeek | jaypipes: add the PK, and it row locks | |
| 17:02:48 | zzzeek | b.c. yes w a gap lock, its crazytown time if you have lots of rows | |
| 17:05:59 | jaypipes | zzzeek: can we back up a sec? I'm a little lost on what you're trying to do in Neutron land. could you link me to the code you are doing/reviewing? | |
| 17:06:37 | zzzeek | jaypipes: sure it's https://review.openstack.org/#/c/635671/ didnt want to pull you into neutron | |
| 17:08:14 | zzzeek | jaypipes: they were ready to add a whole new table and rewrite their whole versioning thing just so they can have incrementing version ids without checking for conflicts | |
| 17:08:43 | zzzeek | jaypipes: as always, if this fails sometimes, it retries. it's not the end of the world. the current implementation fails and retries lots. | |
| 17:13:41 | jaypipes | zzzeek: retries are natural and expected, yes | |
| 17:13:52 | jaypipes | zzzeek: lemme read this patch first :) | |
| 17:14:01 | zzzeek | jaypipes: im going to try the sleep() trick to see if i can get the two updates to interleave | |
| 17:22:30 | openstackgerrit | Jim Rollenhagen proposed openstack/nova master: ironic: check fresh data when sync_power_state doesn't line up https://review.openstack.org/636699 | |
| 17:28:29 | openstackgerrit | Maciej Kucia proposed openstack/nova master: SR-IOV passthrough: Check PF only if VF is enabled https://review.openstack.org/476642 | |
| 17:28:50 | openstackgerrit | Merged openstack/nova master: create_veth_pair is unused, remove it. https://review.openstack.org/624226 | |
| 17:32:43 | openstackgerrit | Elod Illes proposed openstack/nova stable/queens: Make service all-cells min version helper use scatter-gather https://review.openstack.org/637594 | |
| 17:59:49 | jaypipes | zzzeek: hmm, neutron patch is a rabbithole for me... abstraction on top of abstraction. I'm afraid that I'm having a difficult time following what SQL statements would/will be produced at which events with this... | |
| 18:00:13 | zzzeek | jaypipes: I know, neutron is a PITA | |
| 18:00:26 | zzzeek | jaypipes: i never work on it because it is nonsensical | |
| 18:00:35 | zzzeek | jaypipes: but i figured out enough what this does :) | |
| 18:00:37 | openstackgerrit | Elod Illes proposed openstack/nova stable/queens: Make host_manager use scatter-gather and ignore down cells https://review.openstack.org/637599 | |
| 18:02:46 | jaypipes | zzzeek: this patch definitely fixes a number of deadlocks you are seeing, yes? | |
| 18:03:14 | jaypipes | zzzeek: where specifically are those deadlocks being hit? (in the neutron code, that is...) | |
| 18:04:06 | zzzeek | jaypipes: it fixes SQLAlchemy version id misses, not "deadlocks" in the literal sense | |
| 18:04:16 | zzzeek | jaypipes: SQLAlchemy StaleDataErrors | |
| 18:04:55 | zzzeek | jaypipes: they have this test case where they create 300 security group rules | |
| 18:05:15 | zzzeek | jaypipes: and that incurs a versino bump on many related objects because their API requiers that the new number is exposed | |
| 18:05:25 | zzzeek | jaypipes: so the version_id here is used for two purposes | |
| 18:05:57 | zzzeek | jaypipes: so when they make these 300 rules they get a ton of StaleDataErrors on all the related objects, for which they are changing nothing except bumping the version id | |
| 18:06:10 | jaypipes | zzzeek: wait, are you saying that updates of dependent objects cause the version to be incremented more than once? | |
| 18:06:14 | zzzeek | jaypipes: this also incurs that an ORM lookup is happening, a flush, etc. which is all overhead too | |
| 18:06:31 | zzzeek | jaypipes: no, when they have multiple transactions on concurrent API requests | |
| 18:07:20 | zzzeek | script I used to create 300 security group rules asynchronously http://paste.openstack.org/show/744728/" | |
| 18:07:20 | zzzeek | jaypipes: look at feb 8 comments on the gerrit: "POST request response times for 300 security group rules creation http://paste.openstack.org/show/744727/ | |
| 18:08:26 | jaypipes | zzzeek: ok, creating 300 security group rules should increment the security group's version to 301, yes? | |
| 18:08:48 | zzzeek | jaypipes: i dunno. the person testing is saying he is getting the correct results though | |
| 18:08:48 | aspiers | Wow, plantuml is pretty cool. I was able to knock this up pretty quickly from a small text file: https://goo.gl/tPNAQK | |
| 18:09:04 | zzzeek | yes 301 | |
| 18:09:20 | zzzeek | -----------------+ | 2151 | securitygroups | 2019-02-08 09:07:15 | 2019-02-08 09:09:05 | | 301 | +------+----------------+---------------------+---------------------+-------------+-----------------+ | |
| 18:09:20 | zzzeek | Even the revision_number for security group updated properly when creating 300 security group rules MariaDB [neutron]> select * from standardattributes where id="2151"; +------+----------------+---------------------+---------------------+-------------+-----------------+ | id | resource_type | created_at | updated_at | description | revision_number | +------+----------------+---------------------+---------------------+-------------+ | |
| 18:42:42 | mnaser | mriedem: we might want to ping the ML about the change we recently did with disallowing root_gb=0 flavors | |
| 18:43:33 | mnaser | osa gates broke because i assume we created flavors which were using root_gb=0 | |
| 18:43:48 | mriedem | we?!?! | |
| 18:43:50 | mnaser | i can shoot an email if you weren't planning to | |
| 18:43:55 | mriedem | go nuts | |
| 18:44:04 | mnaser | https://review.openstack.org/#/c/603910/ | |
| 18:44:04 | mriedem | i send enough "oh btw everything is going to be broken now" emails | |
| 18:44:08 | mnaser | :D | |
| 18:44:09 | mriedem | yeah i know | |
| 18:59:57 | openstackgerrit | Surya Seetharaman proposed openstack/python-novaclient master: API microversion 2.69: Handles Down Cells https://review.openstack.org/579563 | |
| 19:03:48 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add TargetDBSetupTask https://review.openstack.org/627892 | |
| 19:03:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Execute TargetDBSetupTask https://review.openstack.org/633853 | |
| 19:03:49 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add CrossCellMigrationTask https://review.openstack.org/631581 | |
| 19:03:50 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add prep_snapshot_based_resize_at_dest compute method https://review.openstack.org/633293 | |
| 19:03:50 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add can_connect_volume() compute driver method https://review.openstack.org/621313 | |
| 19:03:51 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add prep_snapshot_based_resize_at_source compute method https://review.openstack.org/634832 | |
| 19:03:51 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add PrepResizeAtDestTask https://review.openstack.org/627890 | |
| 19:03:52 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add finish_snapshot_based_resize_at_dest compute method https://review.openstack.org/635080 | |
| 19:03:52 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add PrepResizeAtSourceTask https://review.openstack.org/627891 | |
| 19:03:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Execute CrossCellMigrationTask from MigrationTask https://review.openstack.org/635668 | |
| 19:03:53 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Add FinishResizeAtDestTask https://review.openstack.org/635646 | |
| 19:03:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Filter duplicates from compute API get_migrations_sorted() https://review.openstack.org/636224 | |
| 19:03:54 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Plumb allow_cross_cell_resize into compute API resize() https://review.openstack.org/635684 | |
| 19:03:55 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add confirm_snapshot_based_resize_at_source https://review.openstack.org/637058 | |
| 19:03:55 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Start functional testing for cross-cell resize https://review.openstack.org/636253 | |
| 19:03:56 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add confirm_snapshot_based_resize conductor RPC method https://review.openstack.org/637075 | |
| 19:03:56 | openstackgerrit | Matt Riedemann proposed openstack/nova master: WIP: Add ConfirmResizeTask https://review.openstack.org/637070 | |
| 19:03:57 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Confirm cross-cell resize from the API https://review.openstack.org/637316 | |
| 19:05:04 | mriedem | that's only about half the series at this point | |
| 19:05:06 | mriedem | swoon away | |
| 19:36:25 | NewBruce | sean-k-mooney still here mate? | |
| 19:41:25 | mordred | mriedem: did nova/devstack introduce a behavior change recentlyish? http://logs.openstack.org/63/637563/1/check/openstacksdk-functional-devstack-tips/f7e749c/testr_results.html.gz | |
| 19:41:45 | mordred | mriedem: | |
| 19:41:47 | mordred | openstack.exceptions.HttpException: HttpException: 403: Client Error for url: https://213.32.77.235/compute/v2.1/servers, Only volume-backed servers are allowed for flavors with zero disk. | |
| 19:41:54 | dansmith | hah | |
| 19:42:02 | dansmith | mordred: [10:42:42] <mnaser>mriedem: we might want to ping the ML about the change we recently did with disallowing root_gb=0 flavors | |
| 19:42:08 | mordred | hahaha | |
| 19:42:23 | dansmith | made it a whole hour | |
| 19:42:31 | mnaser | mordred: http://lists.openstack.org/pipermail/openstack-discuss/2019-February/002842.html :D | |
| 19:43:36 | mordred | mnaser: so - I might need to port that change to apply to non-tempest flavors I suppose? | |
| 19:44:01 | mnaser | mordred: so whatever tests you were doing were trying to do a boot from volume using a flavor that has root_gb=0 | |
| 19:44:14 | mnaser | i assume openstacksdk maybe is trying to create its own flavor? | |
| 19:44:34 | mordred | not to my knowledge, no | |
| 19:44:41 | mordred | we just use whatever flavor is there | |
| 19:44:52 | mordred | but we don't install/run tempest, so I doubt lib/tempest flavors exist | |
| 19:45:44 | mnaser | {"href": "https://213.32.77.235/compute/flavors/c1", "rel": "bookmark"}], "ram": 256, "OS-FLV-DISABLED:disabled": false, "os-flavor-access:is_public": true, "rxtx_factor": 1.0, "disk": 0, "id": "c1", "name": "cirros256", "vcpus": 1, "swap": "", "OS-FLV-EXT-DATA:ephemeral": 0} | |
| 19:45:55 | mordred | mnaser: http://paste.openstack.org/raw/745281/ | |
| 19:46:25 | mordred | mnaser: so I guess I just need to pick a different flavor | |
| 19:46:38 | mnaser | we should probably fix what creates cirros256 | |