| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-09-25 | |||
| 17:32:12 | mdbooth | The check queue is toast | |
| 17:33:20 | melwitt | tssurya: currently there isn't a way, as you point out. I've been thinking about that too and it would be nice if we could include the actual exception raised per cell. I think we still want the sentinel though, so we'd need a new format for the value side of the results dict | |
| 17:33:37 | openstackgerrit | Matthew Booth proposed openstack/nova master: Don't delete disks on shared storage during evacuate https://review.openstack.org/578846 | |
| 17:33:37 | openstack | bug 1550919 in OpenStack Compute (nova) "[Libvirt]Evacuate fail may cause disk image be deleted" [Medium,In progress] https://launchpad.net/bugs/1550919 - Assigned to Matthew Booth (mbooth-9) | |
| 17:33:37 | openstackgerrit | Matthew Booth proposed openstack/nova master: Add regression test for bug 1550919 https://review.openstack.org/591733 | |
| 17:34:04 | tssurya | melwitt: yea we would have to tweak the return part I guess | |
| 17:34:56 | melwitt | tssurya: I suppose we could just change things to include the exception object instead of the sentinel, then you just have to check isinstance(x, Exception) instead of checking "if sentinel" | |
| 17:35:37 | melwitt | to know whether you've got a good result vs an exception | |
| 17:37:03 | tssurya | so you mean at this point https://github.com/openstack/nova/blob/ebab3adb2849c17ebb0249277da89d0d8111e591/nova/context.py#L445 ? | |
| 17:38:55 | melwitt | yeah, one way would be to just store the exception object in 'result' instead of the sentinel. then obviously anywhere in the code that checks "if raised_exception_sentinel" would have to be updated to check isinstance(x, Exception) instead | |
| 17:40:56 | sean-k-mooney | melwitt: i was not following but is that not an anti pattern | |
| 17:42:16 | melwitt | sean-k-mooney: is it? I said earlier if we wanted to keep the sentinels, we'd have to do something to change the format of the value side to be able to include optional additional data (where the exception object could go) | |
| 17:42:33 | sean-k-mooney | melwitt: oh your talking about scatter_gather_cells | |
| 17:42:39 | melwitt | yes | |
| 17:43:40 | sean-k-mooney | the antipattern is the explcit type check in python e.g. isinstance checks but this is not a standard case | |
| 17:45:07 | melwitt | oh, I see. yeah, if results = {cell_uuid: (result or sentinel, optional data)}, then the optional data type could be assumed based on the sentinel | |
| 17:45:25 | sean-k-mooney | melwitt: if the was c++ or pyton3 only i would lean towrods makeing result a concurrent future or std::expected type | |
| 17:46:32 | melwitt | sean-k-mooney: I don't know much about that but the expected type will be different depending on whether the cell returned a result or an exception | |
| 17:47:04 | tssurya | melwitt: for me the requirement came because of wanting to differentiate between InstanceNotFound valid exception versus an actuall cell down exception here: https://review.openstack.org/#/c/591658/7/nova/compute/api.py@2327 , feel free to leave a comment and we can also raise it at tomorrow's cells meeting; need to leave office now | |
| 17:47:58 | sean-k-mooney | in c++ std::expect models a type that either pacages the expected result or and error type. when you use it you can test if it it has an error or not with an if expect ... | |
| 17:48:22 | melwitt | tssurya: ok, will look there. I was thinking we would need to be able to differentiate exceptions but didn't see a concrete example till now | |
| 17:48:50 | melwitt | sean-k-mooney: oh, nice | |
| 17:49:37 | AJaeger | nova stable team, could you review a simple zuul job change, please? https://review.openstack.org/602018 https://review.openstack.org/602019 https://review.openstack.org/602022 https://review.openstack.org/602023 | |
| 17:49:45 | AJaeger | mriedem: I answered your comment on https://review.openstack.org/#/c/602018/1/.zuul.yaml | |
| 17:49:54 | sean-k-mooney | melwitt: so my preference would be have result be a type that you can just do "if result: #handel sucess else : # handel error" | |
| 17:50:58 | melwitt | sean-k-mooney: we also have "did not respond" i.e. timeout waiting for result | |
| 17:51:02 | sean-k-mooney | melwitt: to do that you would have to override __bool__ in the wrapper class to see if the value was an exception type or not | |
| 17:51:19 | melwitt | which could be an error, but then you have to check the type of the error | |
| 17:51:34 | melwitt | but I guess we have to do that anyway if we're differentiating | |
| 17:52:46 | sean-k-mooney | melwitt: ya. if you stuff the exeption into the value field you can just rerais it and cactch it polymorifclly assuming the timeout is a different exception type | |
| 17:53:13 | melwitt | yeah, could do that | |
| 17:53:49 | sean-k-mooney | melwitt: so basically its the same as what you suggted but your hiding the type checking in a dedicated class that does it for you instead of doing it in multiple places | |
| 17:55:34 | sean-k-mooney | melwitt: python3's concurrent futures basically wrap this up nicly for you too but they are python 3 only https://docs.python.org/3/library/concurrent.futures.html#concurrent.futures.Future | |
| 17:56:09 | melwitt | sean-k-mooney: thanks | |
| 17:59:57 | sean-k-mooney | melwitt: want me to mock up an example of using what i described so you can see how it would work. im not sure its the correct solution in this case but its what i would try personally. | |
| 18:05:09 | melwitt | sean-k-mooney: an example that would work for python2 also? I would be interested in looking at it if you mock it up | |
| 18:05:57 | sean-k-mooney | melwitt: yes i can make it work for python too. ill mock something up and push it with some unit tests showing its usage later tonight | |
| 18:06:17 | sean-k-mooney | *python 2 | |
| 18:06:50 | melwitt | thanks | |
| 18:16:34 | openstackgerrit | Jack Ding proposed openstack/nova master: Handle missing marker during online data migration https://review.openstack.org/605164 | |
| 18:17:11 | jaypipes | man I fucking hate the scheduler unit tests. | |
| 18:17:15 | jaypipes | what a pile of shit. | |
| 18:17:59 | sean-k-mooney | jaypipes: let me guess asserting behavior 3 levles deep then just the local function behvior? | |
| 18:19:39 | sean-k-mooney | anyone know where the devstack log is stored by default? ther eused to be an xstack log in /opt/stack/logs but that seams to not be a thing anymore | |
| 18:20:10 | jaypipes | sean-k-mooney: more the endless hierarchy of copy/pasted test case classes. | |
| 18:20:39 | sean-k-mooney | jaypipes: yeah OOP | |
| 18:22:11 | sean-k-mooney | jaypipes: did i mention im playing with creating a programming language. it "object orianted" current it does not allow inheritence but you can have objects :P | |
| 18:26:30 | AJaeger | mriedem: could you review the other stable backports of that ceph change as well, please? | |
| 18:26:47 | mriedem | yeah | |
| 18:28:22 | AJaeger | thanks | |
| 18:31:58 | melwitt | what is going on with the gate lately? changes that have been in the queue for 26 hours | |
| 18:33:12 | AJaeger | melwitt: high failure rate plus one of our clouds down, see clark's email from a week ago or so | |
| 18:33:49 | melwitt | found it, thank you | |
| 18:33:52 | AJaeger | melwitt: http://lists.openstack.org/pipermail/openstack-dev/2018-September/134867.html | |
| 18:34:40 | AJaeger | melwitt: still applies regarding cloud down - clarkb is testing a fix right now | |
| 18:46:27 | openstackgerrit | Jonte Watford proposed openstack/nova master: This commit adds __str__ and __repr__ methods to objects InstanceNUMACell and NUMACell. These will generate more readable representations of the objects for use in logs. https://review.openstack.org/600269 | |
| 18:48:57 | mriedem | AJaeger: waiting to see the renamed job run on ocata https://review.openstack.org/#/c/602023/ | |
| 18:49:44 | AJaeger | mriedem: good catch - you won't see it. We miss the parent devstack job there. This needs different handling - I'll fix... | |
| 18:54:27 | openstackgerrit | Jonte Watford proposed openstack/nova master: This commit adds __str__ method to objects InstanceNUMACell and NUMACell. These will generate more readable representations of the objects for use in logs. https://review.openstack.org/600269 | |
| 19:01:36 | mriedem | jaypipes: i've got an o.vo problem that maybe you can help solve, | |
| 19:01:58 | mriedem | i've got an Instance object from cell1, pulled fresh from the db so there are no fields marked as 'changed', | |
| 19:02:30 | mriedem | i want to clone that object and re-create it in cell2 which should mean just changing the context to point at cell2 db and delete the id field so Instance.create() won't puke on it, | |
| 19:03:01 | mriedem | the problem is, the cloned object doesn't have any changed fields marked, so Instance.create() doesn't actually save anything into the cell2 db for the instance for things like vm_state, task_state, etc | |
| 19:03:54 | mriedem | know of any way to dirty up the object so it looks new? I tried a obj_from_primitive(obj.obj_to_primitive()) dance but that doesn't work b/c it maintains the 'changes' list of dirty fields | |
| 19:04:30 | mriedem | i'm thinking i might just have to do something like, new_obj = objects.Instance(new_context, **instance.obj_to_primitive()['data']) | |
| 19:05:58 | melwitt | mriedem: I think the only precedent we have for that is the get_new_instance in build request https://github.com/openstack/nova/blob/master/nova/objects/build_request.py#L237 | |
| 19:07:29 | sean-k-mooney | mriedem: can you do a deep copy and then instead update the cell and then instaead of resetting the fileds just mark them as dirty? | |
| 19:07:30 | mriedem | blech, ok i'll hack that up the same | |
| 19:08:07 | melwitt | there might be a better way to do it, just mentioning that as a data point | |
| 19:08:49 | mriedem | yeah i'll copy that, | |
| 19:08:55 | mriedem | as i've seen and from dansmith's comment in that code, | |
| 19:09:01 | mriedem | keeping the fields dirty would involve ovo internals | |
| 19:09:24 | melwitt | aye | |
| 19:09:32 | mriedem | like passing a dirty_white_boy=True kwarg to obj_from_primitive | |
| 19:19:14 | jaypipes | mriedem: yeah, it's blech. | |
| 19:20:38 | mriedem | i'm still waiting for someone to get my foreigner reference | |
| 19:24:24 | jaypipes | heh | |
| 19:24:29 | melwitt | sorry, I only know the major foreigner hits | |
| 19:26:02 | mriedem | hey it made it to the top 12 https://en.wikipedia.org/wiki/Dirty_White_Boy_(song)#Chart_performance | |
| 19:26:36 | melwitt | huh. | |
| 19:37:28 | sean-k-mooney | yeah cirros does not have sriov drivers for 1gb intel nics ... | |
| 19:38:07 | mriedem | ok i've got something hacked up for now | |
| 19:38:28 | mriedem | http://paste.openstack.org/show/730743/ | |
| 19:39:41 | sean-k-mooney | mriedem: without messing with internal that looks resonably clean | |
| 19:40:17 | sean-k-mooney | mriedem: it would be nice to a "mark_dirty" function instead of setattr(new_obj, field, getattr(obj, field)) but that works | |
| 19:40:53 | mriedem | i'll post something to ovo that dan can look at while he's pooing in a cave | |
| 19:50:18 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Option "scheduler_default_filters" is deprecated. https://review.openstack.org/604148 | |
| 19:51:35 | imacdonn | mriedem: is there an easy way to see what 'nova-manage db online_data_migrations' would do, without actually doing it ? | |
| 19:51:37 | sean-k-mooney | melwitt: im going to call it a night an grab a beer. am im going to start hacking on that exection wrapper class and ill push something up in an hour or so while i wait for dinner. ill add you as a reviewer when i submit the patch. | |
| 19:52:46 | mriedem | imacdonn: as in a dry run option? | |
| 19:52:49 | mriedem | no there isn't | |
| 19:53:02 | mriedem | jaypipes: sean-k-mooney: like this https://review.openstack.org/605199 | |
| 19:53:19 | melwitt | sean-k-mooney: cool, thanks | |
| 19:53:32 | imacdonn | mriedem: yeah. ok. I think that would be useful. I tried hacking out the check for max-count being greater than zero, but it's not that simple, apparently | |
| 19:54:27 | mriedem | imacdonn: well, you'd have to plumb that down through all of the online data migration routines | |
| 19:54:31 | mriedem | to not actually commit any changes | |
| 19:54:34 | mriedem | so it's not trivial | |
| 19:54:49 | imacdonn | mriedem: also, this doesn't seem right. The counts in the table should be non-zero? Caveat: it's Queens, so if anything's been fixed since then .... http://paste.openstack.org/show/730819/ | |
| 19:55:30 | sean-k-mooney | mriedem: is the condition on line 401 reversed? e.g. should it be if leave_dirty? | |
| 19:57:34 | sean-k-mooney | mriedem: i would have expected the changed filed to be the dirty fields personally but i have not really looked at how this works closely | |