| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-10-01 | |||
| 14:21:41 | opendevreview | Ghanshyam proposed openstack/nova stable/stein: DNM: testing jsonschema version fix https://review.opendev.org/c/openstack/nova/+/812142 | |
| 14:23:15 | gibi | gmann: I found on other hits recently about this | |
| 14:25:36 | gibi | gmann: I see that novncproxy says it is invalid / expired token https://storage.bhs.cloud.ovh.net/v1/AUTH_dcaab5e32b234d56b626f72581e3644c/zuul_opendev_logs_c3f/812092/1/check/tempest-full-py3/c3fde5e/controller/logs/screen-n-novnc-cell1.txt | |
| 14:26:27 | gibi | but I don't know why | |
| 14:26:54 | gibi | the token should not be expired based on the timestamps in the nova-api and novnc proxy logs | |
| 14:29:39 | gmann | k | |
| 14:31:48 | gibi | I don't see anything else | |
| 14:34:38 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Enable auto-generation of API DB migrations https://review.opendev.org/c/openstack/nova/+/812144 | |
| 14:34:39 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove legacy placement models https://review.opendev.org/c/openstack/nova/+/812146 | |
| 14:34:39 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove unused build_requests columns https://review.opendev.org/c/openstack/nova/+/812145 | |
| 14:34:40 | opendevreview | Stephen Finucane proposed openstack/nova master: objects: Remove 'bandwidth' fields from notifications https://review.opendev.org/c/openstack/nova/+/812148 | |
| 14:34:40 | opendevreview | Stephen Finucane proposed openstack/nova master: objects: Stop querying the main DB for keypairs https://review.opendev.org/c/openstack/nova/+/812147 | |
| 14:34:41 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove models for removed services, features https://review.opendev.org/c/openstack/nova/+/812150 | |
| 14:34:41 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove models that were moved to the API database https://review.opendev.org/c/openstack/nova/+/812149 | |
| 14:34:42 | opendevreview | Stephen Finucane proposed openstack/nova master: db: Remove nova-network models https://review.opendev.org/c/openstack/nova/+/812151 | |
| 14:34:43 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: objects: Add Instance, InstanceList v3 https://review.opendev.org/c/openstack/nova/+/812153 | |
| 14:34:43 | opendevreview | Stephen Finucane proposed openstack/nova master: WIP: objects: Prepare for Instance, InstanceList v3 https://review.opendev.org/c/openstack/nova/+/812152 | |
| 14:42:35 | stephenfin | FYI, those patches are me stress testing the alembic infrastructure and fixing things I find along the way. Would be useful to merge early in Yoga (i.e. starting next week?) to give us loads of time to resolve any potential fallout :) | |
| 14:48:17 | gibi | stephenfin: ack, good point | |
| 15:06:48 | spatel | Folk, not sure what i should look for but we have same hardware on both cloud but local storage showing different result - https://paste.opendev.org/show/809727/ | |
| 15:09:06 | spatel | I don't have any option configure here on both clouddisk_cachemodes = | |
| 15:12:39 | artom_ | bauzas, around? Does https://bugs.launchpad.net/nova/+bug/1944947 and the fix at https://review.opendev.org/c/openstack/nova/+/810849 qualify as a review-priority bug? | |
| 15:12:55 | bauzas | still there | |
| 15:12:56 | artom_ | Seems pretty important to be able to restart hw:cpu_policy=dedicated instances after an upgrade :) | |
| 15:13:32 | bauzas | artom: any bug seems to me nice to review | |
| 15:13:39 | bauzas | so +1 on R-P labeling | |
| 15:13:48 | artom | priteau, btw, are you good to continue the patch above? ^^ It's basically just missing unit tests | |
| 15:13:57 | artom | Or would you want someone more experienced with Nova to take over? | |
| 15:14:31 | bauzas | well, we can mentor priteau, I'm sure :) | |
| 15:14:35 | priteau | Hi artom. Sorry, been quite busy so I haven't looked into it yet. | |
| 15:14:40 | bauzas | any new contributor is appreciated :) | |
| 15:14:51 | bauzas | (even if pierre isn't exactly new to me :D ) | |
| 15:15:27 | priteau | I can try to add unit tests, if I can't figure it out I'll ask for help | |
| 15:15:30 | artom | bauzas, yep, I'm happy either way, just interested in keeping things moving | |
| 15:16:00 | bauzas | priteau: sure, you are 1 hour away from my usual time, so you can ping me for help | |
| 15:16:09 | bauzas | I could redirect | |
| 15:16:14 | priteau | bauzas: Actually I live in France now ;-) | |
| 15:16:31 | bauzas | heh, I see the French mafia growing | |
| 15:16:59 | priteau | I've actually thought about an issue with the whole migration approach (including the existing function) | |
| 15:17:13 | priteau | It updates the object but not the version number field | |
| 15:17:28 | priteau | Is this likely to be an issue? | |
| 15:18:40 | priteau | With the existing code, a pinned instance started on Ussuri would have a 1.4 InstanceNUMACell object | |
| 15:18:49 | artom | priteau, that's an excellent question, actually. I suspect there's magic in the ovo code that does it automatically? | |
| 15:19:50 | bauzas | well, I need more context | |
| 15:20:21 | dansmith | I'm not sure what the question is | |
| 15:20:26 | bauzas | we only update the object is something remotable is added | |
| 15:20:34 | bauzas | the object version* | |
| 15:20:37 | artom | dansmith, the context is https://review.opendev.org/c/openstack/nova/+/810849/2/nova/objects/instance_numa.py, and the question is, won't that leave the InstanceNUMACell as version 1.4 in the DB, even if we added the pcpuset and migrated to 1.5 | |
| 15:21:10 | bauzas | artom: is the pcpuset attribute a ovo field ? | |
| 15:21:35 | artom | It's a SetOfIntegersField | |
| 15:21:56 | bauzas | lemme find this object | |
| 15:22:00 | bauzas | the whole module is too large | |
| 15:22:11 | bauzas | ok, I see it | |
| 15:22:20 | bauzas | so, yeah this field exists | |
| 15:22:28 | bauzas | why should we bump the object version ? | |
| 15:22:32 | dansmith | artom: you're asking if you deserialize, tweak and re-serialize, if you'll serialize the 1.4 version that came from the original thing? | |
| 15:23:03 | artom | dansmith, aye | |
| 15:23:26 | artom | Tweak specifically by adding the field that was added in the 1.5 version | |
| 15:23:53 | bauzas | artom: then the remote object won't get it | |
| 15:24:08 | dansmith | no, when you deserialize an object, it becomes a 1.5, which is why you have to be compatible with your object changes | |
| 15:24:36 | dansmith | the new field will be unset because it came from a 1.4, but when you re-serialize it, you'll use the current version unless you ask for an older one (like conductor does when it backports changes for you) | |
| 15:24:49 | priteau | I've just looked at a production system with pinned-instances launched on Train, the system now being on Victoria. The InstanceNUMACell objects are actually showing "nova_object.version": "1.6" | |
| 15:25:14 | priteau | So it looks OK | |
| 15:25:23 | bauzas | wait | |
| 15:25:25 | bauzas | you lost me | |
| 15:25:38 | artom | priteau, cool, so you confirmed in a real system the mechanism that dansmith explained \o/ | |
| 15:25:43 | bauzas | who's on 1.5 (or later) and who's on 1.4 ? | |
| 15:25:53 | dansmith | bauzas: Who is on first | |
| 15:26:43 | bauzas | dansmith: that's what I'm asking | |
| 15:26:58 | bauzas | but it looks it works, so meh | |
| 15:27:27 | priteau | 1.4 are pre-Victoria objects. 1.5, actually none because 1.6 was also introduced in Victoria. Any instance launched or restarted on Victoria gets 1.6 (but only for cpu_policy=dedicated, which is the problem) | |
| 15:27:46 | dansmith | artom: I might actually be wrong here, I' | |
| 15:27:47 | priteau | Anyway, there is no problem with the version number, I just wasn't aware of the nova magic | |
| 15:27:50 | dansmith | I'm looking hang on | |
| 15:28:17 | bauzas | priteau: so, you're asking about what would happen with an old compute and a Victoria conductor or compute, I guess | |
| 15:28:47 | bauzas | dansmith explained it very pretty well | |
| 15:28:59 | bauzas | when you serialize, you add the object version | |
| 15:29:45 | bauzas | so any reader getting the serialized object would know the version and creates a new object based on the compatibility methods we provide | |
| 15:30:30 | bauzas | keeping in mind that obj 1.X doesn't know what to do with 1.y if y > x | |
| 15:30:32 | dansmith | soooo, | |
| 15:30:52 | dansmith | I'm actually not sure this behaves the way I thought.. I don't actually know that it matters, but I think it might be different | |
| 15:31:21 | dansmith | it looks like we _will_ preserve the inbound version that we got from the serialized form when we re-serialize, | |
| 15:31:45 | dansmith | which I think we do for the purposes of not up-leveling objects if we're newer than others in the cluster | |
| 15:32:05 | dansmith | i.e. to avoid a new conductor up-leveling an old version that an api sent to it, knowing an old compute will need the old version | |
| 15:32:27 | dansmith | and also I guess to avoid upleveling something in the database when we load/save it if we're not ready to roll everyone forward yet | |
| 15:32:52 | dansmith | but, the behavior you notice is not the version, it's whether or not the field sticks, which is what I was focusing on, and I think it will, even though that doesn't match the version | |
| 15:32:54 | dansmith | meaningm | |
| 15:33:11 | artom | dansmith, wouldn't upleveling not be a problem because we have code to make it compatible to old versions? | |
| 15:33:25 | dansmith | if you store the object with the new field, it will serialize the new field even if the version is wrong, and a newer node will still pull it out because it doesn't know which version the field belongs to | |
| 15:33:30 | artom | So if the conductor uplevels something, then an old compute loads, it will automatically downlevel for that compute? | |
| 15:34:04 | dansmith | artom: well, I picked conductor because it was easy, but imagine two computes talking | |
| 15:35:43 | dansmith | so I'm not actually sure I can explain priteau's observation of the new versions | |
| 15:36:02 | dansmith | that's how I recalled it working, but I can't back that up with references to the code purely in the deserialization/serialization stuff | |
| 15:36:12 | priteau | These instances were live migrated after the Victoria upgrade, could it explain it? | |
| 15:36:22 | dansmith | do we recreate the numa objects as part of another operation? yeah, like live migration | |
| 15:38:17 | dansmith | this is the test that proves that we don't automatically uplevel it: https://github.com/openstack/oslo.versionedobjects/blob/e7b6d52aa4b1b40e68a21a122c09b968d5959b0e/oslo_versionedobjects/tests/test_objects.py#L778-L785 | |
| 15:38:31 | dansmith | which isn't how I remembered it, but re-reading that test jogs my memory about why | |
| 15:40:33 | dansmith | so I think technically that operation should do something like object.VERSION = ObjectClass.VERSION when it goes to save it out | |
| 15:46:47 | artom | So the upleveling happens at serialization then? | |
| 15:46:58 | artom | Or some other operation like live migration, as you've said | |