From 8d29a4a231666830914903b95599d80da7b97def Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Mon, 15 Dec 2014 17:04:32 -0800 Subject: [PATCH] osd: handle no-op write with snapshot case If we have a transaction that does something to the object but it !exists both before and after, we will continue through the write path. If the snapdir object already exists, and we try to create it again, we will leak a snapdir obc and lock and later crash on an assert when the obc is destroyed: 0> 2014-12-06 01:49:51.750163 7f08d6ade700 -1 osd/osd_types.h: In function 'ObjectContext::~ObjectContext()' thread 7f08d6ade700 time 2014-12-06 01:49:51.605411 osd/osd_types.h: 2944: FAILED assert(rwstate.empty()) Fix is to not recreated the snapdir if it already exists. Fixes: #10262 Signed-off-by: Sage Weil (cherry picked from commit 02fae9fc54c10b5a932102bac43f32199d4cb612) --- src/osd/ReplicatedPG.cc | 6 ++++-- src/test/librados/snapshots.cc | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc index e149abfd2c756..f1911c13f947b 100644 --- a/src/osd/ReplicatedPG.cc +++ b/src/osd/ReplicatedPG.cc @@ -5168,7 +5168,8 @@ void ReplicatedPG::finish_ctx(OpContext *ctx, int log_op_type, bool maintain_ssc } } } else if (ctx->new_snapset.clones.size() && - !ctx->cache_evict) { + !ctx->cache_evict && + (!ctx->snapset_obc || !ctx->snapset_obc->obs.exists)) { // save snapset on _snap hobject_t snapoid(soid.oid, soid.get_key(), CEPH_SNAPDIR, soid.hash, info.pgid.pool(), soid.get_namespace()); @@ -5179,7 +5180,8 @@ void ReplicatedPG::finish_ctx(OpContext *ctx, int log_op_type, bool maintain_ssc eversion_t(), 0, osd_reqid_t(), ctx->mtime)); - ctx->snapset_obc = get_object_context(snapoid, true); + if (!ctx->snapset_obc) + ctx->snapset_obc = get_object_context(snapoid, true); bool got = ctx->snapset_obc->get_write_greedy(ctx->op); assert(got); dout(20) << " got greedy write on snapset_obc " << *ctx->snapset_obc << dendl; diff --git a/src/test/librados/snapshots.cc b/src/test/librados/snapshots.cc index 020af111ddf97..01ab62e970c04 100644 --- a/src/test/librados/snapshots.cc +++ b/src/test/librados/snapshots.cc @@ -145,6 +145,24 @@ TEST_F(LibRadosSnapshotsPP, SnapGetNamePP) { EXPECT_EQ(0, ioctx.snap_remove("snapfoo")); } +TEST_F(LibRadosSnapshotsPP, SnapCreateRemovePP) { + // reproduces http://tracker.ceph.com/issues/10262 + bufferlist bl; + bl.append("foo"); + ASSERT_EQ(0, ioctx.write("foo", bl, bl.length(), 0)); + ASSERT_EQ(0, ioctx.snap_create("snapfoo")); + ASSERT_EQ(0, ioctx.remove("foo")); + ASSERT_EQ(0, ioctx.snap_create("snapbar")); + + librados::ObjectWriteOperation *op = new librados::ObjectWriteOperation(); + op->create(false); + op->remove(); + ASSERT_EQ(0, ioctx.operate("foo", op)); + + EXPECT_EQ(0, ioctx.snap_remove("snapfoo")); + EXPECT_EQ(0, ioctx.snap_remove("snapbar")); +} + TEST_F(LibRadosSnapshotsSelfManaged, Snap) { std::vector my_snaps; my_snaps.push_back(-2); -- 2.39.5