trafficserver git commit: TS-4248: allow metrics to change type on upgrade

[jp...@apache.org]

Repository: trafficserver
Updated Branches:
  refs/heads/master 2797931ab -> a628dd415


TS-4248: allow metrics to change type on upgrade

New software versions can alter the type of metrics, but when that
happens, the metric from the previous software version has already
been restored from persistence. In this case, we can allow the type
of the metric to change to match the new registration.


Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/a628dd41
Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/a628dd41
Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/a628dd41

Branch: refs/heads/master
Commit: a628dd4157338ade211e040b7bdf0e0b6dc73d6c
Parents: 2797931
Author: James Peach <jp...@apache.org>
Authored: Mon Feb 22 21:43:41 2016 -0800
Committer: James Peach <jp...@apache.org>
Committed: Tue Mar 1 20:31:18 2016 -0800

----------------------------------------------------------------------
 lib/records/RecCore.cc | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/a628dd41/lib/records/RecCore.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecCore.cc b/lib/records/RecCore.cc
index eddd3fd..0baf49b 100644
--- a/lib/records/RecCore.cc
+++ b/lib/records/RecCore.cc
@@ -46,17 +46,42 @@ register_record(RecT rec_type, const char *name, RecDataT data_type, RecData dat
 {
   RecRecord *r = NULL;
 
+  // Metrics are restored from persistence before they are registered. In this case, when the registration arrives, we
+  // might find that they yave changed. For example, a metric might change it's type due to a software upgrade. Records
+  // must not flip between config and metrics, but changing within those classes is OK.
   if (ink_hash_table_lookup(g_records_ht, name, (void **)&r)) {
-    ink_release_assert(r->rec_type == rec_type);
-    ink_release_assert(r->data_type == data_type);
-    // Note: do not set r->data as we want to keep the previous value
-    RecDataSet(r->data_type, &(r->data_default), &(data_default));
-    if (updated_p)
+    if (REC_TYPE_IS_STAT(rec_type)) {
+      ink_release_assert(REC_TYPE_IS_STAT(r->rec_type));
+    }
+
+    if (REC_TYPE_IS_CONFIG(rec_type)) {
+      ink_release_assert(REC_TYPE_IS_CONFIG(r->rec_type));
+    }
+
+    if (data_type != r->data_type) {
+      // Clear with the old type before resetting with the new type.
+      RecDataClear(r->data_type, &(r->data));
+      RecDataClear(r->data_type, &(r->data_default));
+
+      // If the data type changed, reset the current value to the default.
+      RecDataSet(data_type, &(r->data), &(data_default));
+    }
+
+    // NOTE: Do not set r->data as we want to keep the previous value because we almost certainly restored a persisted
+    // value before the metric was registered.
+    RecDataSet(data_type, &(r->data_default), &(data_default));
+
+    r->data_type = data_type;
+    r->rec_type = rec_type;
+
+    if (updated_p) {
       *updated_p = true;
+    }
   } else {
     if ((r = RecAlloc(rec_type, name, data_type)) == NULL) {
       return NULL;
     }
+
     // Set the r->data to its default value as this is a new record
     RecDataSet(r->data_type, &(r->data), &(data_default));
     RecDataSet(r->data_type, &(r->data_default), &(data_default));
@@ -65,8 +90,10 @@ register_record(RecT rec_type, const char *name, RecDataT data_type, RecData dat
     if (REC_TYPE_IS_STAT(r->rec_type)) {
       r->stat_meta.persist_type = persist_type;
     }
-    if (updated_p)
+
+    if (updated_p) {
       *updated_p = false;
+    }
   }
 
   // we're now registered