From d93955997a7bbb6eadbc900d037a83e4a417236c Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 12 May 2017 22:34:33 -0400 Subject: [PATCH] blkin: rbd object map update traces Signed-off-by: Jason Dillaman --- src/librbd/ObjectMap.cc | 5 ++-- src/librbd/ObjectMap.h | 23 ++++++++++--------- src/librbd/io/CopyupRequest.cc | 16 +++++++------ src/librbd/io/ObjectRequest.cc | 5 ++-- src/librbd/object_map/UpdateRequest.cc | 5 +++- src/librbd/object_map/UpdateRequest.h | 15 +++++++++--- src/librbd/operation/TrimRequest.cc | 8 +++---- src/test/librbd/mock/MockObjectMap.h | 12 ++++++---- .../object_map/test_mock_UpdateRequest.cc | 10 ++++---- src/test/librbd/test_mock_ObjectMap.cc | 13 ++++++----- .../image_sync/test_mock_ObjectCopyRequest.cc | 8 +++---- .../image_sync/ObjectCopyRequest.cc | 2 +- 12 files changed, 71 insertions(+), 51 deletions(-) diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index 49c8280dae8c6..9a0683fc68dc4 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -242,7 +242,7 @@ void ObjectMap::detained_aio_update(UpdateOperation &&op) { handle_detained_aio_update(cell, r, on_finish); }); aio_update(CEPH_NOSNAP, op.start_object_no, op.end_object_no, op.new_state, - op.current_state, ctx); + op.current_state, op.parent_trace, ctx); } template @@ -269,6 +269,7 @@ template void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, + const ZTracer::Trace &parent_trace, Context *on_finish) { assert(m_image_ctx.snap_lock.is_locked()); assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0); @@ -306,7 +307,7 @@ void ObjectMap::aio_update(uint64_t snap_id, uint64_t start_object_no, auto req = object_map::UpdateRequest::create( m_image_ctx, &m_object_map, snap_id, start_object_no, end_object_no, - new_state, current_state, on_finish); + new_state, current_state, parent_trace, on_finish); req->send(); } diff --git a/src/librbd/ObjectMap.h b/src/librbd/ObjectMap.h index fd43ae97d23b0..427ecdf165a37 100644 --- a/src/librbd/ObjectMap.h +++ b/src/librbd/ObjectMap.h @@ -13,9 +13,8 @@ class Context; class RWLock; -namespace librados { - class IoCtx; -} +namespace librados { class IoCtx; } +namespace ZTracer { struct Trace; } namespace librbd { @@ -57,16 +56,17 @@ public: template bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint8_t new_state, const boost::optional ¤t_state, - T *callback_object) { + const ZTracer::Trace &parent_trace, T *callback_object) { return aio_update(snap_id, start_object_no, start_object_no + 1, - new_state, current_state, callback_object); + new_state, current_state, parent_trace, + callback_object); } template bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, - T *callback_object) { + const ZTracer::Trace &parent_trace, T *callback_object) { assert(start_object_no < end_object_no); if (snap_id == CEPH_NOSNAP) { uint64_t object_no; @@ -82,13 +82,13 @@ public: } UpdateOperation update_operation(start_object_no, end_object_no, - new_state, current_state, + new_state, current_state, parent_trace, util::create_context_callback( callback_object)); detained_aio_update(std::move(update_operation)); } else { aio_update(snap_id, start_object_no, end_object_no, new_state, - current_state, + current_state, parent_trace, util::create_context_callback(callback_object)); } return true; @@ -104,15 +104,16 @@ private: uint64_t end_object_no; uint8_t new_state; boost::optional current_state; + ZTracer::Trace parent_trace; Context *on_finish; UpdateOperation(uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, - Context *on_finish) + const ZTracer::Trace &parent_trace, Context *on_finish) : start_object_no(start_object_no), end_object_no(end_object_no), new_state(new_state), current_state(current_state), - on_finish(on_finish) { + parent_trace(parent_trace), on_finish(on_finish) { } }; @@ -131,7 +132,7 @@ private: void aio_update(uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, - Context *on_finish); + const ZTracer::Trace &parent_trace, Context *on_finish); bool update_required(uint64_t object_no, uint8_t new_state); }; diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index 395ee7762071f..aee2f14d7b065 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -35,9 +35,9 @@ class UpdateObjectMap : public C_AsyncObjectThrottle<> { public: UpdateObjectMap(AsyncObjectThrottle<> &throttle, ImageCtx *image_ctx, uint64_t object_no, const std::vector *snap_ids, - size_t snap_id_idx) - : C_AsyncObjectThrottle(throttle, *image_ctx), - m_object_no(object_no), m_snap_ids(*snap_ids), m_snap_id_idx(snap_id_idx) + const ZTracer::Trace &trace, size_t snap_id_idx) + : C_AsyncObjectThrottle(throttle, *image_ctx), m_object_no(object_no), + m_snap_ids(*snap_ids), m_trace(trace), m_snap_id_idx(snap_id_idx) { } @@ -49,7 +49,7 @@ public: assert(m_image_ctx.exclusive_lock->is_lock_owner()); assert(m_image_ctx.object_map != nullptr); bool sent = m_image_ctx.object_map->aio_update( - CEPH_NOSNAP, m_object_no, OBJECT_EXISTS, {}, this); + CEPH_NOSNAP, m_object_no, OBJECT_EXISTS, {}, m_trace, this); return (sent ? 0 : 1); } @@ -66,7 +66,7 @@ public: } bool sent = m_image_ctx.object_map->aio_update( - snap_id, m_object_no, state, {}, this); + snap_id, m_object_no, state, {}, m_trace, this); assert(sent); return 0; } @@ -74,6 +74,7 @@ public: private: uint64_t m_object_no; const std::vector &m_snap_ids; + const ZTracer::Trace &m_trace; size_t m_snap_id_idx; }; @@ -335,7 +336,8 @@ bool CopyupRequest::send_object_map_head() { if (may_update && (new_state != current_state) && m_ictx->object_map->aio_update( - CEPH_NOSNAP, m_object_no, new_state, current_state, this)) { + CEPH_NOSNAP, m_object_no, new_state, current_state, m_trace, + this)) { return false; } } @@ -357,7 +359,7 @@ bool CopyupRequest::send_object_map() { RWLock::RLocker owner_locker(m_ictx->owner_lock); AsyncObjectThrottle<>::ContextFactory context_factory( boost::lambda::bind(boost::lambda::new_ptr(), - boost::lambda::_1, m_ictx, m_object_no, &m_snap_ids, + boost::lambda::_1, m_ictx, m_object_no, &m_snap_ids, m_trace, boost::lambda::_2)); AsyncObjectThrottle<> *throttle = new AsyncObjectThrottle<>( NULL, *m_ictx, context_factory, util::create_context_callback(this), diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 59e984a4ae400..1cdb696b7171b 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -489,7 +489,7 @@ void AbstractObjectWriteRequest::send_pre_object_map_update() { m_state = LIBRBD_AIO_WRITE_PRE; if (m_ictx->object_map->aio_update( - CEPH_NOSNAP, m_object_no, new_state, {}, this)) { + CEPH_NOSNAP, m_object_no, new_state, {}, this->m_trace, this)) { return; } } @@ -515,7 +515,8 @@ bool AbstractObjectWriteRequest::send_post_object_map_update() { m_state = LIBRBD_AIO_WRITE_POST; if (m_ictx->object_map->aio_update( - CEPH_NOSNAP, m_object_no, OBJECT_NONEXISTENT, OBJECT_PENDING, this)) { + CEPH_NOSNAP, m_object_no, OBJECT_NONEXISTENT, OBJECT_PENDING, + this->m_trace, this)) { return false; } diff --git a/src/librbd/object_map/UpdateRequest.cc b/src/librbd/object_map/UpdateRequest.cc index e88085add4115..8c0ec69de8d15 100644 --- a/src/librbd/object_map/UpdateRequest.cc +++ b/src/librbd/object_map/UpdateRequest.cc @@ -42,7 +42,10 @@ void UpdateRequest::send() { m_new_state, m_current_state); librados::AioCompletion *rados_completion = create_callback_completion(); - int r = m_image_ctx.md_ctx.aio_operate(oid, rados_completion, &op); + std::vector snaps; + int r = m_image_ctx.md_ctx.aio_operate( + oid, rados_completion, &op, 0, snaps, + (m_trace.valid() ? m_trace.get_info() : nullptr)); assert(r == 0); rados_completion->release(); } diff --git a/src/librbd/object_map/UpdateRequest.h b/src/librbd/object_map/UpdateRequest.h index 0bb9cd756e293..175160752dac7 100644 --- a/src/librbd/object_map/UpdateRequest.h +++ b/src/librbd/object_map/UpdateRequest.h @@ -7,6 +7,8 @@ #include "include/int_types.h" #include "librbd/object_map/Request.h" #include "common/bit_vector.hpp" +#include "common/zipkin_trace.h" +#include "librbd/Utils.h" #include class Context; @@ -25,21 +27,27 @@ public: uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, + const ZTracer::Trace &parent_trace, Context *on_finish) { return new UpdateRequest(image_ctx, object_map, snap_id, start_object_no, end_object_no, new_state, current_state, - on_finish); + parent_trace, on_finish); } UpdateRequest(ImageCtx &image_ctx, ceph::BitVector<2> *object_map, uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, - Context *on_finish) + const ZTracer::Trace &parent_trace, Context *on_finish) : Request(image_ctx, snap_id, on_finish), m_object_map(*object_map), m_start_object_no(start_object_no), m_end_object_no(end_object_no), - m_new_state(new_state), m_current_state(current_state) + m_new_state(new_state), m_current_state(current_state), + m_trace(util::create_trace(image_ctx, "update object map", parent_trace)) { + m_trace.event("start"); + } + virtual ~UpdateRequest() { + m_trace.event("finish"); } void send() override; @@ -53,6 +61,7 @@ private: uint64_t m_end_object_no; uint8_t m_new_state; boost::optional m_current_state; + ZTracer::Trace m_trace; }; } // namespace object_map diff --git a/src/librbd/operation/TrimRequest.cc b/src/librbd/operation/TrimRequest.cc index 60b0c79f65d2c..46ec967b5eddc 100644 --- a/src/librbd/operation/TrimRequest.cc +++ b/src/librbd/operation/TrimRequest.cc @@ -277,7 +277,7 @@ void TrimRequest::send_pre_copyup() { RWLock::WLocker object_map_locker(image_ctx.object_map_lock); if (image_ctx.object_map->template aio_update >( CEPH_NOSNAP, m_copyup_start, m_copyup_end, OBJECT_PENDING, - OBJECT_EXISTS, this)) { + OBJECT_EXISTS, {}, this)) { return; } } @@ -309,7 +309,7 @@ void TrimRequest::send_pre_remove() { RWLock::WLocker object_map_locker(image_ctx.object_map_lock); if (image_ctx.object_map->template aio_update >( CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_PENDING, - OBJECT_EXISTS, this)) { + OBJECT_EXISTS, {}, this)) { return; } } @@ -337,7 +337,7 @@ void TrimRequest::send_post_copyup() { RWLock::WLocker object_map_locker(image_ctx.object_map_lock); if (image_ctx.object_map->template aio_update >( CEPH_NOSNAP, m_copyup_start, m_copyup_end, OBJECT_NONEXISTENT, - OBJECT_PENDING, this)) { + OBJECT_PENDING, {}, this)) { return; } } @@ -365,7 +365,7 @@ void TrimRequest::send_post_remove() { RWLock::WLocker object_map_locker(image_ctx.object_map_lock); if (image_ctx.object_map->template aio_update >( CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_NONEXISTENT, - OBJECT_PENDING, this)) { + OBJECT_PENDING, {}, this)) { return; } } diff --git a/src/test/librbd/mock/MockObjectMap.h b/src/test/librbd/mock/MockObjectMap.h index 057364d53e730..9ace5e374800e 100644 --- a/src/test/librbd/mock/MockObjectMap.h +++ b/src/test/librbd/mock/MockObjectMap.h @@ -22,23 +22,25 @@ struct MockObjectMap { template bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint8_t new_state, const boost::optional ¤t_state, - T *callback_object) { + const ZTracer::Trace &parent_trace, T *callback_object) { return aio_update(snap_id, start_object_no, start_object_no + 1, - new_state, current_state, callback_object); + new_state, current_state, parent_trace, + callback_object); } template bool aio_update(uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, - T *callback_object) { + const ZTracer::Trace &parent_trace, T *callback_object) { return aio_update(snap_id, start_object_no, end_object_no, new_state, - current_state, + current_state, parent_trace, util::create_context_callback(callback_object)); } - MOCK_METHOD6(aio_update, bool(uint64_t snap_id, uint64_t start_object_no, + MOCK_METHOD7(aio_update, bool(uint64_t snap_id, uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, + const ZTracer::Trace &parent_trace, Context *on_finish)); MOCK_METHOD2(snapshot_add, void(uint64_t snap_id, Context *on_finish)); MOCK_METHOD2(snapshot_remove, void(uint64_t snap_id, Context *on_finish)); diff --git a/src/test/librbd/object_map/test_mock_UpdateRequest.cc b/src/test/librbd/object_map/test_mock_UpdateRequest.cc index ea40404d39b1d..7f47be28176ef 100644 --- a/src/test/librbd/object_map/test_mock_UpdateRequest.cc +++ b/src/test/librbd/object_map/test_mock_UpdateRequest.cc @@ -68,7 +68,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateInMemory) { C_SaferCond cond_ctx; AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT, - OBJECT_EXISTS, &cond_ctx); + OBJECT_EXISTS, {}, &cond_ctx); { RWLock::RLocker snap_locker(ictx->snap_lock); RWLock::WLocker object_map_locker(ictx->object_map_lock); @@ -100,7 +100,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateHeadOnDisk) { C_SaferCond cond_ctx; AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT, - OBJECT_EXISTS, &cond_ctx); + OBJECT_EXISTS, {}, &cond_ctx); { RWLock::RLocker snap_locker(ictx->snap_lock); RWLock::WLocker object_map_locker(ictx->object_map_lock); @@ -130,7 +130,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateSnapOnDisk) { C_SaferCond cond_ctx; AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_NONEXISTENT, - OBJECT_EXISTS, &cond_ctx); + OBJECT_EXISTS, {}, &cond_ctx); { RWLock::RLocker snap_locker(ictx->snap_lock); RWLock::WLocker object_map_locker(ictx->object_map_lock); @@ -157,7 +157,7 @@ TEST_F(TestMockObjectMapUpdateRequest, UpdateOnDiskError) { C_SaferCond cond_ctx; AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, CEPH_NOSNAP, 0, object_map.size(), OBJECT_NONEXISTENT, - OBJECT_EXISTS, &cond_ctx); + OBJECT_EXISTS, {}, &cond_ctx); { RWLock::RLocker snap_locker(ictx->snap_lock); RWLock::WLocker object_map_locker(ictx->object_map_lock); @@ -187,7 +187,7 @@ TEST_F(TestMockObjectMapUpdateRequest, RebuildSnapOnDisk) { C_SaferCond cond_ctx; AsyncRequest<> *req = new UpdateRequest<>( *ictx, &object_map, snap_id, 0, object_map.size(), OBJECT_EXISTS_CLEAN, - boost::optional(), &cond_ctx); + boost::optional(), {}, &cond_ctx); { RWLock::RLocker snap_locker(ictx->snap_lock); RWLock::WLocker object_map_locker(ictx->object_map_lock); diff --git a/src/test/librbd/test_mock_ObjectMap.cc b/src/test/librbd/test_mock_ObjectMap.cc index f14c6b9a5b274..7deaef03bb8ea 100644 --- a/src/test/librbd/test_mock_ObjectMap.cc +++ b/src/test/librbd/test_mock_ObjectMap.cc @@ -70,6 +70,7 @@ struct UpdateRequest { uint64_t start_object_no, uint64_t end_object_no, uint8_t new_state, const boost::optional ¤t_state, + const ZTracer::Trace &parent_trace, Context *on_finish) { assert(s_instance != nullptr); s_instance->on_finish = on_finish; @@ -180,8 +181,8 @@ TEST_F(TestMockObjectMap, NonDetainedUpdate) { { RWLock::RLocker snap_locker(mock_image_ctx.snap_lock); RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock); - mock_object_map.aio_update(CEPH_NOSNAP, 0, 1, {}, &update_ctx1); - mock_object_map.aio_update(CEPH_NOSNAP, 1, 1, {}, &update_ctx2); + mock_object_map.aio_update(CEPH_NOSNAP, 0, 1, {}, {}, &update_ctx1); + mock_object_map.aio_update(CEPH_NOSNAP, 1, 1, {}, {}, &update_ctx2); } finish_update_2->complete(0); @@ -238,10 +239,10 @@ TEST_F(TestMockObjectMap, DetainedUpdate) { { RWLock::RLocker snap_locker(mock_image_ctx.snap_lock); RWLock::WLocker object_map_locker(mock_image_ctx.object_map_lock); - mock_object_map.aio_update(CEPH_NOSNAP, 1, 4, 1, {}, &update_ctx1); - mock_object_map.aio_update(CEPH_NOSNAP, 1, 3, 1, {}, &update_ctx2); - mock_object_map.aio_update(CEPH_NOSNAP, 2, 3, 1, {}, &update_ctx3); - mock_object_map.aio_update(CEPH_NOSNAP, 0, 2, 1, {}, &update_ctx4); + mock_object_map.aio_update(CEPH_NOSNAP, 1, 4, 1, {}, {}, &update_ctx1); + mock_object_map.aio_update(CEPH_NOSNAP, 1, 3, 1, {}, {}, &update_ctx2); + mock_object_map.aio_update(CEPH_NOSNAP, 2, 3, 1, {}, {}, &update_ctx3); + mock_object_map.aio_update(CEPH_NOSNAP, 0, 2, 1, {}, {}, &update_ctx4); } // updates 2, 3, and 4 are blocked on update 1 diff --git a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc index bfebabdc97ad1..370c25fb5555c 100644 --- a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc +++ b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc @@ -200,18 +200,18 @@ public: librados::snap_t snap_id, uint8_t state, int r) { if (mock_image_ctx.image_ctx->object_map != nullptr) { - auto &expect = EXPECT_CALL(mock_object_map, aio_update(snap_id, 0, 1, state, _, _)); + auto &expect = EXPECT_CALL(mock_object_map, aio_update(snap_id, 0, 1, state, _, _, _)); if (r < 0) { - expect.WillOnce(DoAll(WithArg<5>(Invoke([this, r](Context *ctx) { + expect.WillOnce(DoAll(WithArg<6>(Invoke([this, r](Context *ctx) { m_threads->work_queue->queue(ctx, r); })), Return(true))); } else { - expect.WillOnce(DoAll(WithArg<5>(Invoke([&mock_image_ctx, snap_id, state, r](Context *ctx) { + expect.WillOnce(DoAll(WithArg<6>(Invoke([&mock_image_ctx, snap_id, state, r](Context *ctx) { assert(mock_image_ctx.image_ctx->snap_lock.is_locked()); assert(mock_image_ctx.image_ctx->object_map_lock.is_wlocked()); mock_image_ctx.image_ctx->object_map->aio_update( - snap_id, 0, 1, state, boost::none, ctx); + snap_id, 0, 1, state, boost::none, {}, ctx); })), Return(true))); } diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc index 2d5f1470e8a2a..92817b7a78850 100644 --- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc @@ -334,7 +334,7 @@ void ObjectCopyRequest::send_update_object_map() { bool sent = m_local_image_ctx->object_map->template aio_update< ObjectCopyRequest, &ObjectCopyRequest::handle_update_object_map>( snap_object_state.first, m_object_number, snap_object_state.second, {}, - this); + {}, this); assert(sent); m_local_image_ctx->snap_lock.put_read(); } -- 2.39.5