Merge pull request #4234 from dmitryya/bug_10163

obj_bencher rados cli: rados bench producing wrong values when different blocksize used in writes and reads

Reviewed-by: Loic Dachary <ldachary@redhat.com>

doc/man/8/rados.rst

@@ -53,6 +53,10 @@ Options
 
    Connect to specified monitor (instead of looking through ceph.conf).
 
+.. option:: -b block_size
+
+  Set the block size for put/get ops and for write benchmarking.
+
 
 Global commands
 ===============
@@ -104,7 +108,8 @@ Pool specific commands
   sequential or random. Before running one of the reading benchmarks,
   run a write benchmark with the *--no-cleanup* option. The default
   object size is 4 MB, and the default number of simulated threads
-  (parallel writes) is 16. 
+  (parallel writes) is 16.
+  Note: -b *objsize* option is valid only in *write* mode.
 
 :command:`cleanup`
 

src/common/obj_bencher.cc

@@ -103,7 +103,7 @@ void *ObjBencher::status_printer(void *_bencher) {
     }
     if (cycleSinceChange)
       bandwidth = (double)(data.finished - previous_writes)
-        * (data.trans_size)
+        * (data.object_size)
         / (1024*1024)
         / cycleSinceChange;
     else
@@ -133,7 +133,7 @@ void *ObjBencher::status_printer(void *_bencher) {
       data.history.iops.push_back(iops);
     }
 
-    double avg_bandwidth = (double) (data.trans_size) * (data.finished)
+    double avg_bandwidth = (double) (data.object_size) * (data.finished)
       / (double)(cur_time - data.start_time) / (1024*1024);
     if (previous_writes != data.finished) {
       previous_writes = data.finished;
@@ -170,12 +170,11 @@ void *ObjBencher::status_printer(void *_bencher) {
 int ObjBencher::aio_bench(
   int operation, int secondsToRun,
   int maxObjectsToCreate,
-  int concurrentios, int op_size, bool cleanup, const char* run_name) {
+  int concurrentios, int object_size, bool cleanup, const char* run_name) {
 
   if (concurrentios <= 0) 
     return -EINVAL;
 
-  int object_size = op_size;
   int num_objects = 0;
   int r = 0;
   int prevPid = 0;
@@ -191,15 +190,12 @@ int ObjBencher::aio_bench(
         cerr << "Must write data before running a read benchmark!" << std::endl;
       return r;
     }
-  } else {
-    object_size = op_size;
   }
 
   char* contentsChars = new char[object_size];
   lock.Lock();
   data.done = false;
   data.object_size = object_size;
-  data.trans_size = op_size;
   data.in_flight = 0;
   data.started = 0;
   data.finished = num_objects;

src/common/obj_bencher.h

@@ -35,7 +35,6 @@ struct bench_history {
 struct bench_data {
   bool done; //is the benchmark is done
   int object_size; //the size of the objects
-  int trans_size; //size of the write/read to perform
   // same as object_size for write tests
   int in_flight; //number of reads/writes being waited on
   int started;

src/tools/rados/rados.cc

@@ -158,7 +158,7 @@ void usage(ostream& out)
 "   --target-pool=pool\n"
 "        select target pool by name\n"
 "   -b op_size\n"
-"        set the size of write ops for put or benchmarking\n"
+"        set the block size for put/get ops and for write benchmarking\n"
 "   -s name\n"
 "   --snap name\n"
 "        select given snap name for (read) IO\n"
@@ -1142,6 +1142,7 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts,
   string oloc, target_oloc, nspace, target_nspace;
   int concurrent_ios = 16;
   unsigned op_size = default_op_size;
+  bool block_size_specified = false;
   bool cleanup = true;
   const char *snapname = NULL;
   snap_t snapid = CEPH_NOSNAP;
@@ -1213,6 +1214,7 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts,
     if (rados_sistrtoll(i, &op_size)) {
       return -EINVAL;
     }
+    block_size_specified = true;
   }
   i = opts.find("snap");
   if (i != opts.end()) {
@@ -2260,6 +2262,12 @@ static int rados_tool_common(const std::map < std::string, std::string > &opts,
       operation = OP_RAND_READ;
     else
       usage_exit();
+    if (block_size_specified && (operation != OP_WRITE)){
+      cerr << "-b|--block_size option can be used only with `write' bench test"
+           << std::endl;
+      ret = -EINVAL;
+      goto out;
+    }
     RadosBencher bencher(g_ceph_context, rados, io_ctx);
     bencher.set_show_time(show_time);
     ret = bencher.aio_bench(operation, seconds, num_objs,