| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-05-16 | |||
| 14:39:41 | mriedem | mdbooth: i don't think os-brick is really actually involved much here for connection_info, | |
| 14:39:53 | lyarwood | mriedem: we did bring that up again with walter during summit btw, agreed we should do it, just need people as with every other bug, feature, focus area within OpenStack atm. | |
| 14:39:58 | mriedem | nova calls os-initialize_connection in the cinder api which calls down to the volume driver and return the dict back to nova | |
| 14:40:19 | mriedem | the connector dict is created in os-brick and is something else i toyed with formalizing | |
| 14:40:21 | mriedem | but abandoned | |
| 14:40:27 | mdbooth | mriedem: What else would the extra info in connection_info be for? | |
| 14:40:42 | mdbooth | For terminate_connection? | |
| 14:40:44 | lyarwood | mdbooth: it's for libvirt most of the time | |
| 14:40:50 | mriedem | lyarwood: mdbooth: https://review.openstack.org/#/c/269867/ | |
| 14:40:50 | lyarwood | mdbooth: source_path etc | |
| 14:41:01 | mdbooth | lyarwood: Right, there's device_path | |
| 14:41:09 | mdbooth | That's become kinda standardised | |
| 14:41:31 | mriedem | there are definitely some 'standard' things we can usually expect in connection_info, | |
| 14:41:35 | mriedem | and then vendor-specific extras | |
| 14:41:55 | mdbooth | lyarwood mriedem: So I'm looking specifically here: https://review.openstack.org/#/c/443951/15/nova/virt/libvirt/volume/vrtshyperscale.py | |
| 14:41:58 | mriedem | sort of like what is being attempted with cleaning up the instance diagnostics stuff in nova | |
| 14:42:16 | mdbooth | We're getting device_info back from os-brick, and stuffing some of it into connection_info | |
| 14:42:37 | mdbooth | That's unusual, and there's no comment explaining why we do it | |
| 14:42:42 | mdbooth | Take 'vsa_ip' | |
| 14:43:08 | mdbooth | I don't see where that information would be subsequently used. It's not used by os-brick's disconnect_volume for that driver. | |
| 14:43:25 | mdbooth | The only place I can think off the top of my head would be terminate_connection | |
| 14:43:48 | lyarwood | mdbooth: yeah agreed, unless get_config is going to use it I can't see why would stash it, unless they have plans to use it in the future? | |
| 14:43:49 | mdbooth | lyarwood: Does that make more sense to you? | |
| 14:43:51 | mriedem | we don't pass the connection_info over to terminate_connection | |
| 14:44:02 | mriedem | just the connector | |
| 14:44:15 | mdbooth | mriedem: Ah, not even there, then | |
| 14:44:33 | mdbooth | I thought we did pass it back to cinder in that case | |
| 14:44:48 | mriedem | so yeah good point on stashing stuff in connection_info after calling os-brick, i'm not sure why that's there | |
| 14:45:03 | mriedem | mdbooth: i dropped my +2 if you want to ask why they are doing that | |
| 14:46:32 | mdbooth | mriedem: Done. You notice I also wrote your prior patch btw? ;) | |
| 14:47:38 | mriedem | mdbooth: i didn't | |
| 14:48:05 | mriedem | sorry i was hoping the irc pings notified you that i wrote a patch | |
| 14:49:00 | mdbooth | mriedem: NP, it only took a couple of minutes anyway :) Pretty obvious and can't remember why I didn't do it like that originally. | |
| 14:53:21 | mriedem | still looking for core reviews on this upgrade issue for cells v2 https://review.openstack.org/#/c/464088/ | |
| 14:53:28 | mriedem | ^ needs to go back through newton | |
| 15:06:45 | cfriesen | kashyap, you on? | |
| 15:10:43 | sdague | dansmith / melwitt was anything figured out about db connection leaks yesterday? | |
| 15:10:55 | cfriesen | there's someone on the openstack-dev mailing list asking about whether the cpu-throttle-initial and cpu-throttle-increment options in libvirt auto-converge are stable enough to use....anyone have any thoughts? The Newton spec said not, but that was a while ago now. | |
| 15:11:07 | dansmith | sdague: earlier this morning I said that raising ulimit to actually allow the number of connections we have configured seems to resolve my problem | |
| 15:11:26 | sdague | oh, I see it now | |
| 15:11:35 | dansmith | sdague: I'm about finished with a second run to get another sample, which appears to also have worked | |
| 15:11:44 | mriedem | shouldn't ulimit be 64k in deployment now? | |
| 15:11:48 | sdague | ok, cool, because our ulimit is 1024 by default or something | |
| 15:11:51 | mriedem | deployment == devstack? | |
| 15:12:00 | dansmith | sdague: this bit: https://review.openstack.org/#/c/436094/28/lib/databases/mysql | |
| 15:13:40 | sdague | ok cool | |
| 15:14:28 | dansmith | sdague: so, I dunno whether to just (ask you to allow) that into devstack or what | |
| 15:14:29 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Use plain routes list for os-services endpoint instead of stevedore https://review.openstack.org/460818 | |
| 15:14:48 | dansmith | sdague: 1024 still seems high, but we do have a bunch of databases, tables, and connections going on... | |
| 15:15:21 | sdague | dansmith: yeh, if we've got 1024 db connections it feels like ulimit should at least be double that | |
| 15:15:38 | dansmith | okay, I'll split that out and make it 2048 | |
| 15:15:43 | sdague | otherwise it's just shooting ourselves in the foot | |
| 15:15:47 | dansmith | right | |
| 15:16:24 | sdague | dansmith: so... here is what I think we should do | |
| 15:16:36 | sdague | add a new stackrc variable so that's tunable | |
| 15:16:42 | sdague | default to 2048 | |
| 15:16:45 | dansmith | okay | |
| 15:16:50 | sdague | do the change early in stack.sh for the system | |
| 15:17:07 | sdague | then if people want to play around with other values for constraints, it's easy | |
| 15:17:15 | dansmith | okay | |
| 15:17:38 | dansmith | by early does that mean in stack.sh or in functions somewhere? | |
| 15:18:27 | dansmith | and by "for the system" you mean in limits.conf? | |
| 15:19:02 | dansmith | and hope that systemd just inherits that properly when we start mysql? | |
| 15:19:30 | openstackgerrit | Stephen Finucane proposed openstack/nova master: rbd: Remove unnecessary 'encode' calls https://review.openstack.org/412356 | |
| 15:21:03 | sdague | dansmith: well probably build a function for it, but call it before we start any services in stack.sh directly | |
| 15:21:08 | sdague | yeh, limits.conf | |
| 15:21:15 | dansmith | okay | |
| 15:21:23 | sdague | and if set, I'm hoping it inherits through for the world | |
| 15:22:42 | dansmith | yeah, limits.conf is read by pam, so I'm not sure | |
| 15:24:30 | dansmith | sdague: just tested, doesn't seem to have any effect | |
| 15:24:39 | dansmith | system limits on the service Imean | |
| 15:24:47 | dansmith | even with a daemon-reload | |
| 15:25:17 | mdbooth | https://access.redhat.com/solutions/1257953 | |
| 15:25:27 | mdbooth | Although I think that's paywalled | |
| 15:25:34 | dansmith | mdbooth: I already set it with systemd and it works | |
| 15:25:42 | dansmith | we were hoping we could do it generically with limits.conf | |
| 15:25:49 | mdbooth | But yeah, systemd doesn't read limits.conf | |
| 15:25:57 | dansmith | sdague: ^ | |
| 15:26:19 | dansmith | sdague: so I'm guessing I just make that stackrc var mysql-specific | |
| 15:27:10 | dansmith | or I can just comment over it that it's only enforced for things that look at it (like mysql) | |
| 15:29:10 | cfriesen | dansmith: would something like this be okay, or is there some reason it would be terrible? http://paste.openstack.org/show/609690/ We're seeing the cache update be somewhat expensive and wondering if we can just batch the cache updates when there are multiple network-changed events in a batch. | |
| 15:30:26 | dansmith | cfriesen: terrible? you'd never notice cache changes past the first one | |
| 15:31:47 | mdbooth | cfriesen: Another problem with this kind of cache is that they grow unbounded. Possibly not a practical issue with instance uuids, but still icky. | |
| 15:34:43 | cfriesen | dansmith: could we remove all network-changed events for this instance from the "events" list, then call self.network_api.get_instance_nw_info()? | |
| 15:35:05 | sfinucan | mriedem: Could you look at the 'reserve-numa-with-pci' patches at some point this week or next? https://review.openstack.org/#/q/status:open+project:openstack/nova+topic:bp/reserve-numa-with-pci | |
| 15:35:26 | mriedem | maybe, artom is in front of you | |
| 15:35:32 | cfriesen | dansmith: it seems pointless to call it multiple times if it's going to give us the same answer each time | |
| 15:35:34 | dansmith | cfriesen: eh? the events list is transient.. things get removed from there when processed | |
| 15:35:42 | sdague | hmmmm | |
| 15:35:43 | dansmith | cfriesen: that's the nature of a cache refresh | |
| 15:36:09 | sdague | dansmith: well, what's the set with systemd option? Build that unit file? | |
| 15:36:28 | dansmith | cfriesen: maybe I can't see enough of your change.. are you saying you see a single request to refresh an instance multiple times in the same go? | |
| 15:36:30 | dansmith | sdague: yeah | |
| 15:36:51 | sdague | dansmith: I'm fine fixing it just for the systemd case | |
| 15:36:58 | sdague | we're default there everywhere | |
| 15:37:07 | dansmith | sdague: okay, but it'll only be for mysql | |
| 15:37:19 | sdague | oh ... let me go look again | |
| 15:37:26 | cfriesen | dansmith: we are coming into this code and there are multiple network-changed events for the same instance, so we end up calling self.network_api.get_instance_nw_info() multiple times | |
| 15:37:45 | dansmith | cfriesen: ah, okay I assumed this was in the periodic | |
| 15:37:49 | dansmith | cfriesen: why is that happening? | |