journal: Future does not require metadata shared pointer

Avoid keeping the metadata object alive through the lifespan of the
future.  Callers can expect to receive a re-entrant callback if the
future is already safe and flush/wait is invoked.

Fixes: http://tracker.ceph.com/issues/15364
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/journal/FutureImpl.cc b/src/journal/FutureImpl.cc
index 11eda4456616c2f5f506b49b6dfefdec29cc2dce..aebfe12e310024c9343a8aeb09d17cab58790830 100644 (file)
--- a/src/journal/FutureImpl.cc
+++ b/src/journal/FutureImpl.cc
@@ -2,15 +2,14 @@
 // vim: ts=8 sw=2 smarttab
 
 #include "journal/FutureImpl.h"
-#include "journal/JournalMetadata.h"
 #include "journal/Utils.h"
 
 namespace journal {
 
-FutureImpl::FutureImpl(JournalMetadataPtr journal_metadata, uint64_t tag_tid,
-                       uint64_t entry_tid, uint64_t commit_tid)
-  : RefCountedObject(NULL, 0), m_journal_metadata(journal_metadata),
-    m_tag_tid(tag_tid), m_entry_tid(entry_tid), m_commit_tid(commit_tid),
+FutureImpl::FutureImpl(uint64_t tag_tid, uint64_t entry_tid,
+                       uint64_t commit_tid)
+  : RefCountedObject(NULL, 0), m_tag_tid(tag_tid), m_entry_tid(entry_tid),
+    m_commit_tid(commit_tid),
     m_lock(utils::unique_lock_name("FutureImpl::m_lock", this)), m_safe(false),
     m_consistent(false), m_return_value(0), m_flush_state(FLUSH_STATE_NONE),
     m_consistent_ack(this) {
@@ -51,7 +50,7 @@ void FutureImpl::flush(Context *on_safe) {
   }
 
   if (complete && on_safe != NULL) {
-    m_journal_metadata->queue(on_safe, m_return_value);
+    on_safe->complete(m_return_value);
   } else if (flush_handler) {
     // attached to journal object -- instruct it to flush all entries through
     // this one.  possible to become detached while lock is released, so flush
@@ -69,7 +68,8 @@ void FutureImpl::wait(Context *on_safe) {
       return;
     }
   }
-  m_journal_metadata->queue(on_safe, m_return_value);
+
+  on_safe->complete(m_return_value);
 }
 
 bool FutureImpl::is_complete() const {

diff --git a/src/journal/FutureImpl.h b/src/journal/FutureImpl.h
index 0a9eba58d56d1debbfb83c8cd61a827b821c5927..5c11c4bb105004a87274554d4a3f85ab14fa77ea 100644 (file)
--- a/src/journal/FutureImpl.h
+++ b/src/journal/FutureImpl.h
@@ -18,7 +18,6 @@ class Context;
 namespace journal {
 
 class FutureImpl;
-class JournalMetadata;
 typedef boost::intrusive_ptr<FutureImpl> FutureImplPtr;
 
 class FutureImpl : public RefCountedObject, boost::noncopyable {
@@ -29,11 +28,9 @@ public:
     virtual void get() = 0;
     virtual void put() = 0;
   };
-  typedef boost::intrusive_ptr<JournalMetadata> JournalMetadataPtr;
   typedef boost::intrusive_ptr<FlushHandler> FlushHandlerPtr;
 
-  FutureImpl(JournalMetadataPtr journal_metadata, uint64_t tag_tid,
-             uint64_t entry_tid, uint64_t commit_tid);
+  FutureImpl(uint64_t tag_tid, uint64_t entry_tid, uint64_t commit_tid);
 
   void init(const FutureImplPtr &prev_future);
 
@@ -96,7 +93,6 @@ private:
     virtual void finish(int r) {}
   };
 
-  JournalMetadataPtr m_journal_metadata;
   uint64_t m_tag_tid;
   uint64_t m_entry_tid;
   uint64_t m_commit_tid;

diff --git a/src/journal/JournalRecorder.cc b/src/journal/JournalRecorder.cc
index 065f692821771a7122775e16d437dccfc194d008..b730b267a5ccd994550fa64d387baec569e8a300 100644 (file)
--- a/src/journal/JournalRecorder.cc
+++ b/src/journal/JournalRecorder.cc
@@ -80,8 +80,7 @@ Future JournalRecorder::append(uint64_t tag_tid,
   ObjectRecorderPtr object_ptr = get_object(splay_offset);
   uint64_t commit_tid = m_journal_metadata->allocate_commit_tid(
     object_ptr->get_object_number(), tag_tid, entry_tid);
-  FutureImplPtr future(new FutureImpl(m_journal_metadata, tag_tid, entry_tid,
-                                      commit_tid));
+  FutureImplPtr future(new FutureImpl(tag_tid, entry_tid, commit_tid));
   future->init(m_prev_future);
   m_prev_future = future;
 

diff --git a/src/test/journal/test_FutureImpl.cc b/src/test/journal/test_FutureImpl.cc
index 51e19cf35e913207ec60fab88631cc325b404bde..eb5f80696bd62d45bf5380e9cb364d8f43c4cc20 100644 (file)
--- a/src/test/journal/test_FutureImpl.cc
+++ b/src/test/journal/test_FutureImpl.cc
@@ -25,12 +25,11 @@ public:
     }
   };
 
-  journal::FutureImplPtr create_future(journal::JournalMetadataPtr metadata,
-                                       uint64_t tag_tid, uint64_t entry_tid,
+  journal::FutureImplPtr create_future(uint64_t tag_tid, uint64_t entry_tid,
                                        uint64_t commit_tid,
                                        const journal::FutureImplPtr &prev =
                                          journal::FutureImplPtr()) {
-    journal::FutureImplPtr future(new journal::FutureImpl(metadata, tag_tid,
+    journal::FutureImplPtr future(new journal::FutureImpl(tag_tid,
                                                           entry_tid,
                                                           commit_tid));
     future->init(prev);
@@ -50,7 +49,7 @@ TEST_F(TestFutureImpl, Getters) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   ASSERT_EQ(234U, future->get_tag_tid());
   ASSERT_EQ(123U, future->get_entry_tid());
   ASSERT_EQ(456U, future->get_commit_tid());
@@ -63,7 +62,7 @@ TEST_F(TestFutureImpl, Attach) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   ASSERT_FALSE(future->attach(&m_flush_handler));
   ASSERT_EQ(1U, m_flush_handler.refs);
 }
@@ -75,7 +74,7 @@ TEST_F(TestFutureImpl, AttachWithPendingFlush) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   future->flush(NULL);
 
   ASSERT_TRUE(future->attach(&m_flush_handler));
@@ -89,7 +88,7 @@ TEST_F(TestFutureImpl, Detach) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   ASSERT_FALSE(future->attach(&m_flush_handler));
   future->detach();
   ASSERT_EQ(0U, m_flush_handler.refs);
@@ -102,7 +101,7 @@ TEST_F(TestFutureImpl, DetachImplicit) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   ASSERT_FALSE(future->attach(&m_flush_handler));
   future.reset();
   ASSERT_EQ(0U, m_flush_handler.refs);
@@ -115,7 +114,7 @@ TEST_F(TestFutureImpl, Flush) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   ASSERT_FALSE(future->attach(&m_flush_handler));
 
   C_SaferCond cond;
@@ -133,7 +132,7 @@ TEST_F(TestFutureImpl, FlushWithoutContext) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   ASSERT_FALSE(future->attach(&m_flush_handler));
 
   future->flush(NULL);
@@ -150,10 +149,10 @@ TEST_F(TestFutureImpl, FlushChain) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future1 = create_future(metadata, 234, 123, 456);
-  journal::FutureImplPtr future2 = create_future(metadata, 234, 124, 457,
+  journal::FutureImplPtr future1 = create_future(234, 123, 456);
+  journal::FutureImplPtr future2 = create_future(234, 124, 457,
                                                  future1);
-  journal::FutureImplPtr future3 = create_future(metadata, 235, 1, 458,
+  journal::FutureImplPtr future3 = create_future(235, 1, 458,
                                                  future2);
   ASSERT_FALSE(future1->attach(&m_flush_handler));
   ASSERT_FALSE(future2->attach(&m_flush_handler));
@@ -184,8 +183,8 @@ TEST_F(TestFutureImpl, FlushInProgress) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future1 = create_future(metadata, 234, 123, 456);
-  journal::FutureImplPtr future2 = create_future(metadata, 234, 124, 457,
+  journal::FutureImplPtr future1 = create_future(234, 123, 456);
+  journal::FutureImplPtr future2 = create_future(234, 124, 457,
                                                  future1);
   ASSERT_FALSE(future1->attach(&m_flush_handler));
   ASSERT_FALSE(future2->attach(&m_flush_handler));
@@ -206,7 +205,7 @@ TEST_F(TestFutureImpl, FlushAlreadyComplete) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 123, 456);
+  journal::FutureImplPtr future = create_future(234, 123, 456);
   future->safe(-EIO);
 
   C_SaferCond cond;
@@ -221,7 +220,7 @@ TEST_F(TestFutureImpl, Wait) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 1, 456);
+  journal::FutureImplPtr future = create_future(234, 1, 456);
 
   C_SaferCond cond;
   future->wait(&cond);
@@ -236,7 +235,7 @@ TEST_F(TestFutureImpl, WaitAlreadyComplete) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future = create_future(metadata, 234, 1, 456);
+  journal::FutureImplPtr future = create_future(234, 1, 456);
   future->safe(-EEXIST);
 
   C_SaferCond cond;
@@ -251,8 +250,8 @@ TEST_F(TestFutureImpl, SafePreservesError) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future1 = create_future(metadata, 234, 123, 456);
-  journal::FutureImplPtr future2 = create_future(metadata, 234, 124, 457,
+  journal::FutureImplPtr future1 = create_future(234, 123, 456);
+  journal::FutureImplPtr future2 = create_future(234, 124, 457,
                                                  future1);
 
   future1->safe(-EIO);
@@ -268,8 +267,8 @@ TEST_F(TestFutureImpl, ConsistentPreservesError) {
   journal::JournalMetadataPtr metadata = create_metadata(oid);
   ASSERT_EQ(0, init_metadata(metadata));
 
-  journal::FutureImplPtr future1 = create_future(metadata, 234, 123, 456);
-  journal::FutureImplPtr future2 = create_future(metadata, 234, 124, 457,
+  journal::FutureImplPtr future1 = create_future(234, 123, 456);
+  journal::FutureImplPtr future2 = create_future(234, 124, 457,
                                                  future1);
 
   future2->safe(-EEXIST);

diff --git a/src/test/journal/test_ObjectRecorder.cc b/src/test/journal/test_ObjectRecorder.cc
index 65d74b6e58afb8594c08844bbf91f3920ee5b4f5..f26e5261688b70c9200af57742a1e9aeae778c5c 100644 (file)
--- a/src/test/journal/test_ObjectRecorder.cc
+++ b/src/test/journal/test_ObjectRecorder.cc
@@ -67,11 +67,10 @@ public:
     m_flush_age = i;
   }
 
-  journal::AppendBuffer create_append_buffer(journal::JournalMetadataPtr metadata,
-                                             uint64_t tag_tid, uint64_t entry_tid,
+  journal::AppendBuffer create_append_buffer(uint64_t tag_tid, uint64_t entry_tid,
                                              const std::string &payload) {
-    journal::FutureImplPtr future(new journal::FutureImpl(metadata, tag_tid,
-                                                          entry_tid, 456));
+    journal::FutureImplPtr future(new journal::FutureImpl(tag_tid, entry_tid,
+                                                          456));
     future->init(journal::FutureImplPtr());
 
     bufferlist bl;
@@ -98,16 +97,14 @@ TEST_F(TestObjectRecorder, Append) {
 
   journal::ObjectRecorderPtr object = create_object(oid, 24);
 
-  journal::AppendBuffer append_buffer1 = create_append_buffer(metadata,
-                                                              234, 123,
+  journal::AppendBuffer append_buffer1 = create_append_buffer(234, 123,
                                                               "payload");
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer1};
   ASSERT_FALSE(object->append(append_buffers));
   ASSERT_EQ(1U, object->get_pending_appends());
 
-  journal::AppendBuffer append_buffer2 = create_append_buffer(metadata,
-                                                              234, 124,
+  journal::AppendBuffer append_buffer2 = create_append_buffer(234, 124,
                                                               "payload");
   append_buffers = {append_buffer2};
   ASSERT_FALSE(object->append(append_buffers));
@@ -129,16 +126,14 @@ TEST_F(TestObjectRecorder, AppendFlushByCount) {
   set_flush_interval(2);
   journal::ObjectRecorderPtr object = create_object(oid, 24);
 
-  journal::AppendBuffer append_buffer1 = create_append_buffer(metadata,
-                                                              234, 123,
+  journal::AppendBuffer append_buffer1 = create_append_buffer(234, 123,
                                                               "payload");
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer1};
   ASSERT_FALSE(object->append(append_buffers));
   ASSERT_EQ(1U, object->get_pending_appends());
 
-  journal::AppendBuffer append_buffer2 = create_append_buffer(metadata,
-                                                              234, 124,
+  journal::AppendBuffer append_buffer2 = create_append_buffer(234, 124,
                                                               "payload");
   append_buffers = {append_buffer2};
   ASSERT_FALSE(object->append(append_buffers));
@@ -159,16 +154,14 @@ TEST_F(TestObjectRecorder, AppendFlushByBytes) {
   set_flush_bytes(10);
   journal::ObjectRecorderPtr object = create_object(oid, 24);
 
-  journal::AppendBuffer append_buffer1 = create_append_buffer(metadata,
-                                                              234, 123,
+  journal::AppendBuffer append_buffer1 = create_append_buffer(234, 123,
                                                               "payload");
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer1};
   ASSERT_FALSE(object->append(append_buffers));
   ASSERT_EQ(1U, object->get_pending_appends());
 
-  journal::AppendBuffer append_buffer2 = create_append_buffer(metadata,
-                                                              234, 124,
+  journal::AppendBuffer append_buffer2 = create_append_buffer(234, 124,
                                                               "payload");
   append_buffers = {append_buffer2};
   ASSERT_FALSE(object->append(append_buffers));
@@ -189,15 +182,13 @@ TEST_F(TestObjectRecorder, AppendFlushByAge) {
   set_flush_age(0.1);
   journal::ObjectRecorderPtr object = create_object(oid, 24);
 
-  journal::AppendBuffer append_buffer1 = create_append_buffer(metadata,
-                                                              234, 123,
+  journal::AppendBuffer append_buffer1 = create_append_buffer(234, 123,
                                                               "payload");
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer1};
   ASSERT_FALSE(object->append(append_buffers));
 
-  journal::AppendBuffer append_buffer2 = create_append_buffer(metadata,
-                                                              234, 124,
+  journal::AppendBuffer append_buffer2 = create_append_buffer(234, 124,
                                                               "payload");
   append_buffers = {append_buffer2};
   ASSERT_FALSE(object->append(append_buffers));
@@ -218,15 +209,13 @@ TEST_F(TestObjectRecorder, AppendFilledObject) {
   journal::ObjectRecorderPtr object = create_object(oid, 12);
 
   std::string payload(2048, '1');
-  journal::AppendBuffer append_buffer1 = create_append_buffer(metadata,
-                                                              234, 123,
+  journal::AppendBuffer append_buffer1 = create_append_buffer(234, 123,
                                                               payload);
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer1};
   ASSERT_FALSE(object->append(append_buffers));
 
-  journal::AppendBuffer append_buffer2 = create_append_buffer(metadata,
-                                                              234, 124,
+  journal::AppendBuffer append_buffer2 = create_append_buffer(234, 124,
                                                               payload);
   append_buffers = {append_buffer2};
   ASSERT_TRUE(object->append(append_buffers));
@@ -246,8 +235,7 @@ TEST_F(TestObjectRecorder, Flush) {
 
   journal::ObjectRecorderPtr object = create_object(oid, 24);
 
-  journal::AppendBuffer append_buffer1 = create_append_buffer(metadata,
-                                                              234, 123,
+  journal::AppendBuffer append_buffer1 = create_append_buffer(234, 123,
                                                               "payload");
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer1};
@@ -273,8 +261,7 @@ TEST_F(TestObjectRecorder, FlushFuture) {
 
   journal::ObjectRecorderPtr object = create_object(oid, 24);
 
-  journal::AppendBuffer append_buffer = create_append_buffer(metadata,
-                                                             234, 123,
+  journal::AppendBuffer append_buffer = create_append_buffer(234, 123,
                                                              "payload");
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer};
@@ -298,8 +285,7 @@ TEST_F(TestObjectRecorder, FlushDetachedFuture) {
 
   journal::ObjectRecorderPtr object = create_object(oid, 24);
 
-  journal::AppendBuffer append_buffer = create_append_buffer(metadata,
-                                                             234, 123,
+  journal::AppendBuffer append_buffer = create_append_buffer(234, 123,
                                                              "payload");
 
   journal::AppendBuffers append_buffers;
@@ -326,11 +312,9 @@ TEST_F(TestObjectRecorder, Overflow) {
   journal::ObjectRecorderPtr object2 = create_object(oid, 12);
 
   std::string payload(2048, '1');
-  journal::AppendBuffer append_buffer1 = create_append_buffer(metadata,
-                                                              234, 123,
+  journal::AppendBuffer append_buffer1 = create_append_buffer(234, 123,
                                                               payload);
-  journal::AppendBuffer append_buffer2 = create_append_buffer(metadata,
-                                                              234, 124,
+  journal::AppendBuffer append_buffer2 = create_append_buffer(234, 124,
                                                               payload);
   journal::AppendBuffers append_buffers;
   append_buffers = {append_buffer1, append_buffer2};
@@ -341,8 +325,7 @@ TEST_F(TestObjectRecorder, Overflow) {
   ASSERT_EQ(0, cond.wait());
   ASSERT_EQ(0U, object1->get_pending_appends());
 
-  journal::AppendBuffer append_buffer3 = create_append_buffer(metadata,
-                                                              456, 123,
+  journal::AppendBuffer append_buffer3 = create_append_buffer(456, 123,
                                                               payload);
   append_buffers = {append_buffer3};