[Sync] Store the past 10 traffic records in memory.

sync/engine/traffic_logger.cc

Index: sync/engine/traffic_logger.cc
diff --git a/sync/engine/traffic_logger.cc b/sync/engine/traffic_logger.cc
index b066af5902c5319facc0d8e06c9c0d08c839ce64..af16f8520fa45107c3ef632f181175ece90ccf46 100644
--- a/sync/engine/traffic_logger.cc
+++ b/sync/engine/traffic_logger.cc
@@ -4,6 +4,7 @@
 
 #include "sync/engine/traffic_logger.h"
 
+#include <queue>
 #include <string>
 
 #include "base/json/json_writer.h"
@@ -12,9 +13,25 @@
 #include "base/values.h"
 #include "sync/protocol/proto_value_conversions.h"
 #include "sync/protocol/sync.pb.h"
+#include "sync/sessions/sync_session.h"
 
 namespace browser_sync {
 
+using sessions::SyncSession;
+
+TrafficRecord::TrafficRecord(const std::string& message,
+                             TrafficMessageType message_type,
+                             bool truncated) :
+    message(message),
+    message_type(message_type),
+    truncated(truncated) {
+}
+
+TrafficRecord::~TrafficRecord() {
+}
+
+static const unsigned int kMaxMessages = 10;
+static const unsigned int kMaxMessageSize = 100 * 1024;

[akalin, 2012/03/22 20:27:28]

100k * 10 = 1MB, which seems like a lot just for logging.  I was thinking
something more on the order of 10k-50k total, So that would be max 1k-5k per
message.  Or you could just enforce the total size and let an individual message
grow as large as it can.

[lipalani1, 2012/03/23 00:03:11]

Letting the individual message size unbounded and having limit only on total has
some issues. Eg: if a message is pretty close to the limit it would destroy the
entire queue. Thus we have unpredictable number of messages in the queue. I
think the solution would be trim the specifics if the message size is bigger. I
have added the todo.
On 2012/03/22 20:27:28, akalin wrote:

 namespace {
 template <class T>
 void LogData(const T& data,
@@ -30,21 +47,48 @@ void LogData(const T& data,
     DVLOG(1) << "\n" << description << "\n" << message << "\n";
   }
 }
+
+void AddTrafficToQueue(std::queue<TrafficRecord>* traffic_recorder,

[akalin, 2012/03/22 20:27:28]

you'll run into problems later when reading the entries in the queue, since I
believe you can only look at the top.  You should just use the underlying data
structure (a deque<>) and use push_back/pop_front.

[lipalani1, 2012/03/23 00:03:11]

The next patch would only need to access the front of the queue and once it is
displayed it is popped out. I dont think deque functionality is needed.
On 2012/03/22 20:27:28, akalin wrote:

[akalin, 2012/03/23 00:58:50]

I see.  But that's still not ideal.  So if you do a dump and refresh about:sync
(for whatever reason) you lose the data?  That seems weird.  It should probably
be a read accessor and not a read-and-clear.

[lipalani1, 2012/03/26 21:25:21]

I did not want the memory to be used for displaying in JS and us having a copy
internally. But you are right because it is little weird and it is little hard
to explain to all users.
On 2012/03/23 00:58:50, akalin wrote:

+                       const TrafficRecord& record) {
+  while (traffic_recorder->size() >= kMaxMessages) {
+    traffic_recorder->pop();
+  }
+
+  traffic_recorder->push(record);

[akalin, 2012/03/22 20:27:28]

surely this should come before the popping?  Otherwise, you effectively have a
max count of kMaxMessages+1.

[lipalani1, 2012/03/23 00:03:11]

i did not mean >= in the previous while block. Changed it to just >.
On 2012/03/22 20:27:28, akalin wrote:

+}
+
+void StoreProtoInQueue(const ::google::protobuf::MessageLite& msg,
+                       TrafficMessageType type,
+                       SyncSession* session) {
+  bool truncated = false;
+  std::string message;
+  if (msg.ByteSize() >= kMaxMessageSize) {
+    truncated = true;
+  } else {
+    msg.SerializeToString(&message);
+  }
+
+  TrafficRecord record(message, type, truncated);
+  AddTrafficToQueue(session->context()->traffic_recorder(), record);
+}
+
 }  // namespace
 
-void LogClientToServerMessage(const sync_pb::ClientToServerMessage& msg) {
+void LogClientToServerMessage(const sync_pb::ClientToServerMessage& msg,
+                              SyncSession* session) {
   LogData(msg, &ClientToServerMessageToValue,
           "******Client To Server Message******");
-  // TODO(lipalani) : Store the data (minus specifics)
-  // in a circular buffer in memory.
+
+  StoreProtoInQueue(msg, CLIENT_TO_SERVER_MESSAGE, session);
 }
 
 void LogClientToServerResponse(
-    const sync_pb::ClientToServerResponse& response) {
+    const sync_pb::ClientToServerResponse& response,
+    SyncSession* session) {
   LogData(response, &ClientToServerResponseToValue,
           "******Server Response******");
-  // TODO(lipalani) : Store the data (minus specifics)
-  // in a circular buffer in memory.
+
+  StoreProtoInQueue(response, CLIENT_TO_SERVER_RESPONSE, session);
 }
 
 }  // namespace browser_sync