Skip to content

Commit

Permalink
rbd-mirror: Add sparse read for sync image
Browse files Browse the repository at this point in the history
Currently, the image sync do full read, and we shall add sparse read
to let the sync more efficiently.

Feature: http://tracker.ceph.com/issues/16780
Signed-off-by: tianqing <tianqing@unitedstack.com>
  • Loading branch information
tianqing committed Sep 16, 2016
1 parent b1a2e77 commit c0822c6
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 47 deletions.
11 changes: 11 additions & 0 deletions src/test/librados_test_stub/MockTestMemIoCtxImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,16 @@ class MockTestMemIoCtxImpl : public TestMemIoCtxImpl {
return TestMemIoCtxImpl::notify(o, bl, timeout_ms, pbl);
}

MOCK_METHOD5(sparse_read, int(const std::string& oid,
uint64_t off,
size_t len,
std::map<uint64_t, uint64_t> *m,
bufferlist *bl));
int do_sparse_read(const std::string& oid, uint64_t off, size_t len,
std::map<uint64_t, uint64_t> *m, bufferlist *bl){
return TestMemIoCtxImpl::sparse_read(oid, off, len, m, bl);
}

MOCK_METHOD4(read, int(const std::string& oid,
size_t len,
uint64_t off,
Expand Down Expand Up @@ -142,6 +152,7 @@ class MockTestMemIoCtxImpl : public TestMemIoCtxImpl {
ON_CALL(*this, list_watchers(_, _)).WillByDefault(Invoke(this, &MockTestMemIoCtxImpl::do_list_watchers));
ON_CALL(*this, notify(_, _, _, _)).WillByDefault(Invoke(this, &MockTestMemIoCtxImpl::do_notify));
ON_CALL(*this, read(_, _, _, _)).WillByDefault(Invoke(this, &MockTestMemIoCtxImpl::do_read));
ON_CALL(*this, sparse_read(_, _, _, _, _)).WillByDefault(Invoke(this, &MockTestMemIoCtxImpl::do_sparse_read));
ON_CALL(*this, remove(_, _)).WillByDefault(Invoke(this, &MockTestMemIoCtxImpl::do_remove));
ON_CALL(*this, selfmanaged_snap_create(_)).WillByDefault(Invoke(this, &MockTestMemIoCtxImpl::do_selfmanaged_snap_create));
ON_CALL(*this, selfmanaged_snap_remove(_)).WillByDefault(Invoke(this, &MockTestMemIoCtxImpl::do_selfmanaged_snap_remove));
Expand Down
12 changes: 11 additions & 1 deletion src/test/librados_test_stub/TestMemIoCtxImpl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,6 @@ int TestMemIoCtxImpl::sparse_read(const std::string& oid, uint64_t off,
uint64_t len,
std::map<uint64_t,uint64_t> *m,
bufferlist *data_bl) {
// TODO verify correctness
TestMemRadosClient::SharedFile file;
{
RWLock::RLocker l(m_pool->file_lock);
Expand All @@ -373,19 +372,29 @@ int TestMemIoCtxImpl::sparse_read(const std::string& oid, uint64_t off,
}
}

//interval_set<uint64_t>::const_iterator cit = file->snap_overlap.begin();
for(auto cit = file->snap_overlap.begin(); cit != file->snap_overlap.end(); cit++){
std::cerr << " off: " << cit.get_start() << " len: " << cit.get_len() << std::endl;
}

RWLock::RLocker l(file->lock);
len = clip_io(off, len, file->data.length());

if (m != NULL) {
m->clear();
if (len > 0) {
(*m)[off] = len;
}
}
std::cerr << "m is " << m->size() << std::endl;

if (data_bl != NULL && len > 0) {
bufferlist bit;
bit.substr_of(file->data, off, len);
append_clone(bit, data_bl);
}

std::cerr << "data_bl is " << data_bl->length() << std::endl;
return 0;
}

Expand Down Expand Up @@ -468,6 +477,7 @@ int TestMemIoCtxImpl::write(const std::string& oid, bufferlist& bl, size_t len,

ensure_minimum_length(off + len, &file->data);
file->data.copy_in(off, len, bl);

return 0;
}

Expand Down
60 changes: 23 additions & 37 deletions src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "test/rbd_mirror/test_mock_fixture.h"
#include "include/interval_set.h"
#include "include/rbd/librbd.hpp"
#include "include/rbd/object_map_types.h"
#include "librbd/AioImageRequestWQ.h"
#include "librbd/ImageCtx.h"
#include "librbd/ImageState.h"
Expand Down Expand Up @@ -58,6 +59,7 @@ void scribble(librbd::ImageCtx *image_ctx, int num_ops, size_t max_size,
ASSERT_EQ(static_cast<int>(len), r);

interval_set<uint64_t> w;
std::cerr << "scribble write offset: " << off << " length: " << len << std::endl;
w.insert(off, len);
what->union_of(w);
}
Expand Down Expand Up @@ -108,29 +110,10 @@ class TestMockImageSyncObjectCopyRequest : public TestMockFixture {
0, on_finish);
}

void expect_read(librados::MockTestMemIoCtxImpl &mock_io_ctx, uint64_t offset,
uint64_t length, int r) {
auto &expect = EXPECT_CALL(mock_io_ctx, read(_, length, offset, _));
if (r < 0) {
expect.WillOnce(Return(r));
} else {
expect.WillOnce(DoDefault());
}
}

void expect_read(librados::MockTestMemIoCtxImpl &mock_io_ctx,
const interval_set<uint64_t> &extents, int r) {
for (auto extent : extents) {
expect_read(mock_io_ctx, extent.first, extent.second, r);
if (r < 0) {
break;
}
}
}
void expect_sparse_read(librados::MockTestMemIoCtxImpl &mock_io_ctx, uint64_t offset,
uint64_t length, int r) {

void expect_write(librados::MockTestMemIoCtxImpl &mock_io_ctx,
uint64_t offset, uint64_t length, int r) {
auto &expect = EXPECT_CALL(mock_io_ctx, write(_, _, length, offset, _));
auto &expect = EXPECT_CALL(mock_io_ctx, sparse_read(_, offset, length, _, _));
if (r < 0) {
expect.WillOnce(Return(r));
} else {
Expand All @@ -139,13 +122,15 @@ class TestMockImageSyncObjectCopyRequest : public TestMockFixture {
}

void expect_write(librados::MockTestMemIoCtxImpl &mock_io_ctx,
const interval_set<uint64_t> &extents, int r) {
for (auto extent : extents) {
expect_write(mock_io_ctx, extent.first, extent.second, r);
if (r < 0) {
break;
}
}
const interval_set<uint64_t> w, int r){
for(auto it : w){
auto &expect = EXPECT_CALL(mock_io_ctx, write(_, _, it.first, it.second, _));
if (r < 0){
expect.WillOnce(Return(r));
} else {
expect.WillOnce(DoDefault());
}
}
}

void expect_truncate(librados::MockTestMemIoCtxImpl &mock_io_ctx,
Expand All @@ -171,6 +156,7 @@ class TestMockImageSyncObjectCopyRequest : public TestMockFixture {
librbd::MockObjectMap &mock_object_map,
librados::snap_t snap_id, uint8_t state,
int r) {
std::cerr << ">> the state is " << (int) state << std::endl;
if (mock_image_ctx.image_ctx->object_map != nullptr) {
auto &expect = EXPECT_CALL(mock_object_map, aio_update(snap_id, 0, 1, state, _, _));
if (r < 0) {
Expand Down Expand Up @@ -348,8 +334,8 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Write) {

InSequence seq;
expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
expect_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_write(mock_local_io_ctx, 0, one.range_end(), 0);
expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_write(mock_local_io_ctx, one, 0);
expect_update_object_map(mock_local_image_ctx, mock_object_map,
m_local_snap_ids[0], OBJECT_EXISTS, 0);

Expand Down Expand Up @@ -381,7 +367,7 @@ TEST_F(TestMockImageSyncObjectCopyRequest, ReadError) {

InSequence seq;
expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
expect_read(mock_remote_io_ctx, 0, one.range_end(), -EINVAL);
expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), -EINVAL);

request->send();
ASSERT_EQ(-EINVAL, ctx.wait());
Expand Down Expand Up @@ -412,7 +398,7 @@ TEST_F(TestMockImageSyncObjectCopyRequest, WriteError) {

InSequence seq;
expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
expect_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_write(mock_local_io_ctx, 0, one.range_end(), -EINVAL);

request->send();
Expand Down Expand Up @@ -455,9 +441,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, WriteSnaps) {

InSequence seq;
expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
expect_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_write(mock_local_io_ctx, 0, one.range_end(), 0);
expect_read(mock_remote_io_ctx, two, 0);
expect_sparse_read(mock_remote_io_ctx, 0, two.range_end(), 0);
expect_write(mock_local_io_ctx, two, 0);
expect_update_object_map(mock_local_image_ctx, mock_object_map,
m_local_snap_ids[0], OBJECT_EXISTS, 0);
Expand Down Expand Up @@ -504,7 +490,7 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Trim) {

InSequence seq;
expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
expect_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_write(mock_local_io_ctx, 0, one.range_end(), 0);
expect_truncate(mock_local_io_ctx, trim_offset, 0);
expect_update_object_map(mock_local_image_ctx, mock_object_map,
Expand Down Expand Up @@ -546,7 +532,7 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Remove) {

InSequence seq;
expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
expect_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
expect_write(mock_local_io_ctx, 0, one.range_end(), 0);
expect_remove(mock_local_io_ctx, 0);
expect_update_object_map(mock_local_image_ctx, mock_object_map,
Expand Down
37 changes: 29 additions & 8 deletions src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ void ObjectCopyRequest<I>::send_read_object() {
auto &sync_ops = m_snap_sync_ops.begin()->second;
assert(!sync_ops.empty());

std::cerr << "++++++ sync_ops len " << sync_ops.size() << std::endl;

// map the sync op start snap id back to the necessary read snap id
librados::snap_t remote_snap_seq = m_snap_sync_ops.begin()->first;
m_remote_io_ctx.snap_set_read(remote_snap_seq);
Expand All @@ -110,11 +112,10 @@ void ObjectCopyRequest<I>::send_read_object() {
dout(20) << ": remote_snap_seq=" << remote_snap_seq << dendl;
read_required = true;
}

dout(20) << ": read op: " << std::get<1>(sync_op) << "~"
<< std::get<2>(sync_op) << dendl;
op.read(std::get<1>(sync_op), std::get<2>(sync_op),
&std::get<3>(sync_op), nullptr);
op.sparse_read(std::get<1>(sync_op), std::get<2>(sync_op), &std::get<4>(sync_op),
&std::get<3>(sync_op), nullptr);
break;
default:
break;
Expand Down Expand Up @@ -175,12 +176,21 @@ void ObjectCopyRequest<I>::send_write_object() {
assert(!sync_ops.empty());

librados::ObjectWriteOperation op;
std::map<uint64_t, uint64_t>::const_iterator eit;
uint64_t len = 0;
uint64_t begin = 0;
for (auto &sync_op : sync_ops) {
switch (std::get<0>(sync_op)) {
case SYNC_OP_TYPE_WRITE:
for( auto it : std::get<4>(sync_op)){
bufferlist tmpbl;
len = it.second - it.first;
tmpbl.substr_of(std::get<3>(sync_op), begin, len);
op.write(it.first, tmpbl);
begin += len;
}
dout(20) << ": write op: " << std::get<1>(sync_op) << "~"
<< std::get<3>(sync_op).length() << dendl;
op.write(std::get<1>(sync_op), std::get<3>(sync_op));
<< std::get<4>(sync_op) << dendl;
break;
case SYNC_OP_TYPE_TRUNC:
dout(20) << ": trunc op: " << std::get<1>(sync_op) << dendl;
Expand All @@ -206,6 +216,7 @@ void ObjectCopyRequest<I>::send_write_object() {
template <typename I>
void ObjectCopyRequest<I>::handle_write_object(int r) {
dout(20) << ": r=" << r << dendl;
std::cerr << "+++ in handle_write_object" << std::endl;

if (r == -ENOENT) {
r = 0;
Expand All @@ -223,11 +234,15 @@ void ObjectCopyRequest<I>::handle_write_object(int r) {
return;
}

std::cerr << "will call send_update_object_map" << std::endl;
send_update_object_map();
}

template <typename I>
void ObjectCopyRequest<I>::send_update_object_map() {

std::cerr << " in " << __func__ << std::endl;

m_local_image_ctx->snap_lock.get_read();
if (!m_local_image_ctx->test_features(RBD_FEATURE_OBJECT_MAP,
m_local_image_ctx->snap_lock) ||
Expand Down Expand Up @@ -258,6 +273,9 @@ void ObjectCopyRequest<I>::send_update_object_map() {
Context *ctx = create_context_callback<
ObjectCopyRequest<I>, &ObjectCopyRequest<I>::handle_update_object_map>(
this);


std::cerr << "the stat is " << (int) snap_object_state.second << std::endl;
m_local_image_ctx->object_map->aio_update(snap_object_state.first,
m_object_number,
m_object_number + 1,
Expand Down Expand Up @@ -333,20 +351,23 @@ void ObjectCopyRequest<I>::compute_diffs() {
m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_WRITE,
it.get_start(),
it.get_len(),
bufferlist());
bufferlist(),
std::map<uint64_t, uint64_t>());
}
if (end_size < prev_end_size) {
dout(20) << ": trunc op: " << end_size << dendl;
m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_TRUNC,
end_size, 0U,
bufferlist());
bufferlist(),
std::map<uint64_t, uint64_t>());
}
} else {
if (prev_exists) {
// object remove
dout(20) << ": remove op" << dendl;
m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_REMOVE,
0U, 0U, bufferlist());
0U, 0U, bufferlist(),
std::map<uint64_t, uint64_t>());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class ObjectCopyRequest {
SYNC_OP_TYPE_REMOVE
};

typedef std::tuple<SyncOpType, uint64_t, uint64_t, bufferlist> SyncOp;
typedef std::tuple<SyncOpType, uint64_t, uint64_t, bufferlist, std::map<uint64_t, uint64_t> > SyncOp;
typedef std::list<SyncOp> SyncOps;
typedef std::map<librados::snap_t, SyncOps> SnapSyncOps;
typedef std::map<librados::snap_t, uint8_t> SnapObjectStates;
Expand Down

0 comments on commit c0822c6

Please sign in to comment.