Skip to content

Commit

Permalink
rbd-mirror: use pool id + global image id as deletion primary key
Browse files Browse the repository at this point in the history 
Fixes: http://tracker.ceph.com/issues/16538
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 25203a8)
  • Loading branch information
@ldachary
Jason Dillaman authored and ldachary committed Aug 19, 2016
1 parent b98e27c commit 9a97e1b
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 44 deletions.
51 changes: 34 additions & 17 deletions src/test/rbd_mirror/test_ImageDeleter.cc
View file
Expand Up @@ -216,7 +216,8 @@ TEST_F(TestImageDeleter, Delete_NonPrimary_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(0, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -232,7 +233,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-rbd::mirror::ImageDeleter::EISPRM, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -247,7 +249,8 @@ TEST_F(TestImageDeleter, Fail_Delete_Diff_GlobalId) {
m_image_name, "diff global id");

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, "diff global id",
&ctx);
EXPECT_EQ(-EINVAL, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -261,7 +264,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Child) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(0, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -276,7 +280,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Children) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(0, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -290,7 +295,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(0, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -305,7 +311,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(0, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -319,7 +326,8 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

ASSERT_EQ(1u, m_deleter->get_delete_queue_items().size());
Expand All @@ -329,11 +337,13 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) {
"clone1", GLOBAL_CLONE_IMAGE_ID);

C_SaferCond ctx2;
m_deleter->wait_for_scheduled_deletion("clone1", &ctx2);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_CLONE_IMAGE_ID,
&ctx2);
EXPECT_EQ(0, ctx2.wait());

C_SaferCond ctx3;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx3);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx3);
EXPECT_EQ(0, ctx3.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -352,7 +362,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(0, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -376,7 +387,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(0, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -392,7 +404,8 @@ TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-ENOENT, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -410,7 +423,8 @@ TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -430,13 +444,15 @@ TEST_F(TestImageDeleter, Retry_Failed_Deletes) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

EXPECT_EQ(0, ictx->state->close());

C_SaferCond ctx2;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx2);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx2);
EXPECT_EQ(0, ctx2.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand All @@ -454,7 +470,8 @@ TEST_F(TestImageDeleter, Delete_Is_Idempotent) {
m_image_name, GLOBAL_IMAGE_ID);

C_SaferCond ctx;
m_deleter->wait_for_scheduled_deletion(m_image_name, &ctx);
m_deleter->wait_for_scheduled_deletion(m_local_pool_id, GLOBAL_IMAGE_ID,
&ctx);
EXPECT_EQ(-EBUSY, ctx.wait());

ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size());
Expand Down
6 changes: 3 additions & 3 deletions src/test/rbd_mirror/test_ImageReplayer.cc
View file
Expand Up @@ -567,7 +567,7 @@ TEST_F(TestImageReplayer, Resync)

C_SaferCond delete_ctx;
m_image_deleter->wait_for_scheduled_deletion(
m_replayer->get_local_image_name(), &delete_ctx);
m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx);
EXPECT_EQ(0, delete_ctx.wait());

C_SaferCond cond;
Expand Down Expand Up @@ -632,7 +632,7 @@ TEST_F(TestImageReplayer, Resync_While_Stop)

C_SaferCond delete_ctx;
m_image_deleter->wait_for_scheduled_deletion(
m_replayer->get_local_image_name(), &delete_ctx);
m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx);
EXPECT_EQ(0, delete_ctx.wait());

C_SaferCond cond3;
Expand Down Expand Up @@ -673,7 +673,7 @@ TEST_F(TestImageReplayer, Resync_StartInterrupted)

C_SaferCond delete_ctx;
m_image_deleter->wait_for_scheduled_deletion(
m_replayer->get_local_image_name(), &delete_ctx);
m_local_ioctx.get_id(), m_replayer->get_global_image_id(), &delete_ctx);
EXPECT_EQ(0, delete_ctx.wait());

C_SaferCond cond2;
Expand Down
31 changes: 17 additions & 14 deletions src/tools/rbd_mirror/ImageDeleter.cc
View file
Expand Up @@ -192,18 +192,18 @@ void ImageDeleter::run() {
}

void ImageDeleter::schedule_image_delete(RadosRef local_rados,
uint64_t local_pool_id,
int64_t local_pool_id,
const std::string& local_image_id,
const std::string& local_image_name,
const std::string& global_image_id) {
dout(20) << "enter" << dendl;

Mutex::Locker l(m_delete_lock);
Mutex::Locker locker(m_delete_lock);

auto del_info = find_delete_info(local_image_name);
auto del_info = find_delete_info(local_pool_id, global_image_id);
if (del_info != nullptr) {
dout(20) << "image " << local_image_name << " was already scheduled for "
<< "deletion" << dendl;
dout(20) << "image " << local_image_name << " (" << global_image_id << ") "
<< "was already scheduled for deletion" << dendl;
return;
}

Expand All @@ -213,16 +213,17 @@ void ImageDeleter::schedule_image_delete(RadosRef local_rados,
m_delete_queue_cond.Signal();
}

void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name,
void ImageDeleter::wait_for_scheduled_deletion(int64_t local_pool_id,
const std::string &global_image_id,
Context *ctx,
bool notify_on_failed_retry) {

ctx = new FunctionContext([this, ctx](int r) {
m_work_queue->queue(ctx, r);
});

Mutex::Locker l(m_delete_lock);
auto del_info = find_delete_info(image_name);
Mutex::Locker locker(m_delete_lock);
auto del_info = find_delete_info(local_pool_id, global_image_id);
if (!del_info) {
// image not scheduled for deletion
ctx->complete(0);
Expand All @@ -236,9 +237,10 @@ void ImageDeleter::wait_for_scheduled_deletion(const std::string& image_name,
(*del_info)->notify_on_failed_retry = notify_on_failed_retry;
}

void ImageDeleter::cancel_waiter(const std::string& image_name) {
void ImageDeleter::cancel_waiter(int64_t local_pool_id,
const std::string &global_image_id) {
Mutex::Locker locker(m_delete_lock);
auto del_info = find_delete_info(image_name);
auto del_info = find_delete_info(local_pool_id, global_image_id);
if (!del_info) {
return;
}
Expand Down Expand Up @@ -506,21 +508,22 @@ void ImageDeleter::retry_failed_deletions() {
}

unique_ptr<ImageDeleter::DeleteInfo> const* ImageDeleter::find_delete_info(
const std::string& image_name) {
int64_t local_pool_id, const std::string &global_image_id) {
assert(m_delete_lock.is_locked());

if (m_active_delete && m_active_delete->match(image_name)) {
if (m_active_delete && m_active_delete->match(local_pool_id,
global_image_id)) {
return &m_active_delete;
}

for (const auto& del_info : m_delete_queue) {
if (del_info->match(image_name)) {
if (del_info->match(local_pool_id, global_image_id)) {
return &del_info;
}
}

for (const auto& del_info : m_failed_queue) {
if (del_info->match(image_name)) {
if (del_info->match(local_pool_id, global_image_id)) {
return &del_info;
}
}
Expand Down
21 changes: 13 additions & 8 deletions src/tools/rbd_mirror/ImageDeleter.h
View file
Expand Up @@ -44,14 +44,16 @@ class ImageDeleter {
ImageDeleter& operator=(const ImageDeleter&) = delete;

void schedule_image_delete(RadosRef local_rados,
uint64_t local_pool_id,
int64_t local_pool_id,
const std::string& local_image_id,
const std::string& local_image_name,
const std::string& global_image_id);
void wait_for_scheduled_deletion(const std::string& image_name,
void wait_for_scheduled_deletion(int64_t local_pool_id,
const std::string &global_image_id,
Context *ctx,
bool notify_on_failed_retry=true);
void cancel_waiter(const std::string& image_name);
void cancel_waiter(int64_t local_pool_id,
const std::string &global_image_id);

void print_status(Formatter *f, std::stringstream *ss);

Expand All @@ -75,7 +77,7 @@ class ImageDeleter {

struct DeleteInfo {
RadosRef local_rados;
uint64_t local_pool_id;
int64_t local_pool_id;
std::string local_image_id;
std::string local_image_name;
std::string global_image_id;
Expand All @@ -84,7 +86,7 @@ class ImageDeleter {
bool notify_on_failed_retry;
Context *on_delete;

DeleteInfo(RadosRef local_rados, uint64_t local_pool_id,
DeleteInfo(RadosRef local_rados, int64_t local_pool_id,
const std::string& local_image_id,
const std::string& local_image_name,
const std::string& global_image_id) :
Expand All @@ -94,8 +96,9 @@ class ImageDeleter {
notify_on_failed_retry(true), on_delete(nullptr) {
}

bool match(const std::string& image_name) {
return local_image_name == image_name;
bool match(int64_t local_pool_id, const std::string &global_image_id) {
return (this->local_pool_id == local_pool_id &&
this->global_image_id == global_image_id);
}
void notify(int r);
void to_string(std::stringstream& ss);
Expand Down Expand Up @@ -133,7 +136,9 @@ class ImageDeleter {
void enqueue_failed_delete(int error_code);
void retry_failed_deletions();

unique_ptr<DeleteInfo> const* find_delete_info(const std::string& image_name);
unique_ptr<DeleteInfo> const*
find_delete_info(int64_t local_pool_id, const std::string &global_image_id);

};

} // namespace mirror
Expand Down
8 changes: 6 additions & 2 deletions src/tools/rbd_mirror/Replayer.cc
View file
Expand Up @@ -740,7 +740,9 @@ void Replayer::start_image_replayer(unique_ptr<ImageReplayer<> > &image_replayer
}
}
);
m_image_deleter->wait_for_scheduled_deletion(image_name.get(), ctx, false);

m_image_deleter->wait_for_scheduled_deletion(
m_local_pool_id, image_replayer->get_global_image_id(), ctx, false);
}
}

Expand All @@ -752,7 +754,9 @@ bool Replayer::stop_image_replayer(unique_ptr<ImageReplayer<> > &image_replayer)

// TODO: check how long it is stopping and alert if it is too long.
if (image_replayer->is_stopped()) {
m_image_deleter->cancel_waiter(image_replayer->get_local_image_name());
m_image_deleter->cancel_waiter(m_local_pool_id,
image_replayer->get_global_image_id());

if (!m_stopping.read()) {
dout(20) << "scheduling delete" << dendl;
m_image_deleter->schedule_image_delete(
Expand Down

0 comments on commit 9a97e1b

Please sign in to comment.