#233: removed operations/completion map from rados_mail. changed api …

…function wait_for_write_opertations_complete. fix build

src/librmb/rados-mail.cpp

@@ -23,7 +23,14 @@ using std::ostringstream;
 
 using librmb::RadosMail;
 
-RadosMail::RadosMail() : object_size(-1), active_op(false), save_date_rados(-1), valid(true), index_ref(false) {}
+RadosMail::RadosMail()
+    : object_size(-1),
+      completion(nullptr),
+      write_operation(nullptr),
+      active_op(0),
+      save_date_rados(-1),
+      valid(true),
+      index_ref(false) {}
 
 RadosMail::~RadosMail() {}
 

src/librmb/rados-mail.h

@@ -41,7 +41,7 @@ class RadosMail {
   void set_oid(const char* _oid) { this->oid = _oid; }
   void set_oid(const string& _oid) { this->oid = _oid; }
   void set_mail_size(const int _size) { object_size = _size; }
-  void set_active_op(bool _active) { this->active_op = _active; }
+  void set_active_op(int num_write_op) { this->active_op = num_write_op; }
   void set_rados_save_date(const time_t& _save_date) { this->save_date_rados = _save_date; }
 
   string* get_oid() { return &this->oid; }
@@ -55,10 +55,16 @@ class RadosMail {
   librados::bufferlist* get_mail_buffer() { return &this->mail_buffer; }
   map<string, ceph::bufferlist>* get_metadata() { return &this->attrset; }
 
+  AioCompletion* get_completion() { return completion; }
+  void set_completion(AioCompletion* completion_) { this->completion = completion_; }
+
+  ObjectWriteOperation* get_write_operation() { return write_operation; }
+  void set_write_operation(ObjectWriteOperation* write_operation_) { this->write_operation = write_operation_; }
+
   /*!
    * @return reference to all write operations related with this object
    */
-  map<AioCompletion*, ObjectWriteOperation*>* get_completion_op_map() { return &completion_op; }
+
   void get_metadata(const std::string& key, char** value) {
     if (attrset.find(key) != attrset.end()) {
       *value = attrset[key].c_str();
@@ -75,7 +81,8 @@ class RadosMail {
   void set_index_ref(bool ref) { this->index_ref = ref; }
   bool is_valid() { return valid; }
   void set_valid(bool valid_) { valid = valid_; }
-  bool has_active_op() { return active_op; }
+  bool has_active_op() { return active_op > 0; }
+  int get_num_active_op() { return active_op; }
   string to_string(const string& padding);
   void add_metadata(const RadosMetadata& metadata) { attrset[metadata.key] = metadata.bl; }
 
@@ -99,12 +106,12 @@ class RadosMail {
 
  private:
   string oid;
-
   uint8_t guid[GUID_128_SIZE] = {};
   int object_size;  // byte
-  map<AioCompletion*, ObjectWriteOperation*> completion_op;
+  AioCompletion* completion;
+  ObjectWriteOperation* write_operation;
 
-  bool active_op;
+  int active_op;
   ceph::bufferlist mail_buffer;
   time_t save_date_rados;
 

src/librmb/rados-storage-impl.cpp

@@ -83,10 +83,12 @@ int RadosStorageImpl::split_buffer_and_exec_op(RadosMail *current_object,
       tmp_buffer.substr_of(*current_object->get_mail_buffer(), offset, length);
       op->write(offset, tmp_buffer);
     }
+    current_object->set_active_op(i + 1);
   }
   ret_val = get_io_ctx().aio_operate(*current_object->get_oid(), completion, op);
+  current_object->set_completion(completion);
+  current_object->set_write_operation(op);
 
-  (*current_object->get_completion_op_map())[completion] = op;
   return ret_val;
 }
 
@@ -207,29 +209,29 @@ void RadosStorageImpl::close_connection() {
     cluster->deinit();
   }
 }
-bool RadosStorageImpl::wait_for_write_operations_complete(
-    std::map<librados::AioCompletion *, librados::ObjectWriteOperation *> *completion_op_map) {
-  bool failed = false;
-  for (std::map<librados::AioCompletion *, librados::ObjectWriteOperation *>::iterator map_it =
-           completion_op_map->begin();
-       map_it != completion_op_map->end(); ++map_it) {
-    switch (wait_method) {
-      case WAIT_FOR_COMPLETE_AND_CB:
-        map_it->first->wait_for_complete_and_cb();
-        break;
-      case WAIT_FOR_SAFE_AND_CB:
-        map_it->first->wait_for_safe_and_cb();
-        break;
-      default:
-        map_it->first->wait_for_complete_and_cb();
-        break;
-    }
+bool RadosStorageImpl::wait_for_write_operations_complete(librados::AioCompletion *completion,
+                                                          librados::ObjectWriteOperation *write_operation) {
+  if (completion == nullptr || write_operation == nullptr) {
+    return true;  // failed!
+  }
 
-    failed = map_it->first->get_return_value() < 0 || failed ? true : false;
-    // clean up
-    map_it->first->release();
-    map_it->second->remove();
-    delete map_it->second;
+  bool failed = false;
+  switch (wait_method) {
+    case WAIT_FOR_COMPLETE_AND_CB:
+      completion->wait_for_complete_and_cb();
+      break;
+    case WAIT_FOR_SAFE_AND_CB:
+      completion->wait_for_safe_and_cb();
+      break;
+    default:
+      completion->wait_for_complete_and_cb();
+      break;
+
+      failed = completion->get_return_value() < 0 || failed ? true : false;
+      // clean up
+      completion->release();
+      write_operation->remove();
+      delete write_operation;
   }
   return failed;
 }
@@ -243,11 +245,13 @@ bool RadosStorageImpl::wait_for_rados_operations(const std::vector<librmb::Rados
        it_cur_obj != object_list.end(); ++it_cur_obj) {
     // if we come from copy mail, there is no operation to wait for.
     if ((*it_cur_obj)->has_active_op()) {
-      bool op_failed = wait_for_write_operations_complete((*it_cur_obj)->get_completion_op_map());
+      bool op_failed =
+          wait_for_write_operations_complete((*it_cur_obj)->get_completion(), (*it_cur_obj)->get_write_operation());
 
       ctx_failed = ctx_failed ? ctx_failed : op_failed;
-      (*it_cur_obj)->get_completion_op_map()->clear();
-      (*it_cur_obj)->set_active_op(false);
+      (*it_cur_obj)->set_active_op(0);
+      (*it_cur_obj)->set_completion(nullptr);
+      (*it_cur_obj)->set_write_operation(nullptr);
     }
   }
   return ctx_failed;
@@ -369,11 +373,10 @@ bool RadosStorageImpl::save_mail(librados::ObjectWriteOperation *write_op_xattr,
   time_t save_date = mail->get_rados_save_date();
   write_op_xattr->mtime(&save_date);
   int ret = split_buffer_and_exec_op(mail, write_op_xattr, get_max_write_size_bytes());
-  mail->set_active_op(true);
   if (ret != 0) {
     write_op_xattr->remove();
     delete write_op_xattr;
-    mail->set_active_op(false);
+    mail->set_active_op(0);
   } else if (!save_async) {
     std::vector<librmb::RadosMail *> objects;
     objects.push_back(mail);

src/librmb/rados-storage-impl.h

@@ -54,8 +54,8 @@ class RadosStorageImpl : public RadosStorage {
   int open_connection(const std::string &poolname, const std::string &clustername,
                       const std::string &rados_username) override;
   void close_connection() override;
-  bool wait_for_write_operations_complete(
-      std::map<librados::AioCompletion *, librados::ObjectWriteOperation *> *completion_op_map) override;
+  bool wait_for_write_operations_complete(librados::AioCompletion *completion,
+                                          librados::ObjectWriteOperation *write_operation) override;
 
   bool wait_for_rados_operations(const std::vector<librmb::RadosMail *> &object_list) override;
 

src/librmb/rados-storage.h

@@ -124,8 +124,8 @@ class RadosStorage {
    * @param[in] completion_op_map map of write operations with matching completion objects.
    * @return false if successful !!!!
    *  */
-  virtual bool wait_for_write_operations_complete(
-      std::map<librados::AioCompletion *, librados::ObjectWriteOperation *> *completion_op_map) = 0;
+  virtual bool wait_for_write_operations_complete(librados::AioCompletion *completion,
+                                                  librados::ObjectWriteOperation *write_operation) = 0;
   /*!
    * wait for all rados operations
    *

src/storage-rbox/rbox-mail.cpp

@@ -139,13 +139,6 @@ static int rbox_mail_metadata_get(struct rbox_mail *rmail, enum rbox_metadata_ke
     return -1;
   }
   rmail->rados_mail->get_metadata(key, value_r);
-
-#ifdef DEBUG
-  else {
-    // this may not be a problem, because we only save metadata as omap value if it really has a value
-    i_warning("no value for metadata '%c' found ", static_cast<char>(key));
-  }
-#endif
   FUNC_END();
   return 0;
 }

src/tests/librmb/it_test_librmb.cpp

@@ -54,7 +54,7 @@ TEST(librmb, split_write_operation) {
   int ret_storage = storage.split_buffer_and_exec_op(&obj, op, max_size);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   // stat the object
   uint64_t size;
@@ -71,7 +71,7 @@ TEST(librmb, split_write_operation) {
   EXPECT_EQ(0, ret_storage);
   EXPECT_EQ(0, ret_stat);
   EXPECT_EQ(0, ret_remove);
-  EXPECT_EQ(5, (int)obj.get_completion_op_map()->size());
+  EXPECT_NE(5, (int)obj.get_num_active_op());
 }
 /**
  * Test object split operation
@@ -101,7 +101,7 @@ TEST(librmb1, split_write_operation_1) {
   int ret_storage = storage.split_buffer_and_exec_op(&obj, op, max_size);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   // stat the object
   uint64_t size;
@@ -118,7 +118,7 @@ TEST(librmb1, split_write_operation_1) {
   EXPECT_EQ(0, ret_storage);
   EXPECT_EQ(0, ret_stat);
   EXPECT_EQ(0, ret_remove);
-  EXPECT_EQ(1, (int)obj.get_completion_op_map()->size());
+  EXPECT_EQ(1, (int)obj.get_num_active_op());
 }
 /**
  * Test Rados Metadata type conversion
@@ -179,7 +179,7 @@ TEST(librmb1, read_mail) {
   int ret_storage = storage.split_buffer_and_exec_op(&obj, op, max_size);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   // stat the object
   uint64_t size;
@@ -243,7 +243,7 @@ TEST(librmb, load_metadata) {
   int ret_storage = storage.split_buffer_and_exec_op(&obj, op, max_size);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   ms.load_metadata(&obj);
   std::cout << "load metadata ok" << std::endl;
@@ -255,7 +255,7 @@ TEST(librmb, load_metadata) {
   EXPECT_EQ(buffer_length, size);
   EXPECT_EQ(0, ret_storage);
   EXPECT_EQ(0, ret_stat);
-  EXPECT_EQ(5, (int)obj.get_completion_op_map()->size());
+  EXPECT_EQ(5, (int)obj.get_num_active_op());
   EXPECT_EQ(2, (int)obj.get_metadata()->size());
   std::cout << " load with null" << std::endl;
   int i = ms.load_metadata(nullptr);
@@ -305,7 +305,7 @@ TEST(librmb, AttributeVersions) {
   int ret_storage = storage.split_buffer_and_exec_op(&obj, op, max_size);
   EXPECT_EQ(ret_storage, 0);
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   // stat the object
   uint64_t size;
@@ -381,7 +381,7 @@ TEST(librmb, json_ima) {
   EXPECT_EQ(ret_storage, 0);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   // check
   std::map<std::string, ceph::bufferlist> attr_list;
@@ -450,7 +450,7 @@ TEST(librmb, json_ima_2) {
   EXPECT_EQ(ret_storage, 0);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   // check there should be ima and F (Flags)
   std::map<std::string, ceph::bufferlist> attr_list;
@@ -526,7 +526,7 @@ TEST(librmb, json_ima_3) {
   EXPECT_EQ(ret_storage, 0);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   // check there should be ima and F (Flags)
   std::map<std::string, ceph::bufferlist> attr_list;
@@ -607,7 +607,7 @@ TEST(librmb, test_default_metadata_load_attributes) {
   EXPECT_EQ(ret_storage, 0);
 
   // wait for op to finish.
-  storage.wait_for_write_operations_complete(obj.get_completion_op_map());
+  storage.wait_for_write_operations_complete(obj.get_completion(), obj.get_write_operation());
 
   librmb::RadosMail obj2;
   obj2.set_oid("test_ima");

src/tests/mocks/mock_test.h

@@ -24,11 +24,11 @@
 #include "gmock/gmock.h"
 
 namespace librmbtest {
-using librmb::RadosStorage;
 using librmb::RadosMail;
 using librmb::RadosMetadata;
-using librmb::RadosStorageMetadataModule;
 using librmb::RadosMetadataStorage;
+using librmb::RadosStorage;
+using librmb::RadosStorageMetadataModule;
 
 class RadosStorageMock : public RadosStorage {
  public:
@@ -54,8 +54,8 @@ class RadosStorageMock : public RadosStorage {
   MOCK_METHOD3(open_connection,
                int(const std::string &poolname, const std::string &clustername, const std::string &rados_username));
   MOCK_METHOD0(close_connection, void());
-  MOCK_METHOD1(wait_for_write_operations_complete,
-               bool(std::map<librados::AioCompletion *, librados::ObjectWriteOperation *> *completion_op_map));
+  MOCK_METHOD2(wait_for_write_operations_complete,
+               bool(librados::AioCompletion *completion, librados::ObjectWriteOperation *write_operation));
   MOCK_METHOD1(wait_for_rados_operations, bool(const std::vector<librmb::RadosMail *> &object_list));
   MOCK_METHOD1(set_ceph_wait_method, void(enum librmb::rbox_ceph_aio_wait_method wait_method));
   MOCK_METHOD2(read_mail, int(const std::string &oid, librados::bufferlist *buffer));