| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-07 | |||
| 10:24:38 | johnthetubaguy | mdbooth: given my queue of work today, I can't see me getting the time to look at that I am afraid | |
| 10:25:00 | mdbooth | johnthetubaguy: Well it's been sitting for months, so I'm not going to stress about today :) | |
| 10:25:29 | mdbooth | If you have a 'sometime soon' list, though, that would be awesome :) | |
| 10:25:39 | johnthetubaguy | mdbooth: I understand it better now, having chatted through it, which is all good, yeah, its totally on that list | |
| 10:25:48 | mdbooth | Cool, thanks. | |
| 10:26:08 | johnthetubaguy | mdbooth: if I haven't been to review that early next week, totally bug me again | |
| 10:26:18 | mdbooth | johnthetubaguy: You may regret that. | |
| 10:26:21 | mdbooth | Sure. | |
| 10:26:28 | johnthetubaguy | mdbooth: probably, but its for the best :) | |
| 10:30:36 | openstackgerrit | Akira KAMIO proposed openstack/nova master: Replace root volume during rebuild https://review.openstack.org/442295 | |
| 10:30:37 | gibi | johnthetubaguy: hi! a frindly reminder about the Show persisted scheduler hints in server details spec https://review.openstack.org/#/c/440580/ It gathered even more +1s yesterday :) | |
| 10:34:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Move 'floating_ips' opts into 'network' https://review.openstack.org/431039 | |
| 10:34:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Deprecate 'default_floating_pool' https://review.openstack.org/431038 | |
| 10:34:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Add neutron.floating_pool https://review.openstack.org/431037 | |
| 10:35:40 | jvarlamova__ | Hi, can someone help me? I get error "VirtualInterfaceCreateException: Virtual Interface creation failed", when trying to boot multiple instances in parallel. What's the reason? | |
| 10:39:31 | rpodolyaka | jvarlamova__: could you please provide a complete stack trace from the nova-compute log? | |
| 10:40:35 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Standardize naming of variables https://review.openstack.org/430272 | |
| 10:40:36 | openstackgerrit | Stephen Finucane proposed openstack/nova master: trivial: Standardize indentation of test_vif https://review.openstack.org/430271 | |
| 10:45:59 | openstackgerrit | Stephen Finucane proposed openstack/nova master: libvirt: Make 'get_domain' private https://review.openstack.org/417378 | |
| 10:47:04 | openstackgerrit | Stephen Finucane proposed openstack/nova master: console: Move proxies to 'console/proxy' https://review.openstack.org/408192 | |
| 10:53:31 | openstackgerrit | Akira KAMIO proposed openstack/nova master: Replace root volume during rebuild https://review.openstack.org/442295 | |
| 11:07:13 | openstackgerrit | Evgeny Antyshev proposed openstack/nova master: get_model method missing for Ploop image https://review.openstack.org/441989 | |
| 11:12:56 | jvarlamova__ | rpodolyaka: http://pastebin.com/0jY6Ax1C | |
| 11:20:13 | rpodolyaka | jvarlamova__: this exception is raised when nova-compute times out waiting for vif-plugged event - https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L5052-L5061 | |
| 11:21:18 | rpodolyaka | jvarlamova__: you should check nova-api / neutron-server / neutron l2 agent logs for this device id (== instance uuid) to see if such event was delivered to nova at all | |
| 11:27:41 | mdbooth | alex_xu: Hey, just thinking about your response on https://review.openstack.org/#/c/441829/4 about the cache.clear(). This is all running with the placement service, right? Are we guaranteed to only have 1 of those globally? | |
| 11:27:52 | mdbooth | cdent: ^^^ ? | |
| 11:28:27 | mdbooth | alex_xu: I'm just thinking that if a Trait is created through 1 service, then accessed through another, the cache on the second won't have been refreshed. | |
| 11:28:47 | mdbooth | However, I don't know how this is intended to be deployed, so this may not make sense. | |
| 11:29:31 | cdent | mdbooth: there's only one placement database, but could be multiple placement api servers | |
| 11:29:44 | mdbooth | cdent: That's what I would have expected. Thanks. | |
| 11:30:15 | cdent | I'm a bit nonplussed about the traits and resource class caches | |
| 11:30:35 | mdbooth | cdent: Explain | |
| 11:30:56 | cdent | they are trying to optimize sql queries when some of the data is not in an rdbms | |
| 11:31:09 | cdent | and thus they are...weird | |
| 11:31:21 | cdent | I would have prefered the optimizations to come later | |
| 11:33:24 | cdent | mdbooth: bonus points for "Explain" | |
| 11:33:32 | mdbooth | :) | |
| 11:34:03 | mdbooth | cdent: They look like they're getting all their data from the db | |
| 11:34:08 | mdbooth | What did I miss? | |
| 11:34:23 | mdbooth | For context: I never looked at this before yesterday | |
| 11:34:37 | cdent | which "they" did you mean? the traits cache may be different (and I haven't looked closely at it yet) | |
| 11:35:00 | mdbooth | 'they' == resource class and traits caches | |
| 11:35:35 | mdbooth | cdent: Ah... Yes, some resource classes are hard-coded | |
| 11:35:45 | mdbooth | I've slept since I read that | |
| 11:36:03 | cdent | you sleep? | |
| 11:36:10 | mdbooth | Don't tell anyone | |
| 11:37:10 | jvarlamova__ | rpodolyaka: As I see from logs, neutron sent 'network-vif-plugged' event for that instance to nova, and there's message "Creating event network-vif -plugged ...for instance ..." in nova-api logs. Looks like nova received event. | |
| 11:41:49 | alex_xu | mdbooth: ah...i got you, good point | |
| 11:45:34 | alex_xu | mdbooth: cdent probably resource-class won't have such problem. the cache for resource-class is just for query single resource-class. | |
| 11:47:41 | mdbooth | alex_xu: The negative lookup is going to be tough for the startswith match, I think. | |
| 11:48:04 | cdent | alex_xu: is it possible to avoid having the cache through the initial implementation, and only add the cache at the end? | |
| 11:48:08 | mdbooth | alex_xu: Whereas the other 2 are definite lookups: it exists or it doesn't, and you normally expect that it exists | |
| 11:48:26 | cdent | building in cache from the outset almost always leads to harder to maintain code | |
| 11:48:29 | mdbooth | Returning no values or some values doesn't necessarily mean that you're not out of date | |
| 11:50:06 | mdbooth | cdent: How often will we hit this thing? Would an index optimised for a startswith query be appropriate? | |
| 11:50:47 | cdent | (thus my concerns about early optimizations and speculations before measuring, etc) | |
| 11:50:53 | mdbooth | From reading the context, the expectation seems to be a lot. Whose was that expectation? | |
| 11:51:02 | alex_xu | cdent: in the poc, actually I query the db directly. but I try to get the API "GET /traits?name=in:TRAIT1,TRAIT2" to use the cache, since that is the API used to validate whether the traits is valid or not | |
| 11:51:18 | cdent | mdbooth: it will be alot, eventually | |
| 11:51:31 | cdent | but that's based on abstractions, not measurement | |
| 11:52:03 | mdbooth | Are we carefully avoiding the db throughout this whole code path? | |
| 11:52:36 | alex_xu | wait, no, that is for "GET /traits?prefix=xxx", that can be from the db directly. "GET /traits?name=in:TRAIT1,TRAIT2" can get benefit from the single trait lookup | |
| 11:53:24 | cdent | but I haven't had a chance yet | |
| 11:53:29 | mdbooth | alex_xu: Yeah, I think the concrete lookup is ok to cache. Either you get a hit (but wait... what if it was deleted?), or you don't, in which case you can refresh. | |
| 11:53:30 | cdent | (been concentrating on the spec) | |
| 11:54:33 | alex_xu | mdbooth: yea, if user query a non-existed trait, that will trigger a db call | |
| 11:54:55 | mdbooth | alex_xu: Deleting a cached trait/resource class is also an interesting case. | |
| 11:55:38 | mdbooth | Does the user have an expectation that after a delete call completes, a subsequent lookup will fail? | |
| 11:55:43 | mdbooth | What if it doesn't? | |
| 11:55:59 | mdbooth | I suspect that probably doesn't matter as much as the other case. | |
| 11:56:48 | alex_xu | yes, I remember I point out that problem from the resource class patch, but I forget what answer I got now | |
| 11:57:07 | cdent | you guys are proving my point | |
| 11:57:10 | cdent | first make it right | |
| 11:57:12 | cdent | then add the cache | |
| 11:57:49 | mdbooth | alex_xu: Would cdent's suggestion fly? | |
| 11:57:52 | jvarlamova__ | rpodolyaka: by the way, some instances were created successfully while other got an exception. Is it a known issue of parallel instances creation? | |
| 11:58:10 | alex_xu | cdent: all the cache? even for single trait lookup? | |
| 11:59:03 | cdent | theoretically, yes: that's just good practice: make code that does the right thing | |
| 11:59:10 | cdent | only once it is doign the right thing consider adding cache | |
| 11:59:22 | cdent | the mix between custom traits and the os-traits library makes things a bit weird in all this | |
| 11:59:53 | cdent | but since the number of traits is, let's say, less than 10,000 python can happily work with lists of strings of that size | |
| 12:00:23 | cdent | but again: I haven't looked at the codet yet! :) | |
| 12:00:49 | cdent | I just hear a lot of early optimizating, and feel like that's worth flagging before we get too caught up in the details of that when we don't even have a fully working api yet | |
| 12:00:56 | alex_xu | cdent: yea, that sounds make sense. hope we can catch jay also. I feel he will say something when I remove the cache :) | |
| 12:01:14 | cdent | I'm sure he will, jay is a notorious early optimizer :) | |
| 12:01:39 | alex_xu | cdent: heh :) | |
| 12:01:47 | mdbooth | alex_xu: Anyway, I'm going to remind you again that I don't have nearly enough context here. I can only point out potential problems and ask if they're real problems. | |
| 12:02:02 | mdbooth | alex_xu: Don't mean to be a pita. | |
| 12:02:22 | cdent | if what we're really concerned about is performance of the api then we should, _later_, consider a namespaced memcached layer on top of the placement api that allows marking collections as dirty | |
| 12:02:35 | alex_xu | mdbooth: appreciate your review same, it is really helpful :) | |
| 12:02:54 | cdent | alex_xu, mdbooth: I think we should have lots more of this type of conversation, so I think this is good | |
| 12:03:10 | alex_xu | mdbooth: btw, the spec is at https://review.openstack.org/345138, in case you are interesting :) | |
| 12:03:29 | mdbooth | alex_xu: Thanks, I should probably have read that first. | |
| 12:04:12 | cdent | I think jay's concerns are at the database layer, in which case we can address that by simply having all the traits in the same table, and isn't that being done when we sync from os-traits? | |
| 12:04:56 | alex_xu | cdent: yes, all the traits in one table | |
| 12:05:07 | cdent | then why have a cache at all? | |
| 12:05:11 | alex_xu | by 'placement-manage os-traits sync' | |
| 12:06:16 | alex_xu | cdent: I guess jay think the trait/resource class won't update frequently, then to same some db call... | |
| 12:06:37 | cdent | -1 | |