test(storage): fix handling of optional ranges (#9808)

I confused myself on how to interpret the default read and offset
ranges. The intent was to *not* set a `ReadRange()` option if the
command line arguments were not set. The effect was to set an empty
`ReadRange()` value, which produced an unexpected `Range:` header.
Fortunately, the value of the header had no effect, but really confused
me.

With this change the optional parameters are represented by
`absl::optional` values. Which makes it easier to treat them correctly.

Author: coryan

google/cloud/storage/benchmarks/storage_throughput_vs_cpu_benchmark.cc

@@ -166,6 +166,16 @@ int main(int argc, char* argv[]) {
     std::cout << "\n# " << name << " Range: [" << minimum << ',' << maximum
               << "]\n# " << name << " Quantum: " << quantum;
   };
+  auto output_optional_quantized_range =
+      [](std::string const& name, auto minimum, auto maximum, auto quantum) {
+        if (!minimum.has_value() || !maximum.has_value()) {
+          std::cout << "\n# " << name << " Range: [not set]";
+        } else {
+          std::cout << "\n# " << name << " Range: [" << *minimum << ','
+                    << *maximum << "]";
+        }
+        std::cout << "\n# " << name << " Quantum: " << quantum;
+      };
 
   std::cout << "# Start time: " << gcs_bm::CurrentTime()      //
             << "\n# Labels: " << options->labels              //
@@ -204,12 +214,12 @@ int main(int argc, char* argv[]) {
   gcs_bm::PrintOptions(std::cout, "Grpc", options->grpc_options);
   gcs_bm::PrintOptions(std::cout, "Direct Path", options->direct_path_options);
 
-  output_quantized_range("Read Offset", options->minimum_read_offset,
-                         options->maximum_read_offset,
-                         options->read_offset_quantum);
-  output_quantized_range("Read Size", options->minimum_read_size,
-                         options->maximum_read_size,
-                         options->read_size_quantum);
+  output_optional_quantized_range("Read Offset", options->minimum_read_offset,
+                                  options->maximum_read_offset,
+                                  options->read_offset_quantum);
+  output_optional_quantized_range("Read Size", options->minimum_read_size,
+                                  options->maximum_read_size,
+                                  options->read_size_quantum);
 
   std::cout << "\n# Build info: " << notes << "\n";
   // Make the output generated so far immediately visible, helps with debugging.
@@ -356,15 +366,21 @@ void RunThread(ThroughputOptions const& options, std::string const& bucket_name,
   auto read_buffer_size_generator = quantized_range_generator(
       options.minimum_read_buffer_size, options.maximum_read_buffer_size,
       options.read_buffer_quantum);
-  auto read_offset_generator = quantized_range_generator(
-      options.minimum_read_offset, options.maximum_read_offset,
-      options.read_offset_quantum);
-  auto read_size_generator = quantized_range_generator(
-      options.minimum_read_size, options.maximum_read_size,
-      options.read_size_quantum);
 
   auto read_range_generator = [&](auto& g, std::int64_t object_size)
       -> absl::optional<std::pair<std::int64_t, std::int64_t>> {
+    if (!options.minimum_read_size.has_value() ||
+        !options.maximum_read_size.has_value() ||
+        !options.minimum_read_offset.has_value() ||
+        !options.maximum_read_offset.has_value()) {
+      return absl::nullopt;
+    }
+    auto read_offset_generator = quantized_range_generator(
+        *options.minimum_read_offset, *options.maximum_read_offset,
+        options.read_offset_quantum);
+    auto read_size_generator = quantized_range_generator(
+        *options.minimum_read_size, *options.maximum_read_size,
+        options.read_size_quantum);
     auto offset = (std::min)(object_size, read_offset_generator(g));
     auto size = (std::min)(object_size - offset, read_size_generator(g));
     // This makes it easier to control the ratio of ranged vs. full reads from

google/cloud/storage/benchmarks/throughput_options.cc

@@ -27,19 +27,27 @@ namespace {
 namespace gcs = ::google::cloud::storage;
 namespace gcs_ex = ::google::cloud::storage_experimental;
 
-Status ValidateQuantizedRange(std::string const& name, std::int64_t minimum,
-                              std::int64_t maximum, std::int64_t quantum) {
+Status ValidateQuantizedRange(std::string const& name,
+                              absl::optional<std::int64_t> minimum,
+                              absl::optional<std::int64_t> maximum,
+                              std::int64_t quantum) {
   using ::google::cloud::StatusCode;
-  if (minimum > maximum || minimum < 0 || maximum < 0) {
+  if (!minimum.has_value() && !maximum.has_value()) return {};
+  if (!minimum.has_value() || !maximum.has_value()) {
     std::ostringstream os;
-    os << "Invalid range for " << name << " [" << minimum << ',' << maximum
+    os << "One of the range limits for " << name << " is missing";
+    return google::cloud::Status{StatusCode::kInvalidArgument, os.str()};
+  }
+  if (*minimum > *maximum || *minimum < 0 || *maximum < 0) {
+    std::ostringstream os;
+    os << "Invalid range for " << name << " [" << *minimum << ',' << *maximum
        << "]";
     return google::cloud::Status{StatusCode::kInvalidArgument, os.str()};
   }
-  if (quantum <= 0 || (quantum > minimum && minimum != 0)) {
+  if (quantum <= 0 || (quantum > *minimum && *minimum != 0)) {
     std::ostringstream os;
     os << "Invalid quantum for " << name << " (" << quantum
-       << "), it should be in the (0," << minimum << "] range";
+       << "), it should be in the (0," << *minimum << "] range";
     return google::cloud::Status{StatusCode::kInvalidArgument, os.str()};
   }
 

google/cloud/storage/benchmarks/throughput_options.h

@@ -57,11 +57,11 @@ struct ThroughputOptions {
   std::vector<bool> enabled_crc32c = {false, true};
   std::vector<bool> enabled_md5 = {false, true};
   std::chrono::milliseconds minimum_sample_delay{};
-  std::int64_t minimum_read_offset = 0;
-  std::int64_t maximum_read_offset = 0;
+  absl::optional<std::int64_t> minimum_read_offset;
+  absl::optional<std::int64_t> maximum_read_offset;
   std::int64_t read_offset_quantum = 128 * kKiB;
-  std::int64_t minimum_read_size = 0;
-  std::int64_t maximum_read_size = 0;
+  absl::optional<std::int64_t> minimum_read_size;
+  absl::optional<std::int64_t> maximum_read_size;
   std::int64_t read_size_quantum = 128 * kKiB;
   Options client_options;
   Options rest_options;

google/cloud/storage/benchmarks/throughput_options_test.cc

@@ -326,6 +326,16 @@ TEST(ThroughputOptions, Validate) {
       "--maximum-read-offset=8",
       "--read-offset-quantum=5",
   }));
+  EXPECT_FALSE(ParseThroughputOptions({
+      "self-test",
+      "--region=r",
+      "--minimum-read-offset=4",
+  }));
+  EXPECT_FALSE(ParseThroughputOptions({
+      "self-test",
+      "--region=r",
+      "--maximum-read-offset=4",
+  }));
 
   EXPECT_FALSE(ParseThroughputOptions({
       "self-test",
@@ -340,6 +350,17 @@ TEST(ThroughputOptions, Validate) {
       "--maximum-read-size=8",
       "--read-size-quantum=5",
   }));
+  EXPECT_FALSE(ParseThroughputOptions({
+      "self-test",
+      "--region=r",
+      "--minimum-read-size=8",
+  }));
+  EXPECT_FALSE(ParseThroughputOptions({
+      "self-test",
+      "--region=r",
+      "--maximum-read-size=8",
+  }));
+
   EXPECT_FALSE(ParseThroughputOptions({
       "self-test",
       "--region=r",