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 <[email protected]>
Authored: Mon Feb 22 21:43:41 2016 -0800
Committer: James Peach <[email protected]>
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

Reply via email to