rgw/d4n: removing calls to redis MULTI, DISCARD, WATCH,

UNWATCH and EXEC since other options for Redis transactions
are being explored.

Signed-off-by: Pritha Srivastava <prsrivas@redhat.com>

diff --git a/src/rgw/driver/d4n/d4n_policy.cc b/src/rgw/driver/d4n/d4n_policy.cc
index 0bce11753d9d9d072866dc6c9a50c3a3c3a6704d..9ab9bf3d6e3c9ad8f3022034ba67b432991a946b 100644 (file)
--- a/src/rgw/driver/d4n/d4n_policy.cc
+++ b/src/rgw/driver/d4n/d4n_policy.cc
@@ -865,11 +865,6 @@ void LFUDAPolicy::cleaning(const DoutPrefixProvider* dpp)
        block.blockID = 0;
        //non-versioned case
        if (!c_obj->have_instance()) {
-         //add watch on latest entry, as it can be modified by a put or a del
-         ret = blockDir->watch(dpp, &block, y);
-         if (ret < 0) {
-           ldpp_dout(dpp, 0) << __func__ << "(): Failed to add a watch on: " << block.cacheObj.objName << ", ret=" << ret << dendl;
-         }
          // hash entry for latest version
          op_ret = blockDir->get(dpp, &block, y);
          if (op_ret < 0) {
@@ -888,20 +883,10 @@ void LFUDAPolicy::cleaning(const DoutPrefixProvider* dpp)
                if (null_block.version == e->version) {
                  block.cacheObj.dirty = false;
                  null_block.cacheObj.dirty = false;
-                 //start redis transaction using MULTI
-                 blockDir->multi(dpp, y);
                  auto blk_op_ret = blockDir->set(dpp, &block, y);
                  auto null_op_ret = blockDir->set(dpp, &null_block, y);
                  if (blk_op_ret < 0 || null_op_ret < 0) {
-                   blockDir->discard(dpp, y);
                    ldpp_dout(dpp, 0) << __func__ << "(): Failed to Queue update dirty flag for latest entry/null entry in block directory" << dendl;
-                 } else {
-                   std::vector<std::string> responses;
-                   ret = blockDir->exec(dpp, responses, y);
-                   if (responses.empty()) {
-                     //transaction failed, which means latest hash entry has been modified by a put/del so ignore and do not update the entries
-                     ldpp_dout(dpp, 0) << __func__ << "(): Execute responses are empty which means transaction failed!" << dendl;
-                   }
                  }
                }
              }
@@ -937,11 +922,6 @@ void LFUDAPolicy::cleaning(const DoutPrefixProvider* dpp)
          //the next steps remove the entry from the ordered set and if needed the latest hash entry also in case of versioned buckets
          rgw::d4n::CacheBlock latest_block = block;
          latest_block.cacheObj.objName = c_obj->get_name();
-         //add watch on latest entry, as it can be modified by a put or a del
-         ret = blockDir->watch(dpp, &latest_block, y);
-         if (ret < 0) {
-           ldpp_dout(dpp, 0) << __func__ << "(): Failed to add a watch on: " << latest_block.cacheObj.objName << ", ret=" << ret << dendl;
-         }
          int retry = 3;
          while(retry) {
            retry--;
@@ -950,14 +930,11 @@ void LFUDAPolicy::cleaning(const DoutPrefixProvider* dpp)
            if (ret < 0) {
              ldpp_dout(dpp, 0) << __func__ << "(): Failed to get latest entry in block directory for: " << latest_block.cacheObj.objName << ", ret=" << ret << dendl;
            }
-           //start redis transaction using MULTI
-           blockDir->multi(dpp, y);
            if (latest_block.version == e->version) {
              //remove object entry from ordered set
              if (c_obj->have_instance()) {
-               blockDir->del(dpp, &latest_block, y, true);
+               blockDir->del(dpp, &latest_block, y);
                if (ret < 0) {
-                 blockDir->discard(dpp, y);
                  ldpp_dout(dpp, 0) << __func__ << "(): Failed to queue del for latest hash entry: " << latest_block.cacheObj.objName << ", ret=" << ret << dendl;
                  continue;
                }
@@ -970,16 +947,9 @@ void LFUDAPolicy::cleaning(const DoutPrefixProvider* dpp)
            };
            ret = objDir->zremrangebyscore(dpp, &dir_obj, e->creationTime, e->creationTime, y, true);
            if (ret < 0) {
-             blockDir->discard(dpp, y);
              ldpp_dout(dpp, 0) << __func__ << "(): Failed to remove object from ordered set with error: " << ret << dendl;
              continue;
            }
-           std::vector<std::string> responses;
-           ret = blockDir->exec(dpp, responses, y);
-           if (responses.empty()) {
-             ldpp_dout(dpp, 0) << __func__ << "(): Execute responses are empty hence continuing!" << dendl;
-             continue;
-           }
            break;
          }//end-while (retry)
        }

diff --git a/src/rgw/driver/d4n/rgw_sal_d4n.cc b/src/rgw/driver/d4n/rgw_sal_d4n.cc
index ef471eee83c04a55c0a98a318930641ed04ceefd..af31d247937156ec01a8bdca617f58477b362b84 100644 (file)
--- a/src/rgw/driver/d4n/rgw_sal_d4n.cc
+++ b/src/rgw/driver/d4n/rgw_sal_d4n.cc
@@ -576,12 +576,6 @@ int D4NFilterObject::create_delete_marker(const DoutPrefixProvider* dpp, optiona
       driver->get_policy_driver()->get_cache_policy()->update(dpp, key, 0, bl.length(), version, true, rgw::d4n::RefCount::NOOP, y);
       std::vector<std::string> exec_responses;
       ret = this->set_head_obj_dir_entry(dpp, &exec_responses , y, true, true);
-      if (exec_responses.empty()) {
-        ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Exec respones are empty, error occured!" << dendl;
-        driver->get_policy_driver()->get_cache_policy()->erase(dpp, key, y);
-        driver->get_cache_driver()->delete_data(dpp, key, y);
-        return -ERR_INTERNAL_ERROR;
-      }
       if (ret < 0) {
         ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for head object, ret=" << ret << dendl;
         return ret;
@@ -641,12 +635,9 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, std::
 
     //dirty objects
     if (dirty) {
-    //start redis transaction using MULTI, to ensure that both latest and null block are added at the same time.
-      blockDir->multi(dpp, y);
       auto ret = blockDir->set(dpp, &block, y);
       if (ret < 0) {
         ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for head object with ret: " << ret << dendl;
-        blockDir->discard(dpp, y);
         return ret;
       }
       /* bucket is non versioned, set a null instance
@@ -657,7 +648,6 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, std::
         ret = blockDir->set(dpp, &block, y);
         if (ret < 0) {
           ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for null head object with ret: " << ret << dendl;
-          blockDir->discard(dpp, y);
           return ret;
         }
       }
@@ -672,34 +662,20 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, std::
       auto score = ceph::real_clock::to_time_t(mtime);
       ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): Score of object name: "<< this->get_name() << " version: " << object_version << " is: "  << std::setprecision(std::numeric_limits<double>::max_digits10) << score << ret << dendl;
       rgw::d4n::ObjectDirectory* objDir = this->driver->get_obj_dir();
-      ret = objDir->zadd(dpp, &object, score, object_version, y, true);
+      ret = objDir->zadd(dpp, &object, score, object_version, y);
       if (ret < 0) {
         ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): Failed to add object to ordered set with error: " << ret << dendl;
-        blockDir->discard(dpp, y);
-        return ret;
-      }
-      //execute redis transaction using EXEC
-      std::vector<std::string> responses;
-      ret = blockDir->exec(dpp, responses, y);
-      if (ret < 0) {
-        ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory execute method failed for latest and null head object with ret: " << ret << dendl;
         return ret;
       }
-      if (exec_responses) {
-        *exec_responses = responses;
-      }
     } else { //for clean/non-dirty objects
       rgw::d4n::CacheBlock latest = block;
       auto ret = blockDir->get(dpp, &latest, y);
       if (ret == -ENOENT) {
         if (!(this->get_bucket()->versioned())) {
-          //start redis transaction using MULTI, to ensure that both latest and null block are added at the same time.
-          blockDir->multi(dpp, y);
           //we can explore pipelining to send the two 'HSET' commands together
           ret = blockDir->set(dpp, &block, y);
           if (ret < 0) {
-      ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for head object with ret: " << ret << dendl;
-            blockDir->discard(dpp, y);
+              ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for head object with ret: " << ret << dendl;
             return ret;
           }
           //bucket is non versioned, set a null instance
@@ -707,19 +683,8 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, std::
           ret = blockDir->set(dpp, &block, y);
           if (ret < 0) {
             ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for null head object with ret: " << ret << dendl;
-            blockDir->discard(dpp, y);
-            return ret;
-          }
-          //execute redis transaction using EXEC
-          std::vector<std::string> responses;
-          ret = blockDir->exec(dpp, responses, y);
-          if (ret < 0) {
-            ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory execute method failed for latest and null head object with ret: " << ret << dendl;
             return ret;
           }
-          if (exec_responses) {
-            *exec_responses = responses;
-          }
         }
       } else if (ret < 0) {
         ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory get method failed for head object with ret: " << ret << dendl;
@@ -736,12 +701,9 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, std::
         /* even if the head block is found, overwrite existing values with new version in case of non-versioned bucket, clean objects
            and versioned and non-versioned buckets dirty objects */
         if (!(this->get_bucket()->versioned())) {
-          //start redis transaction using MULTI, to ensure that both latest and null block are added at the same time.
-          blockDir->multi(dpp, y);
           ret = blockDir->set(dpp, &block, y);
           if (ret < 0) {
             ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for head object with ret: " << ret << dendl;
-            blockDir->discard(dpp, y);
             return ret;
           }
           //bucket is non versioned, set a null instance
@@ -749,19 +711,8 @@ int D4NFilterObject::set_head_obj_dir_entry(const DoutPrefixProvider* dpp, std::
           ret = blockDir->set(dpp, &block, y);
           if (ret < 0) {
             ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory set method failed for null head object with ret: " << ret << dendl;
-            blockDir->discard(dpp, y);
             return ret;
           }
-          //execute redis transaction using EXEC
-          std::vector<std::string> responses;
-          ret = blockDir->exec(dpp, responses, y);
-          if (ret < 0) {
-            ldpp_dout(dpp, 10) << "D4NFilterObject::" << __func__ << "(): BlockDirectory execute method failed for latest and null head object with ret: " << ret << dendl;
-            return ret;
-          }
-          if (exec_responses) {
-            *exec_responses = responses;
-          }
         }//end-if !(this->get_bucket()->versioned())
       } //end-if ret = 0
     } //end-else
@@ -2050,11 +2001,6 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
           //add watch on latest entry, as it can be modified by a put or another del
           rgw::d4n::CacheBlock latest_block = block;
           latest_block.cacheObj.objName = objName;
-          ret = blockDir->watch(dpp, &latest_block, y);
-          if (ret < 0) {
-            ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to add a watch on: " << latest_block.cacheObj.objName << ", ret=" << ret << dendl;
-            return ret;
-          }
           int retry = 3;
           while(retry) {
             retry--;
@@ -2062,7 +2008,6 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
             ret = blockDir->get(dpp, &latest_block, y);
             if (ret < 0) {
               ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to get latest entry in block directory for: " << latest_block.cacheObj.objName << ", ret=" << ret << dendl;
-              blockDir->unwatch(dpp, y);
               return ret;
             }
             //simple delete request with no version id - create a delete marker
@@ -2077,7 +2022,6 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
                   if (ret == -ERR_INTERNAL_ERROR) {
                     continue;
                   } else {
-                    blockDir->unwatch(dpp, y);
                     return ret;
                   }
                 }
@@ -2088,12 +2032,6 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
                   return 0;
                 }
               }
-              //unwatch the key (latest entry), as it is already a delete marker and we won't do anything
-              ret = blockDir->unwatch(dpp, y);
-              if (ret < 0) {
-                ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to add a watch on: " << latest_block.cacheObj.objName << ", ret=" << ret << dendl;
-                return ret;
-              }
               transaction_success = true;
               return 0;
             } else { //not a simple request, delete version requested
@@ -2103,7 +2041,6 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
                   .objName = objName,
                   .bucketName = source->get_bucket()->get_bucket_id(),
                 };
-                bool startmulti = false;
                 //check if version to be deleted is the same as latest version
                 if (latest_block.version == block.version) {
                   std::vector<std::string> members;
@@ -2111,60 +2048,38 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
                   ret = objDir->zrevrange(dpp, &dir_obj, 0, 1, members, y);
                   if (ret < 0) {
                     ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to get the second latest version for: " << dir_obj.objName << ", ret=" << ret << dendl;
-                    blockDir->unwatch(dpp, y);
                     return ret;
                   }
                   //if there is a second latest version
                   if (members.size() == 2) {
                     rgw::d4n::CacheBlock version_block = latest_block;
                     version_block.cacheObj.objName = "_:" + members[1] + "_" + source->get_name();
-                    //add watch on the second latest versioned entry also as it might be modified by another del
-                    ret = blockDir->watch(dpp, &version_block, y);
-                    if (ret < 0) {
-                      ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to add a watch on: " << version_block.cacheObj.objName << ", ret=" << ret << dendl;
-                      blockDir->unwatch(dpp, y);
-                      return ret;
-                    }
                     //get versioned entry
                     ret = blockDir->get(dpp, &version_block, y);
                     if (ret < 0) {
                       ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to get the versioned entry for: " << version_block.cacheObj.objName << ", ret=" << ret << dendl;
-                      blockDir->unwatch(dpp, y);
                       return 0;
                     }
-                    //start redis transaction using MULTI
-                    blockDir->multi(dpp, y);
-                    startmulti = true;
                     //set versioned entry as the latest entry
                     version_block.cacheObj.objName = latest_block.cacheObj.objName;
                     ldpp_dout(dpp, 20) << "D4NFilterObject::" << __func__ << "(): INFO: promoting latest version entry to version: " << version_block.version << ", ret=" << ret << dendl;
                     ret = blockDir->set(dpp, &version_block, y);
                     if (ret < 0) {
                       ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to set new latest entry for: " << version_block.cacheObj.objName << ", ret=" << ret << dendl;
-                      blockDir->discard(dpp, y);
                       return 0;
                     }
                   } else { // there are no more versions left
-                    //start redis transaction using MULTI
-                    blockDir->multi(dpp, y);
-                    startmulti = true;
                     //delete latest block entry
-                    ret = blockDir->del(dpp, &latest_block, y, true);
+                    ret = blockDir->del(dpp, &latest_block, y);
                     if (ret < 0) {
                       ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to delete latest entry in block directory, when it is the same as version requested, for: " << block.cacheObj.objName << ", ret=" << ret << dendl;
-                      blockDir->discard(dpp, y);
                       return ret;
                     }
                   }
                 } //end-if latest_block.version == block.version
                 //delete versioned entry (handles delete markers also)
-                if (!startmulti) {
-                  //start redis transaction using MULTI
-                  blockDir->multi(dpp, y);
-                }
-                if ((ret = blockDir->del(dpp, &block, y, true)) < 0 && ret != -ENOENT) {
+                if ((ret = blockDir->del(dpp, &block, y)) < 0 && ret != -ENOENT) {
                   ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to delete head object in block directory for: " << block.cacheObj.objName << ", ret=" << ret << dendl;
-                  blockDir->discard(dpp, y);
                   return ret;
                 }
                 //delete entry from ordered set
@@ -2173,15 +2088,8 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
                 ret = objDir->zrem(dpp, &dir_obj, version, y, true);
                 if (ret < 0) {
                   ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to Queue zrem request in object directory for: " << source->get_name() << ", ret=" << ret << dendl;
-                  blockDir->discard(dpp, y);
                   return ret;
                 }
-                std::vector<std::string> responses;
-                ret = blockDir->exec(dpp, responses, y);
-                if (responses.empty()) {
-                  ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Execute responses are empty hence continuing!" << dendl;
-                  continue;
-                }
                 if (ret < 0) {
                   ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to execute exec in block directory: " << "ret= " << ret << dendl;
                   return ret;
@@ -2204,13 +2112,10 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
     /* Non-versioned buckets - we will delete the latest entry and the "null" entry
        dirty objects - delete "null" entry from ordered set also */
     if (!source->get_bucket()->versioned()) {
-      //start redis transaction using MULTI to delete the latest entry and the "null" entry together
-      blockDir->multi(dpp, y);
       //explore redis pipelining to send the two 'DEL' commands together in a single request
-      ret = blockDir->del(dpp, &block, y, true);
+      ret = blockDir->del(dpp, &block, y);
       if (ret < 0) {
         ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to Queue delete head object op in block directory for: " << block.cacheObj.objName << ", ret=" << ret << dendl;
-        blockDir->discard(dpp, y);
         return ret;
       }
       //if we get request for latest head entry, delete the null block and vice versa
@@ -2219,9 +2124,10 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
       } else {
         block.cacheObj.objName = source->get_name();
       }
-      if ((ret = blockDir->del(dpp, &block, y)) < 0) {
+      ret = blockDir->del(dpp, &block, y);
+      if (ret < 0) {
         ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to Queue delete head object in block directory for: " << block.cacheObj.objName << ", ret=" << ret << dendl;
-        blockDir->discard(dpp, y);
+        return ret;
       }
       //dirty objects - delete from ordered set
       if (objDirty) {
@@ -2232,16 +2138,9 @@ int D4NFilterObject::D4NFilterDeleteOp::delete_obj(const DoutPrefixProvider* dpp
         ret = objDir->zrem(dpp, &dir_obj, "null", y, true);
         if (ret < 0) {
           ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to Queue zrem request in object directory for: " << source->get_name() << ", ret=" << ret << dendl;
-          blockDir->discard(dpp, y);
           return ret;
         }
       }
-      std::vector<std::string> responses;
-      ret = blockDir->exec(dpp, responses, y);
-      if (ret < 0) {
-        ldpp_dout(dpp, 0) << "D4NFilterObject::" << __func__ << "(): Failed to execute exec in block directory: " << "ret= " << ret << dendl;
-        return ret;
-      }
     } //end-if non-versioned buckets
 
     int size;

diff --git a/src/test/rgw/test_d4n_directory.cc b/src/test/rgw/test_d4n_directory.cc
index c75c0f0610b94386f4c02d1227caf7342a7b57d6..4816c47666db0fa840ac12d8a7530ff0e8ba4996 100644 (file)
--- a/src/test/rgw/test_d4n_directory.cc
+++ b/src/test/rgw/test_d4n_directory.cc
@@ -717,8 +717,8 @@ TEST_F(BlockDirectoryFixture, MultiExecuteYield)
       }
       {
         request req;
-        //string as response here as the command is only getting queued, not executed
-        //if response type is changed to int then the operation fails
+        /* string as response here as the command is only getting queued, not executed
+           if response type is changed to int then the operation fails */
         response<std::string> resp;
         req.push("DEL", "key3");                  // Command 4
         conn->async_exec(req, resp, yield[ec]);