| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Transform metrics.update notification https://review.openstack.org/580567 | 00:23 |
|---|---|---|
| openstackgerrit | Merged openstack/nova master: update project/user for consumer in allocation https://review.openstack.org/581139 | 00:53 |
| *** edmondsw has joined #openstack-placement | 01:05 | |
| *** edmondsw has quit IRC | 01:09 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Update some placement docs to reflect modern times https://review.openstack.org/581151 | 01:33 |
| openstackgerrit | Merged openstack/nova master: unquiesce instance after quiesce failure https://review.openstack.org/550865 | 01:34 |
| openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Use hard coded values in schema than reference https://review.openstack.org/581288 | 01:34 |
| *** yikun has joined #openstack-placement | 01:52 | |
| *** mriedem has left #openstack-placement | 02:00 | |
| *** mriedem has joined #openstack-placement | 02:01 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy to InstanceGroup object https://review.openstack.org/563375 | 02:07 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Fix TypeError in prep_resize allocation cleanup https://review.openstack.org/581548 | 02:13 |
| openstackgerrit | Brin Zhang proposed openstack/nova master: Add unshelve instance error info to fault table https://review.openstack.org/579747 | 02:15 |
| *** tetsuro has joined #openstack-placement | 02:15 | |
| openstackgerrit | Lei Zhang proposed openstack/nova master: Add method to get cpu traits https://review.openstack.org/560317 | 02:25 |
| *** mriedem has quit IRC | 02:28 | |
| openstackgerrit | Brin Zhang proposed openstack/nova master: Add unshelve instance error info to fault table https://review.openstack.org/579747 | 02:34 |
| *** edmondsw has joined #openstack-placement | 02:53 | |
| openstackgerrit | Merged openstack/nova master: Remove unused variable in migration https://review.openstack.org/581464 | 02:56 |
| *** edmondsw has quit IRC | 02:57 | |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Use ThreadPoolExecutor for max_concurrent_live_migrations https://review.openstack.org/563505 | 02:58 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | 03:00 |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Compute: add support to abort queued live migration https://review.openstack.org/568542 | 04:05 |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Fix ServerMigrationSampleJsonTestsV2_24 to use its own sample file https://review.openstack.org/581562 | 04:18 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | 04:22 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | 04:22 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | 04:22 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534 | 04:22 |
| *** deepak_mourya has joined #openstack-placement | 05:05 | |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Add policy field to ServerGroup notification object https://review.openstack.org/563401 | 05:11 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Change the ServerGroupAntiAffinityFilter to adapt to new policy https://review.openstack.org/571166 | 05:11 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Adapt _validate_instance_group_policy to new policy model https://review.openstack.org/571465 | 05:11 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534 | 05:11 |
| *** e0ne has joined #openstack-placement | 05:16 | |
| *** e0ne has quit IRC | 05:19 | |
| openstackgerrit | Merged openstack/nova master: Update some placement docs to reflect modern times https://review.openstack.org/581151 | 05:28 |
| *** takashin has left #openstack-placement | 05:30 | |
| openstackgerrit | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.openstack.org/578019 | 06:45 |
| *** e0ne has joined #openstack-placement | 07:11 | |
| *** tssurya has joined #openstack-placement | 07:12 | |
| *** bhagyashri_s has quit IRC | 07:12 | |
| *** e0ne has quit IRC | 07:16 | |
| *** peereb has joined #openstack-placement | 07:17 | |
| *** e0ne has joined #openstack-placement | 07:19 | |
| openstackgerrit | huanhongda proposed openstack/nova-specs master: Add .idea folder to .gitignore https://review.openstack.org/581611 | 07:37 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Microversion 2.64 - Use new format policy in server group https://review.openstack.org/567534 | 07:50 |
| openstackgerrit | Yikun Jiang (Kero) proposed openstack/nova master: Address nit in afc7650e64753ab7687ae2c4f2714d4bb78a4e5a https://review.openstack.org/581616 | 07:58 |
| *** ttsiouts has joined #openstack-placement | 08:01 | |
| *** belmoreira has joined #openstack-placement | 08:13 | |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Compute: add support to abort queued live migration https://review.openstack.org/568542 | 08:16 |
| openstackgerrit | Zhenyu Zheng proposed openstack/nova master: Fix ServerMigrationSampleJsonTestsV2_24 to use its own sample file https://review.openstack.org/581562 | 08:26 |
| *** belmoreira has quit IRC | 08:26 | |
| *** belmoreira has joined #openstack-placement | 08:33 | |
| openstackgerrit | Lee Yarwood proposed openstack/nova stable/ocata: libvirt: slow live-migration to ensure network is ready https://review.openstack.org/559034 | 09:12 |
| openstackgerrit | huanhongda proposed openstack/nova-specs master: Return "deleted" time of instance when vm_state is SOFT-DELETED https://review.openstack.org/581638 | 09:13 |
| *** tetsuro has quit IRC | 09:29 | |
| *** belmoreira has quit IRC | 09:43 | |
| *** cdent has joined #openstack-placement | 09:44 | |
| *** belmoreira has joined #openstack-placement | 09:48 | |
| *** belmoreira has quit IRC | 09:57 | |
| *** ttsiouts has quit IRC | 10:04 | |
| *** ttsiouts has joined #openstack-placement | 10:06 | |
| *** ttsiouts has quit IRC | 10:10 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Transform metrics.update notification https://review.openstack.org/580567 | 10:19 |
| openstackgerrit | Lee Yarwood proposed openstack/nova stable/ocata: libvirt: slow live-migration to ensure network is ready https://review.openstack.org/559034 | 10:28 |
| openstackgerrit | huanhongda proposed openstack/nova master: Microversion 2.64 - Add "deleted" time in GET server response https://review.openstack.org/574159 | 10:28 |
| openstackgerrit | Van Hung Pham proposed openstack/nova master: convert py35 jobs to py3 https://review.openstack.org/581663 | 10:44 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova master: Update queued-for-delete from the ComputeAPI during deletion. https://review.openstack.org/566813 | 10:46 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova master: Update queued-for-delete from the ComputeAPI during deletion/restoration https://review.openstack.org/566813 | 10:48 |
| *** ttsiouts has joined #openstack-placement | 11:04 | |
| openstackgerrit | huanhongda proposed openstack/nova master: Microversion 2.64 - Add "deleted" time in GET server response https://review.openstack.org/574159 | 11:28 |
| *** e0ne has quit IRC | 12:03 | |
| *** e0ne has joined #openstack-placement | 12:22 | |
| *** edmondsw has joined #openstack-placement | 12:27 | |
| *** mriedem has joined #openstack-placement | 12:33 | |
| *** ttsiouts has quit IRC | 12:36 | |
| cdent | mriedem, jaypipes : Is there a reason in install (not upgrade) docs we have people sync the database (run migrations), instead of allowing an option to base.metadata.create_all(engine)? Is it because we might have a migration that adds some required rows to even empty tables? Something else? | 12:39 |
| jaypipes | cdent: good question. I *think* that we do that mostly to have a single, consistent DB setup/migrate/upgrade step. But not entirely sure, tbh | 12:41 |
| *** e0ne has quit IRC | 12:41 | |
| mriedem | i don't really understand the question | 12:42 |
| mriedem | what is base.metadata.create_all? | 12:42 |
| cdent | it creates all the tables | 12:42 |
| jaypipes | mriedem: it's the "normal" way of creatring the DB schema via SQLAlehcmy | 12:42 |
| mriedem | the first migration also does that | 12:42 |
| jaypipes | mriedem: the first migration creates the *original* schema definition for the tables, and the following migrations change those definitions. | 12:43 |
| mriedem | 216_havana or whatever | 12:43 |
| jaypipes | mriedem: right, at the start of each release we have a "savepoint"-like schema migration | 12:43 |
| mriedem | is the question why don't we just throw the models at sqla and have it do the migration rather than go through sqlalchemy-migrate? | 12:43 |
| cdent | It came up because of the issues I was having yesterday with not being able to get some of the migrations to work, so thought: I don't care about migrations, I care about tables. So I did this: https://github.com/cdent/placedock/commit/b5ca753a0d97e0d9a324e196349e3a19eb62668b | 12:43 |
| cdent | mriedem: no the question is: we have table definitions in api_models.py (etc) so we can use those. we don't need to migrate. | 12:44 |
| mriedem | i think i'm asking the same question but w/o the proper words, | 12:45 |
| mriedem | and i think (forgot his name, guy from rax that did db stuff) at one point we were going down that route | 12:45 |
| cdent | Once I did that, I thought: hmmm, I wonder if we can speed up any of our tests? And: I wonder if we could provide a lighter weight install for people | 12:45 |
| mriedem | but then it fell through | 12:45 |
| cdent | (not that the migrations are slow when the db is empty or anything, but they _do_ fail when you try to use weird dbs) | 12:46 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Skip ServerShowV257Test.test_rebuild_server for cells v1 job https://review.openstack.org/581717 | 12:49 |
| cdent | mriedem, jaypipes : is having a create_db command something we should consider for...$random_future? | 12:49 |
| mriedem | dansmith might remember what i'm trying to remember | 12:49 |
| mriedem | create_db as in create the actual database? | 12:50 |
| cdent | sorry create_db_tables | 12:51 |
| *** ttsiouts has joined #openstack-placement | 12:51 | |
| *** e0ne has joined #openstack-placement | 12:51 | |
| *** e0ne has quit IRC | 12:52 | |
| mriedem | i wanna say a guy was working on that so you could run things with alembic(?) but again, this was years ago and fell through | 12:53 |
| *** e0ne has joined #openstack-placement | 12:55 | |
| *** belmoreira has joined #openstack-placement | 12:58 | |
| *** belmoreira has quit IRC | 12:59 | |
| jaypipes | cdent: I'd have to think about that for a bit. | 13:01 |
| *** openstack has joined #openstack-placement | 13:06 | |
| *** ChanServ sets mode: +o openstack | 13:06 | |
| openstackgerrit | sahid proposed openstack/nova stable/queens: hardware: fix hugepages memory usage per intances https://review.openstack.org/581736 | 13:06 |
| jaypipes | cdent, mriedem: "savepoint migrations" things like 206_havana etc | 13:07 |
| jaypipes | the "consolidated migration" thingees | 13:07 |
| jaypipes | sorry, terminology sucks | 13:07 |
| cdent | jaypipes: I'm aware of that file, but I don't understand how it fits in with, or compares to, doing a metadata.create_all()? | 13:09 |
| mriedem | it creates all the tables | 13:10 |
| mriedem | it was a compression of all of the db migrations up to havana | 13:11 |
| mriedem | we haven't done one of those since though | 13:11 |
| mriedem | and we were told years ago that operators didn't like those compressed migrations anyway | 13:11 |
| jaypipes | oh? I thought for some reason we did that at each release... shows you how much I follow this. | 13:11 |
| mriedem | i forget the exact reasons why, might have had to do with skip level migrations | 13:11 |
| mriedem | sdague brought it back as feedback from an ops thing years ago | 13:12 |
| cdent | hmmm. I'm still confused. 216_havanna is huge. But you can create all the tables in three lines of code (assuming you already have the classes in the *models.py) | 13:13 |
| mriedem | cdent: yes and that's the thing i'm saying the guy from rax was doing | 13:13 |
| mriedem | i forget the name and forget why it was dropped - not sure if it works still or not | 13:13 |
| mriedem | but work was put into making that happen | 13:13 |
| mriedem | because years ago it wasn't possible for some reason | 13:14 |
| cdent | yeah, my question was more directed at jaypipes saying "useful to have ... savepoint" | 13:14 |
| mriedem | probably due to models that didn't match the migrations | 13:14 |
| mriedem | savepoint isn't really a thing, | 13:14 |
| mriedem | we don't do compacted migrations anymore, | 13:14 |
| mriedem | and we don't do downgrades | 13:14 |
| mriedem | i want to say we have a test (somewhere - maybe it's a fixture from oslo.db) that compares the schema *after* running the migration scripts to the models | 13:15 |
| mriedem | and fails if those aren't aligned | 13:15 |
| mriedem | so that one *can* deploy from just the models w/o running the scripts | 13:15 |
| mriedem | the other thing you can't do with just the models on upgrades is stuff where we did blocker migrations, like in ocata | 13:16 |
| mriedem | which is necessary for times when we want to drop compat routines | 13:16 |
| mriedem | i.e. you haven't done your online data migratoins yet, so db sync fails until you do and then we can drop compat code | 13:16 |
| * cdent nods | 13:16 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Fix TypeError in prep_resize allocation cleanup https://review.openstack.org/581741 | 13:17 |
| cdent | my main focus with these questions is on fresh installs | 13:17 |
| openstackgerrit | Chris Dent proposed openstack/nova master: [placement] add error.code on a ConcurrentUpdateDetected https://review.openstack.org/581742 | 13:19 |
| cdent | efried, jaypipes, edleafe ^ not certain we want that, but it's there if we do | 13:20 |
| cdent | came up because I was reading through code and saw https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L1748 | 13:21 |
| efried | cdent: Yup | 13:23 |
| openstackgerrit | Chris Dent proposed openstack/nova master: Test for unsanitized consumer UUID https://review.openstack.org/581137 | 13:24 |
| cdent | gibi, efried : I rebased ^ because it appeared stuck, so gibi if you want to put your +W back on it | 13:25 |
| efried | cdent: Arguably, we should resolve that TODO by raising Retry on *any* 409 though. | 13:25 |
| efried | I guess it's a NOTE | 13:25 |
| cdent | efried: doing it on any 409 seems a bit dangerous because you can't know for certain what the conflict might be without further information | 13:26 |
| efried | I take it back. A 409 for a gen conflict wouldn't ... | 13:26 |
| cdent | the world might change and a 409 might happen which is basically " you screwed" | 13:27 |
| efried | cdent: to be clear, your change is going to set the same error code for gen conflicts *and* for non-gen "concurrent updates". | 13:30 |
| efried | IMO it would be nice to be able to distinguish the two, but I seem to be the only one pulling for highly-granular error codes around here. | 13:31 |
| cdent | ConcurrentUpdateDetected is only supposed to be used when there is a generation conflicdt | 13:32 |
| cdent | and in fact that is currently how it is sued | 13:32 |
| cdent | it is true that it is used for both consumer gen and rp gen | 13:34 |
| cdent | but only that | 13:34 |
| cdent | or am I missing something efried ? | 13:34 |
| efried | cdent: I thought you could also get ConcurrentUpdateDetected (and/or possibly DBDuplicateEntry?) if | 13:37 |
| efried | you start a transaction | 13:37 |
| efried | another thread starts and completes a transaction | 13:37 |
| efried | you complete your transaction | 13:37 |
| efried | ...even if the conflict isn't caused by generation (i.e. you're using a pre-generation microversion, or calling an op that doesn't involve generations) | 13:37 |
| cdent | there are exactly two places where ConcurrentUpdateDetected is raised | 13:38 |
| cdent | but there are many places where it is caught | 13:38 |
| cdent | I added comment= in all the places where it is caught that I could find | 13:38 |
| gibi | cdent: done | 13:40 |
| cdent | thanks gibi | 13:40 |
| efried | cdent: So maybe I have this backwards. | 13:41 |
| *** ttsiouts has quit IRC | 13:41 | |
| *** e0ne has quit IRC | 13:45 | |
| *** ttsiouts has joined #openstack-placement | 13:46 | |
| efried | cdent: There are two different classes of code path where we 409 for generation conflicts. | 13:47 |
| efried | 1) Where we explicitly check in python with != | 13:47 |
| efried | 2) Where we implicitly check in the db UPDATE for generation with rowcount != 1 | 13:47 |
| efried | With the change as you've made it, only the latter will get the CONCURRENT_UPDATE code. | 13:47 |
| efried | Is that the right thing? I think it's not; I think to the API consumer, both of the above are the same and should appear thus. | 13:47 |
| efried | Because both mean that another transaction completed since we GETted our payload's generation. | 13:48 |
| cdent | efried: I think you're saying my change is not complete. That's correct. _All_ it is doing is adding a code when ConcurrentUpdateDetected is raised | 13:49 |
| cdent | there are additional places where HTTPConflic is raised and means the same thing | 13:49 |
| efried | cdent: okay, cool, we're in agreement on that point. I think it's worth making the change complete in that sense, don't you? | 13:49 |
| cdent | I don't know, I'd be more inclined to do is separately | 13:50 |
| cdent | but if you like I can do it together | 13:50 |
| cdent | efried: part of the reason for doing it separately is that the case I haven't covered is actually testable | 13:51 |
| cdent | (given the current ways we test) | 13:51 |
| efried | cdent: Right, but the other kind is. | 13:51 |
| cdent | i'll do the second kind in a fup | 13:54 |
| efried | cdent: Okay, +2 with notes on all of that. | 13:54 |
| efried | cdent: Are you looking to do the consumer gen work on the report client side? | 13:56 |
| cdent | no | 13:56 |
| efried | ight | 13:56 |
| cdent | unless I had signed up for that and forgotten~? | 13:56 |
| openstackgerrit | Eric Fried proposed openstack/nova master: Delete orphan compute nodes before updating resources https://review.openstack.org/579922 | 13:57 |
| efried | cdent: Updated commit message as suggested ^ | 13:58 |
| cdent | efried: I think you would agree that this https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/util.py#L681-L684 is not a concurent update. (Just confirming because I'm cruising through HTTPConflictS) | 14:00 |
| efried | cdent: Well. | 14:01 |
| efried | It is a generation conflict. | 14:01 |
| *** mriedem has quit IRC | 14:01 | |
| efried | I get what you're saying that it's not strictly a "concurrent update". | 14:01 |
| cdent | it is also a special case of generation conflict | 14:02 |
| cdent | so it should not be the same code as anything else | 14:02 |
| efried | But I think this means the error code isn't perfectly named, not that we shouldn't mark it. | 14:02 |
| cdent | we should not mark it with what is being marked now | 14:02 |
| cdent | so it's not a part of this change, I'd say | 14:02 |
| *** mriedem has joined #openstack-placement | 14:02 | |
| efried | What do you mean "with what is being marked now"? | 14:02 |
| efried | You're saying it shouldn't even be a 409? | 14:03 |
| cdent | it is already 409 | 14:03 |
| efried | I know, I'm asking, are you saying it shouldn't be? | 14:03 |
| cdent | but it should get a placerment.concurrent_update code | 14:03 |
| efried | I'm confused. | 14:03 |
| efried | which "this change" are we talking about, the one I just reviewed, or the new one where you're finding the non-concurrent gen conflicts? | 14:04 |
| cdent | I consider incoming gen != store gen also concurrent gen conflicts, the concurrency is happening in a broader domain than just the database. that's what generation conflict means | 14:05 |
| cdent | but the issue at https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/util.py#L681-L684 is different | 14:05 |
| efried | No, it's not different. | 14:05 |
| cdent | s/different/special/ | 14:05 |
| efried | In real usage, this probably *does* represent a "normal" generation conflict, in fact. Thread A GETs allocation on an existing consumer (gen=5); thread B DELETEs allocations; thread A PUTs with gen=5 and bounces because we now expect gen=null. | 14:05 |
| cdent | that's what's at the forefront of your mind because we've just just been in hell with regard to deleting consumers | 14:06 |
| cdent | but what 'null' really means is "I want to create new allocations". It is is your first attempt | 14:06 |
| *** e0ne has joined #openstack-placement | 14:07 | |
| cdent | and if there is a stored gen, then your expectations about "new" were all wrong | 14:07 |
| efried | This code path isn't saying "you said null but it should be non-null". It's saying the opposite. | 14:07 |
| efried | In this case you thought it WASN'T new, but it was. | 14:07 |
| efried | And the race I described above is realistically how you would get there (other than just being an idiot and putting random garbage into a generation field, which could happen in any of these code paths) | 14:08 |
| cdent | blargh | 14:08 |
| efried | Even if it was the opposite, my answer would be the same. | 14:08 |
| cdent | double blargh | 14:08 |
| cdent | we're requiring far too much brains in both the client and the server for this stuff | 14:09 |
| efried | You say null but it's non-null, means you thought it was new but someone else came in and created it before you got there. Generation conflict. | 14:09 |
| efried | I'm not having trouble with the concept tbh. | 14:09 |
| cdent | I have trouble with consumers, so I'm started off on the wrong foot | 14:10 |
| cdent | but my statement about brains was more about frustration with regard to how much we expect the client to do | 14:10 |
| cdent | I know it is where we are now but I really don't like it | 14:10 |
| cdent | I'll just go ahead and signal the issue here, and we'll see how it holds up | 14:11 |
| openstackgerrit | Merged openstack/nova master: Remove irrelevant comment https://review.openstack.org/578821 | 14:12 |
| efried | cdent: Goes back to the 404 thing. If we 404 on nonexistent consumer, the client should: | 14:14 |
| efried | resp = GET /allocations/{u} | 14:14 |
| efried | if resp.status == 404: | 14:14 |
| efried | put_gen = None | 14:14 |
| efried | existing_allocs = {} | 14:14 |
| efried | else: | 14:14 |
| efried | put_gen = resp.json()['consumer_generation'] | 14:14 |
| efried | existing_allocs = resp.json()['allocations'] | 14:14 |
| efried | <do stuff to existing_allocs> | 14:14 |
| efried | PUT /allocations/{u} { payload including put_gen } | 14:14 |
| cdent | efried: some of my confusion above may be because of the comment being not quite right: https://github.com/openstack/nova/blob/master/nova/api/openstack/placement/util.py#L676-L677 | 14:14 |
| cdent | "None for the consumer generation if we get here" | 14:14 |
| cdent | efried: I don't think we should ever have to require a client to GET /allocations | 14:15 |
| efried | If we 200 { 'allocations': {} } on nonexistent consumer, it looks more like: | 14:15 |
| efried | resp = GET /allocations/{u} | 14:15 |
| efried | put_gen = resp.json().get('consumer_generation') | 14:15 |
| efried | existing_allocs = resp.json().get('allocations') | 14:15 |
| efried | ... | 14:15 |
| openstackgerrit | Surya Seetharaman proposed openstack/nova master: Update queued-for-delete from the ComputeAPI during deletion/restoration https://review.openstack.org/566813 | 14:16 |
| efried | which is less code, I'll grant you, but it still doesn't mean we should do the 200 - that's still wrong imo. | 14:16 |
| * cdent is not sure what efried is try8ing to say | 14:16 | |
| efried | I'm saying the burden on the client isn't especially onerous. | 14:16 |
| cdent | how is that related to what we're talking about now? (I agree that 404 is the right response) | 14:16 |
| efried | ...whether we 200 or 404. | 14:16 |
| cdent | you're thinking about the client in terms of python code | 14:16 |
| cdent | I think about the client in terms of HTTP requests by any number of many systems | 14:17 |
| efried | Regardless, the burden on the client isn't bad. | 14:17 |
| cdent | we'll agree to disagree | 14:17 |
| efried | And whoah, "I don't think we should ever have to require a client to GET /allocations" ?? | 14:17 |
| efried | If you don't GET the existing allocations, you can't modify them intelligently; you can only replace them. | 14:18 |
| cdent | in the normal course of events the client should GET /allocation_candidates and PUT /allocations | 14:18 |
| efried | Um, okay, then we're even less burdensome on the client. | 14:19 |
| efried | Because the allocation candidate will have the right generation in it. | 14:19 |
| cdent | "modifying allocations intelligently" is not something that should be done in the normal course of events | 14:19 |
| efried | This ^ seems like a nova statement | 14:19 |
| cdent | (I'm using "should" with powerful intent here) | 14:19 |
| efried | But anyway, okay, if the pattern *should* always be GET /a_c => PUT /allocations, then we're expecting nothing of the client wrt understanding generations, consumer or provider. | 14:20 |
| efried | If your PUT bounces 409, you either PUT a different candidate or you redo your GET /a_c. | 14:20 |
| efried | If you want to use error code to determine whether the 409 was caused by concurrent update or something else (e.g. inventory exceeded) it might inform which of those things you do. | 14:22 |
| efried | which is why we should have different error codes for those things. | 14:22 |
| efried | but you don't have to do that, if you think it's too burdensome; you can just always redrive the GET /a_c => PUT /allocations procedure (or fail out). | 14:23 |
| cdent | "different error codes for those things" <- what are the different those things? | 14:24 |
| efried | [generation conflict] vs. e.g. [inventory exceeded] | 14:25 |
| jaypipes | cdent: +Wallaby'd https://review.openstack.org/#/c/581742 | 14:25 |
| cdent | efried: right, and that's what I'm doing, yes? | 14:26 |
| efried | yup. | 14:26 |
| cdent | I had the impression you were trying to assert something else | 14:27 |
| efried | I'm disputing that anything about the consumer model (at least in the form we're trying to shape it up to) is particularly onerous/burdensome to the API client. | 14:27 |
| efried | consumer or generation model | 14:28 |
| cdent | The original model for allocations was that if there was room on the rps, you could write the allocations. I liked that. | 14:29 |
| cdent | that it is more complicated than that now (because we've decided it has to be) is frustrating | 14:29 |
| cdent | I continue to wonder if there were other ways "out" | 14:30 |
| efried | Shrug, we've made it more powerful such that you can now design a system that tolerates multiple threads interacting with the same consumers/allocations at the same if that's something you need/want to do. If it's not, you can use the old microversion and just blow away allocations without gen checking. | 14:31 |
| cdent | Yeah, you say "we've made it more powerful such that you can now design a system that tolerates multiple threads interacting with the same consumers/allocations at the same if that's something you need/want to do" like it's a good thing. | 14:32 |
| efried | Given that HTTP APIs are supposed to be able to tolerate multiple threads hitting the same routes at the same time, yeah, I think it's a good thing. | 14:33 |
| cdent | If we modelled consumers as a single locus of control, then we'd have a simplified system in many ways | 14:34 |
| cdent | in that (theoretical) model what provides thread safety for the HTTP APIs is unique (and opaque) ids | 14:34 |
| efried | Then consumers would behave like resource providers do, in terms of generation etc., which would be okay. The complexity arises from having tried to make consumers hidden/implicit/automatic, not from how we've done generations. | 14:36 |
| cdent | I think the opposite actually | 14:36 |
| cdent | by labelling allocation groups as "consumers" we walked into a trap | 14:37 |
| cdent | nova should conceptualize things as consumers | 14:37 |
| cdent | placement should not | 14:37 |
| cdent | I, nova, am going to create an allocation for my uniquely identified consumer, which I'm controlling in this thread and this thread alone | 14:39 |
| openstackgerrit | do3meli proposed openstack/nova master: docs: add nova host-evacuate command to evacuate documentation https://review.openstack.org/578040 | 14:43 |
| *** ttsiouts has quit IRC | 14:48 | |
| jaypipes | cdent: and what about when we have multiple services wanting to consume things from multiple providers in a single transaction? who gets to determine what the consumer is? | 14:51 |
| cdent | you say that like it's a good thing too | 14:51 |
| jaypipes | cdent: I'm asking you a question, nothing more. | 14:51 |
| cdent | above when I said "should" with intent I was trying to indicate that reality is a pain in the ass | 14:51 |
| cdent | in this other reality the idea of "multiple services consuming things in a single transaction" wouldn't happen that way | 14:52 |
| cdent | consumption would be a one (and only one) stop on a path | 14:53 |
| cdent | an early step in the path would be "determine the consumer" | 14:53 |
| jaypipes | cdent: so something should create the consumer object ahead of time and then call PUT /consumers/{consumer_uuid}/allocations. I **completely** agree with you, Chris! :) | 14:53 |
| cdent | ha! no :) | 14:53 |
| jaypipes | :) | 14:54 |
| cdent | we already have something which creates the instance and its uuid. if we had this mythical nodelet thing that people are vaguely talking about, it would presumably use a something like that. | 14:55 |
| cdent | from the placement standpoint it would just be an identifier | 14:55 |
| *** ttsiouts has joined #openstack-placement | 14:55 | |
| cdent | for the allocations | 14:55 |
| jaypipes | cdent: I think the nodelet would be responsible for creating the providers, not the consumers, no? | 14:57 |
| jaypipes | cdent: or am I misunderstanding what the nodelet is? | 14:57 |
| jaypipes | I thought nodelet was vaguely like kubelet? | 14:57 |
| cdent | one sec, writing a commit message, brb | 14:57 |
| jaypipes | np | 14:57 |
| cdent | sorry, jaypipes, I was jumping steps in my writing. yeah, nodelet is vaguely like kubelet, but the idea is that it woudl coordinate activity on the node, such that it would be in charge (to large extent) of the entire suite of resource providers. When something needs to be built (whatever that is) a nodelet would eventually handle it. The thing which tells the nodelet to do stuff is responsible for creating the id | 15:02 |
| cdent | which would eventually be used when writing all the allocations for the thing being built | 15:02 |
| openstackgerrit | Chris Dent proposed openstack/nova master: Add placement.concurrent_udpate to generation pre-checks https://review.openstack.org/581771 | 15:02 |
| cdent | efried, jaypipes there's the followup | 15:02 |
| efried | ack | 15:03 |
| cdent | I guess that teller is the super super conductor | 15:03 |
| * cdent shrugs | 15:03 | |
| cdent | I dunno | 15:03 |
| jaypipes | efried, cdent: well, it's the requestspec.instance.uuid for nova at least. but could easily be heat or some other orchestrator I suppose. | 15:05 |
| efried | cdent: +2, nice one guvnor | 15:10 |
| openstackgerrit | Matt Riedemann proposed openstack/nova-specs master: Fix nits in the handling down cell spec https://review.openstack.org/581243 | 15:21 |
| cdent | efried, jaypipes, edleafe: reminder that I'll be gone much of tomorrow and all of friday. If any of my code needs adjustment, please feel free | 15:25 |
| jaypipes | cdent: cheers, will do. | 15:27 |
| cdent | the main one I'm thinking of is https://review.openstack.org/#/c/576927/ which is skeletal | 15:28 |
| edleafe | cdent: enjoy! | 15:28 |
| mriedem | update on the udpate | 15:35 |
| *** tssurya has quit IRC | 15:44 | |
| *** peereb has quit IRC | 15:46 | |
| openstackgerrit | do3meli proposed openstack/nova master: docs: add nova host-evacuate command to evacuate documentation https://review.openstack.org/578040 | 15:47 |
| *** ttsiouts has quit IRC | 16:09 | |
| efried | I'm going to take the afternoon off. | 16:18 |
| mriedem | enjoy | 16:23 |
| *** efried is now known as efried_off | 16:23 | |
| openstackgerrit | Merged openstack/nova master: Test for unsanitized consumer UUID https://review.openstack.org/581137 | 16:27 |
| *** efried_off has quit IRC | 16:31 | |
| *** efried_off has joined #openstack-placement | 16:31 | |
| openstackgerrit | Matt Rabe proposed openstack/nova master: Add destination MSP IP address to PowerVM migrate data https://review.openstack.org/579676 | 16:33 |
| *** e0ne has quit IRC | 16:39 | |
| *** belmoreira has joined #openstack-placement | 17:39 | |
| *** mriedem1 has joined #openstack-placement | 18:02 | |
| *** mriedem has quit IRC | 18:05 | |
| *** edmondsw has quit IRC | 18:07 | |
| *** edmondsw has joined #openstack-placement | 18:07 | |
| *** mriedem1 is now known as mriedem | 18:12 | |
| *** e0ne has joined #openstack-placement | 18:16 | |
| openstackgerrit | Merged openstack/nova master: [placement] add error.code on a ConcurrentUpdateDetected https://review.openstack.org/581742 | 18:20 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add nova-status upgrade check for request spec migrations https://review.openstack.org/581813 | 18:34 |
| *** tssurya has joined #openstack-placement | 18:48 | |
| *** e0ne has quit IRC | 18:49 | |
| *** cdent has quit IRC | 18:52 | |
| *** e0ne has joined #openstack-placement | 19:01 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Add functional regressions tests for server_group_members OverQuota https://review.openstack.org/581845 | 19:06 |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/queens: Fix server_group_members quota check https://review.openstack.org/581846 | 19:06 |
| *** e0ne has quit IRC | 19:06 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Add functional regressions tests for server_group_members OverQuota https://review.openstack.org/581866 | 19:17 |
| openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Fix server_group_members quota check https://review.openstack.org/581867 | 19:17 |
| *** e0ne has joined #openstack-placement | 19:17 | |
| *** belmoreira has quit IRC | 19:21 | |
| *** e0ne has quit IRC | 19:24 | |
| *** e0ne has joined #openstack-placement | 19:28 | |
| *** e0ne has quit IRC | 19:31 | |
| *** e0ne has joined #openstack-placement | 19:41 | |
| *** edmondsw has quit IRC | 19:42 | |
| *** edmondsw has joined #openstack-placement | 19:43 | |
| *** e0ne has quit IRC | 20:31 | |
| *** e0ne has joined #openstack-placement | 20:35 | |
| openstackgerrit | Dan Smith proposed openstack/nova master: Avoid requesting DISK_GB allocation for root_gb on BFV instances https://review.openstack.org/580720 | 20:39 |
| *** e0ne has quit IRC | 20:42 | |
| *** jaypipes has quit IRC | 21:09 | |
| openstackgerrit | Dan Smith proposed openstack/nova master: Avoid requesting DISK_GB allocation for root_gb on BFV instances https://review.openstack.org/580720 | 21:10 |
| openstackgerrit | Merged openstack/nova master: Add queued for delete to instance_mappings table. https://review.openstack.org/566788 | 21:32 |
| openstackgerrit | Merged openstack/nova master: Fix TypeError in prep_resize allocation cleanup https://review.openstack.org/581548 | 21:32 |
| openstackgerrit | Merged openstack/nova master: Add functional regressions tests for server_group_members OverQuota https://review.openstack.org/580755 | 21:33 |
| openstackgerrit | Merged openstack/nova master: Fix server_group_members quota check https://review.openstack.org/580684 | 21:33 |
| *** takashin has joined #openstack-placement | 21:39 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova-specs master: Create specs directory for Stein https://review.openstack.org/573602 | 22:00 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (5) https://review.openstack.org/570842 | 22:01 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (6) https://review.openstack.org/571330 | 22:01 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (7) https://review.openstack.org/571992 | 22:02 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (8) https://review.openstack.org/571993 | 22:02 |
| *** tssurya has quit IRC | 22:05 | |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (3) https://review.openstack.org/574104 | 22:21 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (4) https://review.openstack.org/574106 | 22:22 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (5) https://review.openstack.org/574110 | 22:22 |
| *** edmondsw has quit IRC | 23:12 | |
| *** edmondsw has joined #openstack-placement | 23:13 | |
| *** edmondsw has quit IRC | 23:17 | |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Add another up-call to the cells v2 caveats list https://review.openstack.org/581910 | 23:23 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (6) https://review.openstack.org/574113 | 23:24 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in unit/network/test_neutronv2.py (7) https://review.openstack.org/574974 | 23:25 |
| openstackgerrit | Takashi NATSUME proposed openstack/nova master: Remove mox in libvirt/test_driver.py (5) https://review.openstack.org/570842 | 23:29 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Short-circuit targets_cell if already targeted https://review.openstack.org/581912 | 23:57 |
| openstackgerrit | Matt Riedemann proposed openstack/nova master: Short-circuit targets_cell if already targeted https://review.openstack.org/581912 | 23:59 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!