Remove broken variables from sync's AllStatus

chrome/browser/sync/protocol/client_debug_info.proto

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
 // Common sync protocol for encrypted data.
 
 syntax = "proto2";
 
 option optimize_for = LITE_RUNTIME;
 option retain_unknown_fields = true;
 
 package sync_pb;
 
 // The additional info here is from SyncerStatus. They get sent when the event
 // SYNC_CYCLE_COMPLETED  is sent.
 message SyncCycleCompletedEventInfo {
-  optional bool syncer_stuck = 1;
+  // optional bool syncer_stuck = 1; // Was always false, now obsolete.

[rlarocque, 2012/01/10 00:07:43]

This is my preferred mechanism for deprecating protobuf fields.  If you know of
a better way to handle it, let me know.

[tim (not reviewing), 2012/01/10 17:01:47]

I really wonder if the spirit of a "stuck" signal is obsolete.  We were
mis-reporting it, but it's not like we haven't had reports of "stuck" like
behavior.  Are you absolutely certain it would never be useful to send this as
debug info to the server?  I'm not.  The cost of an extra .proto field isn't
that great.  We could add the deprecated=true option (does nothing in C++ but
generates @Deprecated annotation + warnings in java), I'm not sure if that is
more or less reversible than deleting altogether... perhaps Raz or Albert can
comment.

[rlarocque, 2012/01/10 21:39:32]

Even if we revive the concept of "syncer_stuck" (I hope we don't), it will be
based on significantly different logic than the current variable.  They should
be named differently, so we don't have to research whether or not the flag is
broken in a particular version of Chrome.

[tim (not reviewing), 2012/01/11 02:15:35]

It's just that we have made no change that ensures a client will never become
"stuck". And we only added this bit here a few months ago and I don't think
we've used it a single time yet (thankfully we haven't had any major prod issues
to debug!), so I don't think there is much legacy convention to worry about. And
lastly, hopes and wishes aside, I think it's safe to say this concept will
always be around, unless we figure out how to achieve complete perfection even
in the face of cosmic rays :)

But anyway, I'm okay with removing it.  I'd be interested to hear Lingesh's
thoughts (do we want a bug to track sending up something more useful?)

[lipalani1, 2012/01/12 23:55:49]

I guess since this is a brand new feature there is not much to worry about.
Since this field is not calculated I am OK deprecating it. and when we have the
new way to calculate the stuck behaviour we could simply use a new field without
worrying about legacy.

As far as the meta question goes: I too think stuck is a useful state to track.
How about we upload 'x', the number of  sync cycles that failed consecutively up
to this point. we track that and upload the x? Which in most cases should be 0
or 1.
If we upload this then from the server side we could do about:sync page
displaying clients with high numbers for x and do some digging there.
On 2012/01/11 02:15:35, timsteele wrote:

[albertb, 2012/01/13 15:47:03]

[deprecated=true] is fine for the server. We'll get a warning if we reference
the field.

   optional int32 num_blocking_conflicts = 2;
   optional int32 num_non_blocking_conflicts = 3;
 }
 
 message DebugEventInfo {
   // These events are sent by |SyncManager| class. Note: In the code they each
   // of these events have some additional info but we are not sending them to
   // server.
   enum EventType {
     AUTH_ERROR = 1; // Auth error. Note this gets generated even during
@@ skipping 20 matching lines @@
 
   // Whether cryptographer is ready to encrypt and decrypt data.
   optional bool cryptographer_ready = 2;
 
   // Cryptographer has pending keys which indicates the correct passphrase
   // has not been provided yet.
   optional bool cryptographer_has_pending_keys = 3;
 
   // Indicates client has dropped some events to save bandwidth.
   optional bool events_dropped = 4;
-}
+}