Remove delta update in Report batch

include/istio/mixerclient/options.h

@@ -45,8 +45,8 @@ struct CheckOptions {
 // Options controlling report batch.
 struct ReportOptions {
   // Default constructor.
-  // Default to batch up to 1000 reports or 1 seconds.
-  ReportOptions() : max_batch_entries(1000), max_batch_time_ms(1000) {}
+  // Default to batch up to 500 reports or 1 seconds.
+  ReportOptions() : max_batch_entries(100), max_batch_time_ms(1000) {}
 
   // Constructor.
   ReportOptions(int max_batch_entries, int max_batch_time_ms)

src/istio/mixerclient/BUILD

@@ -42,8 +42,6 @@ cc_library(
         "check_cache.h",
         "client_impl.cc",
         "client_impl.h",
-        "delta_update.cc",
-        "delta_update.h",
         "global_dictionary.cc",
         "global_dictionary.h",
         "quota_cache.cc",
@@ -96,17 +94,6 @@ cc_test(
     ],
 )
 
-cc_test(
-    name = "delta_update_test",
-    size = "small",
-    srcs = ["delta_update_test.cc"],
-    linkstatic = 1,
-    deps = [
-        ":mixerclient_lib",
-        "//external:googletest_main",
-    ],
-)
-
 cc_test(
     name = "report_batch_test",
     size = "small",

src/istio/mixerclient/attribute_compressor.cc

@@ -15,7 +15,6 @@
 
 #include "src/istio/mixerclient/attribute_compressor.h"
 #include "include/istio/utils/protobuf.h"
-#include "src/istio/mixerclient/delta_update.h"
 #include "src/istio/mixerclient/global_dictionary.h"
 
 using ::istio::mixer::v1::Attributes;
@@ -77,22 +76,15 @@ ::istio::mixer::v1::StringMap CreateStringMap(
   return compressed_map;
 }
 
-bool CompressByDict(const Attributes& attributes, MessageDictionary& dict,
-                    DeltaUpdate& delta_update, CompressedAttributes* pb) {
-  delta_update.Start();
-
+void CompressByDict(const Attributes& attributes, MessageDictionary& dict,
+                    CompressedAttributes* pb) {
   // Fill attributes.
   for (const auto& it : attributes.attributes()) {
     const std::string& name = it.first;
     const Attributes_AttributeValue& value = it.second;
 
     int index = dict.GetIndex(name);
 
-    // Check delta update. If same, skip it.
-    if (delta_update.Check(index, value)) {
-      continue;
-    }
-
     // Fill the attribute to proper map.
     switch (value.value_case()) {
       case Attributes_AttributeValue::kStringValue:
@@ -124,26 +116,19 @@ bool CompressByDict(const Attributes& attributes, MessageDictionary& dict,
         break;
     }
   }
-
-  return delta_update.Finish();
 }
 
 class BatchCompressorImpl : public BatchCompressor {
  public:
   BatchCompressorImpl(const GlobalDictionary& global_dict)
-      : dict_(global_dict),
-        delta_update_(DeltaUpdate::Create()),
-        report_(new ::istio::mixer::v1::ReportRequest) {
+      : dict_(global_dict), report_(new ::istio::mixer::v1::ReportRequest) {
     report_->set_global_word_count(global_dict.size());
   }
 
-  bool Add(const Attributes& attributes) override {
+  void Add(const Attributes& attributes) override {
     CompressedAttributes pb;
-    if (!CompressByDict(attributes, dict_, *delta_update_, &pb)) {
-      return false;
-    }
+    CompressByDict(attributes, dict_, &pb);
     pb.GetReflection()->Swap(report_->add_attributes(), &pb);
-    return true;
   }
 
   int size() const override { return report_->attributes_size(); }
@@ -157,7 +142,6 @@ class BatchCompressorImpl : public BatchCompressor {
 
  private:
   MessageDictionary dict_;
-  std::unique_ptr<DeltaUpdate> delta_update_;
   std::unique_ptr<::istio::mixer::v1::ReportRequest> report_;
 };
 
@@ -194,9 +178,7 @@ void AttributeCompressor::Compress(
     const Attributes& attributes,
     ::istio::mixer::v1::CompressedAttributes* pb) const {
   MessageDictionary dict(global_dict_);
-  std::unique_ptr<DeltaUpdate> delta_update = DeltaUpdate::CreateNoOp();
-
-  CompressByDict(attributes, dict, *delta_update, pb);
+  CompressByDict(attributes, dict, pb);
 
   for (const std::string& word : dict.GetWords()) {
     pb->add_words(word);

src/istio/mixerclient/attribute_compressor.h

@@ -50,8 +50,7 @@ class BatchCompressor {
   virtual ~BatchCompressor() {}
 
   // Add an attribute set to the batch.
-  // Return false if it could not be added for delta update.
-  virtual bool Add(const ::istio::mixer::v1::Attributes& attributes) = 0;
+  virtual void Add(const ::istio::mixer::v1::Attributes& attributes) = 0;
 
   // Get the batched size.
   virtual int size() const = 0;

src/istio/mixerclient/attribute_compressor_test.cc

@@ -146,10 +146,22 @@ attributes {
   }
 }
 attributes {
+  strings {
+    key: 2
+    value: 127
+  }
+  strings {
+    key: 6
+    value: 101
+  }
   int64s {
     key: 1
     value: 135
   }
+  int64s {
+    key: 8
+    value: 8080
+  }
   int64s {
     key: 27
     value: 111
@@ -162,6 +174,75 @@ attributes {
     key: 71
     value: false
   }
+  timestamps {
+    key: 132
+    value {
+    }
+  }
+  durations {
+    key: 29
+    value {
+      seconds: 5
+    }
+  }
+  bytes {
+    key: 0
+    value: "text/html; charset=utf-8"
+  }
+  string_maps {
+    key: 15
+    value {
+      entries {
+        key: 32
+        value: 90
+      }
+      entries {
+        key: 58
+        value: 104
+      }
+    }
+  }
+}
+attributes {
+  strings {
+    key: 2
+    value: 127
+  }
+  strings {
+    key: 6
+    value: 101
+  }
+  int64s {
+    key: 1
+    value: 135
+  }
+  int64s {
+    key: 8
+    value: 8080
+  }
+  doubles {
+    key: 78
+    value: 123.99
+  }
+  bools {
+    key: 71
+    value: false
+  }
+  timestamps {
+    key: 132
+    value {
+    }
+  }
+  durations {
+    key: 29
+    value {
+      seconds: 5
+    }
+  }
+  bytes {
+    key: 0
+    value: "text/html; charset=utf-8"
+  }
   string_maps {
     key: 15
     value {
@@ -177,7 +258,7 @@ attributes {
   }
 }
 default_words: "JWT-Token"
-global_word_count: 111
+global_word_count: 221
 )";
 
 class AttributeCompressorTest : public ::testing::Test {
@@ -234,7 +315,7 @@ TEST_F(AttributeCompressorTest, BatchCompressTest) {
   AttributeCompressor compressor;
   auto batch_compressor = compressor.CreateBatchCompressor();
 
-  EXPECT_TRUE(batch_compressor->Add(attributes_));
+  batch_compressor->Add(attributes_);
 
   // modify some attributes
   utils::AttributesBuilder builder(&attributes_);
@@ -245,13 +326,13 @@ TEST_F(AttributeCompressorTest, BatchCompressTest) {
   builder.AddStringMap("request.headers", {{"content-type", "application/json"},
                                            {":method", "GET"}});
 
-  // Since there is no deletion, batch is good
-  EXPECT_TRUE(batch_compressor->Add(attributes_));
+  // Batch the second one with added attributes
+  batch_compressor->Add(attributes_);
 
   // remove a key
   attributes_.mutable_attributes()->erase("response.size");
-  // Batch should fail.
-  EXPECT_FALSE(batch_compressor->Add(attributes_));
+  // Batch the third with a removed attribute.
+  batch_compressor->Add(attributes_);
 
   auto report_pb = batch_compressor->Finish();
 
@@ -262,7 +343,7 @@ TEST_F(AttributeCompressorTest, BatchCompressTest) {
   ::istio::mixer::v1::ReportRequest expected_report_pb;
   ASSERT_TRUE(
       TextFormat::ParseFromString(kReportAttributes, &expected_report_pb));
-  report_pb->set_global_word_count(111);
+  report_pb->set_global_word_count(221);
   EXPECT_TRUE(MessageDifferencer::Equals(*report_pb, expected_report_pb));
 }