On 03/17/2018 03:44 PM, Radoslaw Zarzynski wrote:
Hello, some time ago I've promised to Greg a post highlighting performance issue that was discovered couple weeks ago: many calls to the costly `md_config_t::get_val<T>` on OSD's hot paths. The problem is that the method really needs to acquire a mutex and isn't supposed to be used in performance-critical situations. For handling them the config rework designated the `md_config_obs_t` interface. Implementing it allows clients to locally cache setting's value giving very fast access. Though, it's understandable that one, single call is much easier to use as it doesn't require any boilerplate. Because of that a grain of sugar was added: `md_config_cacher_t` [1]. It handles registration/value update/unregistration simplifying the observer usage in typical cases. The idea [2] is to make `md_config_cacher_t<SettingT>` a member of a longly living instance (e.g. shard), then initialize it with a name of interesting configurable (+ a reference to md_config_t). Obtaining the cached value is simplified thanks to the implicit conversion to `SettingT`. At the moment there is a limitation on `SettingT`: it can't be std::string due to the std::atomic<> config cacher uses inside. Surely, there is no business in converting everything to use the config observer pattern. `get_val<T>` is perfectly fine during initialization, error handling and generally on all infrequent paths. Only the hottest ones might be an issue.
FWIW, I saw this too when I was testing petstore. In high performance scenarios md_config_t::get_val<T> was taking up somewhere around 5-10% of each tp_osd_tp thread's wallclock time (also in several other threads), and that was only likely to get worse as configuration values are moved to the new scheme. Using get_val in the hot path is really bad.
Mark
Best regards. Radosław Zarzyński `md_config_cacher_t` implementation: [1] https://github.com/ceph/ceph/blob/master/src/common/config_cacher.h Example of usage: [2] https://github.com/ceph/ceph/pull/20320/commits/248750fea441efd16c1a431617cfbd1f868a6eeb -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
-- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
