From: Casey Bodley Date: Thu, 30 Aug 2018 17:16:45 +0000 (-0400) Subject: rgw: cleanups for sync tracing X-Git-Tag: v14.0.1~374^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f71a21c4e844f4f84439a7b4a5aed84dd0111a78;p=ceph.git rgw: cleanups for sync tracing RGWSyncTraceManager::add_node() now allocates the nodes themselves. this simplifies some of the interactions between the Manager and Node classes, and saves a lot of typing for the callers Signed-off-by: Casey Bodley --- diff --git a/src/rgw/rgw_data_sync.cc b/src/rgw/rgw_data_sync.cc index ca98c0c88653..81756d770c2d 100644 --- a/src/rgw/rgw_data_sync.cc +++ b/src/rgw/rgw_data_sync.cc @@ -519,7 +519,8 @@ public: rgw_data_sync_status *status) : RGWCoroutine(_sync_env->cct), sync_env(_sync_env), store(sync_env->store), pool(store->get_zone_params().log_pool), - num_shards(num_shards), status(status) { + num_shards(num_shards), status(status), + tn(sync_env->sync_tracer->add_node(_tn_parent, "init_data_sync_status")) { lock_name = "sync_lock"; status->sync_info.instance_id = instance_id; @@ -532,11 +533,6 @@ public: sync_status_oid = RGWDataSyncStatusManager::sync_status_oid(sync_env->source_zone); - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "init_data_sync_status", - string())); } int operate() override { @@ -677,11 +673,7 @@ int RGWRemoteDataLog::init(const string& _source_zone, RGWRESTConn *_conn, RGWSy return ret; } - tn = sync_env.sync_tracer->add_node(new RGWSyncTraceNode(sync_env.cct, - sync_env.sync_tracer, - sync_env.sync_tracer->root_node, - "data", - string())); + tn = sync_env.sync_tracer->add_node(sync_env.sync_tracer->root_node, "data"); initialized = true; @@ -1024,14 +1016,10 @@ class RGWRunBucketSyncCoroutine : public RGWCoroutine { public: RGWRunBucketSyncCoroutine(RGWDataSyncEnv *_sync_env, const rgw_bucket_shard& bs, const RGWSyncTraceNodeRef& _tn_parent) : RGWCoroutine(_sync_env->cct), sync_env(_sync_env), bs(bs), - status_oid(RGWBucketSyncStatusManager::status_oid(sync_env->source_zone, bs)) { + status_oid(RGWBucketSyncStatusManager::status_oid(sync_env->source_zone, bs)), + tn(sync_env->sync_tracer->add_node(_tn_parent, "bucket", + SSTR(bucket_shard_str{bs}))) { logger.init(sync_env, "Bucket", bs.get_key()); - string bss = SSTR(bucket_shard_str{bs}); - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "bucket", - bss)); } ~RGWRunBucketSyncCoroutine() override { if (lease_cr) { @@ -1072,11 +1060,7 @@ public: marker_tracker(_marker_tracker), error_repo(_error_repo), remove_from_repo(_remove_from_repo) { set_description() << "data sync single entry (source_zone=" << sync_env->source_zone << ") key=" <<_raw_key << " entry=" << entry_marker; - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "entry", - raw_key)); + tn = sync_env->sync_tracer->add_node(_tn_parent, "entry", raw_key); } int operate() override { @@ -1525,11 +1509,7 @@ public: pool(_pool), shard_id(_shard_id), sync_marker(_marker) { - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "shard", - SSTR(shard_id))); + tn = sync_env->sync_tracer->add_node(_tn_parent, "shard", std::to_string(shard_id)); } RGWCoroutine *alloc_cr() override { @@ -1746,11 +1726,7 @@ public: RGWDataSyncControlCR(RGWDataSyncEnv *_sync_env, uint32_t _num_shards, RGWSyncTraceNodeRef& _tn_parent) : RGWBackoffControlCR(_sync_env->cct, exit_on_error), sync_env(_sync_env), num_shards(_num_shards) { - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "sync", - string())); + tn = sync_env->sync_tracer->add_node(_tn_parent, "sync"); } RGWCoroutine *alloc_cr() override { @@ -2586,11 +2562,7 @@ public: set_status("init"); logger.init(sync_env, "Object", ss.str()); - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "entry", - SSTR(key))); + tn = sync_env->sync_tracer->add_node(_tn_parent, "entry", SSTR(key)); tn->log(20, SSTR("bucket sync single entry (source_zone=" << sync_env->source_zone << ") b=" << ss.str() << " log_entry=" << entry_marker << " op=" << (int)op << " op_state=" << (int)op_state)); error_injection = (sync_env->cct->_conf->rgw_sync_data_inject_err_probability > 0); @@ -2717,14 +2689,11 @@ public: : RGWCoroutine(_sync_env->cct), sync_env(_sync_env), bs(bs), bucket_info(_bucket_info), lease_cr(lease_cr), sync_info(sync_info), marker_tracker(sync_env, status_oid, sync_info.full_marker), - status_oid(status_oid) { + status_oid(status_oid), + tn(sync_env->sync_tracer->add_node(tn_parent, "full_sync", + SSTR(bucket_shard_str{bs}))) { logger.init(sync_env, "BucketFull", bs.get_key()); zones_trace.insert(sync_env->source_zone); - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - tn_parent, - "full_sync", - SSTR(bucket_shard_str{bs}))); marker_tracker.set_tn(tn); } @@ -2876,17 +2845,14 @@ public: : RGWCoroutine(_sync_env->cct), sync_env(_sync_env), bs(bs), bucket_info(_bucket_info), lease_cr(lease_cr), sync_info(sync_info), marker_tracker(sync_env, status_oid, sync_info.inc_marker), - status_oid(status_oid), zone_id(_sync_env->store->get_zone().id) + status_oid(status_oid), zone_id(_sync_env->store->get_zone().id), + tn(sync_env->sync_tracer->add_node(_tn_parent, "inc_sync", + SSTR(bucket_shard_str{bs}))) { set_description() << "bucket shard incremental sync bucket=" << bucket_shard_str{bs}; set_status("init"); logger.init(sync_env, "BucketInc", bs.get_key()); - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "inc_sync", - SSTR(bucket_shard_str{bs}))); marker_tracker.set_tn(tn); } diff --git a/src/rgw/rgw_sync.cc b/src/rgw/rgw_sync.cc index 70f9328399db..5f03da96a141 100644 --- a/src/rgw/rgw_sync.cc +++ b/src/rgw/rgw_sync.cc @@ -287,11 +287,7 @@ int RGWRemoteMetaLog::init() init_sync_env(&sync_env); - tn = sync_env.sync_tracer->add_node(new RGWSyncTraceNode(sync_env.cct, - sync_env.sync_tracer, - sync_env.sync_tracer->root_node, - "meta", - string())); + tn = sync_env.sync_tracer->add_node(sync_env.sync_tracer->root_node, "meta"); return 0; } @@ -819,11 +815,7 @@ public: num_shards(_num_shards), ret_status(0), lease_cr(nullptr), lease_stack(nullptr), lost_lock(false), failed(false), markers(_markers) { - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "fetch_all_meta", - string())); + tn = sync_env->sync_tracer->add_node(_tn_parent, "fetch_all_meta"); } ~RGWFetchAllMetaCR() override { @@ -1013,11 +1005,8 @@ public: section(_section), key(_key), pbl(_pbl) { - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "read_remote_meta", - section + ":" + key)); + tn = sync_env->sync_tracer->add_node(_tn_parent, "read_remote_meta", + section + ":" + key); } int operate() override { @@ -1231,11 +1220,7 @@ RGWMetaSyncSingleEntryCR::RGWMetaSyncSingleEntryCR(RGWMetaSyncEnv *_sync_env, pos(0), sync_status(0), marker_tracker(_marker_tracker), tries(0) { error_injection = (sync_env->cct->_conf->rgw_sync_meta_inject_err_probability > 0); - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - _tn_parent, - "entry", - raw_key)); + tn = sync_env->sync_tracer->add_node(_tn_parent, "entry", raw_key); } int RGWMetaSyncSingleEntryCR::operate() { @@ -1844,7 +1829,6 @@ class RGWMetaSyncShardControlCR : public RGWBackoffControlCR rgw_meta_sync_marker sync_marker; const std::string period_marker; - RGWSyncTraceNodeRef tn_parent; RGWSyncTraceNodeRef tn; static constexpr bool exit_on_error = false; // retry on all errors @@ -1858,14 +1842,10 @@ public: : RGWBackoffControlCR(_sync_env->cct, exit_on_error), sync_env(_sync_env), pool(_pool), period(period), realm_epoch(realm_epoch), mdlog(mdlog), shard_id(_shard_id), sync_marker(_marker), - period_marker(std::move(period_marker)), - tn_parent(_tn_parent) { - tn = sync_env->sync_tracer->add_node(new RGWSyncTraceNode(sync_env->cct, - sync_env->sync_tracer, - tn_parent, - "shard", - SSTR(shard_id))); - } + period_marker(std::move(period_marker)) { + tn = sync_env->sync_tracer->add_node(_tn_parent, "shard", + std::to_string(shard_id)); + } RGWCoroutine *alloc_cr() override { return new RGWMetaSyncShardCR(sync_env, pool, period, realm_epoch, mdlog, diff --git a/src/rgw/rgw_sync_trace.cc b/src/rgw/rgw_sync_trace.cc index 8b3dd8cd802b..1566ff4c8b03 100644 --- a/src/rgw/rgw_sync_trace.cc +++ b/src/rgw/rgw_sync_trace.cc @@ -13,13 +13,14 @@ #define dout_context g_ceph_context #define dout_subsys ceph_subsys_rgw_sync -RGWSyncTraceNode::RGWSyncTraceNode(CephContext *_cct, RGWSyncTraceManager *_manager, +RGWSyncTraceNode::RGWSyncTraceNode(CephContext *_cct, uint64_t _handle, const RGWSyncTraceNodeRef& _parent, const string& _type, const string& _id) : cct(_cct), - manager(_manager), parent(_parent), type(_type), - id(_id), history(cct->_conf->rgw_sync_trace_per_node_log_size) + id(_id), + handle(_handle), + history(cct->_conf->rgw_sync_trace_per_node_log_size) { if (parent.get()) { prefix = parent->get_prefix(); @@ -32,7 +33,6 @@ RGWSyncTraceNode::RGWSyncTraceNode(CephContext *_cct, RGWSyncTraceManager *_mana } prefix += ":"; } - handle = manager->alloc_handle(); } void RGWSyncTraceNode::log(int level, const string& s) @@ -49,11 +49,6 @@ void RGWSyncTraceNode::log(int level, const string& s) } } -void RGWSyncTraceNode::finish() -{ - manager->finish_node(this); -} - class RGWSyncTraceServiceMapThread : public RGWRadosThread { RGWRados *store; @@ -80,15 +75,18 @@ int RGWSyncTraceServiceMapThread::process() return 0; } -RGWSyncTraceNodeRef RGWSyncTraceManager::add_node(RGWSyncTraceNode *node) +RGWSyncTraceNodeRef RGWSyncTraceManager::add_node(const RGWSyncTraceNodeRef& parent, + const std::string& type, + const std::string& id) { shunique_lock wl(lock, ceph::acquire_unique); - RGWSyncTraceNodeRef& ref = nodes[node->handle]; - ref.reset(node); + auto handle = alloc_handle(); + RGWSyncTraceNodeRef& ref = nodes[handle]; + ref.reset(new RGWSyncTraceNode(cct, handle, parent, type, id)); // return a separate shared_ptr that calls finish() on the node instead of // deleting it. the lambda capture holds a reference to the original 'ref' - auto deleter = [ref] (RGWSyncTraceNode *node) { node->finish(); }; - return {node, deleter}; + auto deleter = [ref, this] (RGWSyncTraceNode *node) { finish_node(node); }; + return {ref.get(), deleter}; } bool RGWSyncTraceNode::match(const string& search_term, bool search_history) diff --git a/src/rgw/rgw_sync_trace.h b/src/rgw/rgw_sync_trace.h index 8fe9a8a6f837..2a372f3a34fc 100644 --- a/src/rgw/rgw_sync_trace.h +++ b/src/rgw/rgw_sync_trace.h @@ -31,12 +31,10 @@ class RGWSyncTraceServiceMapThread; using RGWSyncTraceNodeRef = std::shared_ptr; -class RGWSyncTraceNode { +class RGWSyncTraceNode final { friend class RGWSyncTraceManager; CephContext *cct; - - RGWSyncTraceManager *manager{nullptr}; RGWSyncTraceNodeRef parent; uint16_t state{0}; @@ -44,7 +42,6 @@ class RGWSyncTraceNode { Mutex lock{"RGWSyncTraceNode::lock"}; -protected: std::string type; std::string id; @@ -55,10 +52,13 @@ protected: uint64_t handle; boost::circular_buffer history; -public: - RGWSyncTraceNode(CephContext *_cct, RGWSyncTraceManager *_manager, const RGWSyncTraceNodeRef& _parent, - const std::string& _type, const std::string& _id); + // private constructor, create with RGWSyncTraceManager::add_node() + RGWSyncTraceNode(CephContext *_cct, uint64_t _handle, + const RGWSyncTraceNodeRef& _parent, + const std::string& _type, const std::string& _id); + + public: void set_resource_name(const string& s) { resource_name = s; } @@ -77,7 +77,6 @@ public: return (state & f) == f; } void log(int level, const std::string& s); - void finish(); std::string to_str() { return prefix + " " + status; @@ -114,10 +113,11 @@ class RGWSyncTraceManager : public AdminSocketHook { std::atomic count = { 0 }; std::list > admin_commands; -protected: + uint64_t alloc_handle() { return ++count; } + void finish_node(RGWSyncTraceNode *node); public: RGWSyncTraceManager(CephContext *_cct, int max_lru) : cct(_cct), complete_nodes(max_lru) {} @@ -127,8 +127,9 @@ public: const RGWSyncTraceNodeRef root_node; - RGWSyncTraceNodeRef add_node(RGWSyncTraceNode *node); - void finish_node(RGWSyncTraceNode *node); + RGWSyncTraceNodeRef add_node(const RGWSyncTraceNodeRef& parent, + const std::string& type, + const std::string& id = ""); int hook_to_admin_command(); bool call(std::string_view command, const cmdmap_t& cmdmap,