trafficserver-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From zw...@apache.org
Subject [26/28] git commit: TS-2519: handle stat persistence type changes
Date Mon, 27 Jan 2014 20:41:56 GMT
TS-2519: handle stat persistence type changes

When Traffic Server is upgraded, it is possible for stats to change
their persistence type. If it changes from persistent to non-persistent,
we don't want to preserve the previous value. Detect this case when
we load the records snapshot and also when we register stats.


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

Branch: refs/heads/5.0.x
Commit: e2ffb69c179c7daf65e819b29c90ebde4546b94e
Parents: 7eeb5c7
Author: James Peach <jpeach@apache.org>
Authored: Thu Jan 23 16:21:25 2014 -0800
Committer: James Peach <jpeach@apache.org>
Committed: Mon Jan 27 09:40:13 2014 -0800

----------------------------------------------------------------------
 lib/records/I_RecCore.h  |  1 +
 lib/records/P_RecCore.cc | 30 ++++++++++++++++++++++++--
 lib/records/RecCore.cc   | 49 ++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 75 insertions(+), 5 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e2ffb69c/lib/records/I_RecCore.h
----------------------------------------------------------------------
diff --git a/lib/records/I_RecCore.h b/lib/records/I_RecCore.h
index e9b1a4f..5bc1358 100644
--- a/lib/records/I_RecCore.h
+++ b/lib/records/I_RecCore.h
@@ -151,6 +151,7 @@ int RecGetRecordByte(const char *name, RecByte * rec_byte, bool lock =
true);
 //------------------------------------------------------------------------
 int RecGetRecordType(const char *name, RecT * rec_type, bool lock = true);
 int RecGetRecordDataType(const char *name, RecDataT * data_type, bool lock = true);
+int RecGetRecordPersistenceType(const char *name, RecPersistT * persist_type, bool lock =
true);
 int RecGetRecordUpdateCount(RecT data_type);
 int RecGetRecordOrderAndId(const char *name, int *order, int *id, bool lock = true);
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e2ffb69c/lib/records/P_RecCore.cc
----------------------------------------------------------------------
diff --git a/lib/records/P_RecCore.cc b/lib/records/P_RecCore.cc
index 152bf05..b90a5d3 100644
--- a/lib/records/P_RecCore.cc
+++ b/lib/records/P_RecCore.cc
@@ -531,6 +531,7 @@ RecReadStatsFile()
   RecRecord *r;
   RecMessage *m;
   RecMessageItr itr;
+  RecPersistT persist_type = RECP_NULL;
   xptr<char> snap_fpath(RecConfigReadPersistentStatsPath());
 
   // lock our hash table
@@ -539,8 +540,33 @@ RecReadStatsFile()
   if ((m = RecMessageReadFromDisk(snap_fpath)) != NULL) {
     if (RecMessageUnmarshalFirst(m, &itr, &r) != REC_ERR_FAIL) {
       do {
-        if ((r->name == NULL) || (!strlen(r->name)))
+        if ((r->name == NULL) || (!strlen(r->name))) {
           continue;
+        }
+
+        // If we don't have a persistence type for this record, it means that it is not a
stat, or it is
+        // not registered yet. Either way, it's ok to just set the persisted value and keep
going.
+        if (RecGetRecordPersistenceType(r->name, &persist_type, false /* lock */)
!= REC_ERR_OKAY) {
+          RecDebug(DL_Debug, "restoring value for persisted stat '%s'", r->name);
+          RecSetRecord(r->rec_type, r->name, r->data_type, &(r->data), &(r->stat_meta.data_raw),
false);
+          continue;
+        }
+
+        if (!REC_TYPE_IS_STAT(r->rec_type)) {
+          // This should not happen, but be defensive against records changing their type
..
+          RecLog(DL_Warning, "skipping restore of non-stat record '%s'", r->name);
+          continue;
+        }
+
+        // Check whether the persistence type was changed by a new software version. If the
record is
+        // already registered with an updated persistence type, then we don't want to set
it. We should
+        // keep the registered value.
+        if (persist_type == RECP_NON_PERSISTENT) {
+          RecDebug(DL_Debug, "preserving current value of formerly persistent stat '%s'",
r->name);
+          continue;
+        }
+
+        RecDebug(DL_Debug, "restoring value for persisted stat '%s'", r->name);
         RecSetRecord(r->rec_type, r->name, r->data_type, &(r->data), &(r->stat_meta.data_raw),
false);
       } while (RecMessageUnmarshalNext(m, &itr, &r) != REC_ERR_FAIL);
     }
@@ -579,7 +605,7 @@ RecSyncStatsFile()
       r = &(g_records[i]);
       rec_mutex_acquire(&(r->lock));
       if (REC_TYPE_IS_STAT(r->rec_type)) {
-        if (r->stat_meta.persist_type != RECP_NON_PERSISTENT) {
+        if (r->stat_meta.persist_type == RECP_PERSISTENT) {
           m = RecMessageMarshal_Realloc(m, r);
           sync_to_disk = true;
         }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/e2ffb69c/lib/records/RecCore.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecCore.cc b/lib/records/RecCore.cc
index 484a9c6..1ed4e10 100644
--- a/lib/records/RecCore.cc
+++ b/lib/records/RecCore.cc
@@ -44,7 +44,7 @@ RecTree *g_records_tree = NULL;
 // register_record
 //-------------------------------------------------------------------------
 static RecRecord *
-register_record(RecT rec_type, const char *name, RecDataT data_type, RecData data_default)
+register_record(RecT rec_type, const char *name, RecDataT data_type, RecData data_default,
RecPersistT persist_type)
 {
   RecRecord *r = NULL;
 
@@ -61,6 +61,10 @@ register_record(RecT rec_type, const char *name, RecDataT data_type, RecData
dat
     RecDataSet(r->data_type, &(r->data), &(data_default));
     RecDataSet(r->data_type, &(r->data_default), &(data_default));
     ink_hash_table_insert(g_records_ht, name, (void *) r);
+
+    if (REC_TYPE_IS_STAT(r->rec_type)) {
+      r->stat_meta.persist_type = persist_type;
+    }
   }
 
   // we're now registered
@@ -467,6 +471,34 @@ RecGetRecordDataType(const char *name, RecDataT * data_type, bool lock)
 }
 
 int
+RecGetRecordPersistenceType(const char *name, RecPersistT * persist_type, bool lock)
+{
+  int err = REC_ERR_FAIL;
+  RecRecord *r = NULL;
+
+  if (lock) {
+    ink_rwlock_rdlock(&g_records_rwlock);
+  }
+
+  *persist_type = RECP_NULL;
+
+  if (ink_hash_table_lookup(g_records_ht, name, (void **) &r)) {
+    rec_mutex_acquire(&(r->lock));
+    if (REC_TYPE_IS_STAT(r->rec_type)) {
+      *persist_type = r->stat_meta.persist_type;
+      err = REC_ERR_OKAY;
+    }
+    rec_mutex_release(&(r->lock));
+  }
+
+  if (lock) {
+    ink_rwlock_unlock(&g_records_rwlock);
+  }
+
+  return err;
+}
+
+int
 RecGetRecordUpdateCount(RecT data_type)
 {
   return g_num_update[data_type];
@@ -690,10 +722,21 @@ RecRegisterStat(RecT rec_type, const char *name, RecDataT data_type,
RecData dat
   RecRecord *r = NULL;
 
   ink_rwlock_wrlock(&g_records_rwlock);
-  if ((r = register_record(rec_type, name, data_type, data_default)) != NULL) {
+  if ((r = register_record(rec_type, name, data_type, data_default, persist_type)) != NULL)
{
+    // If the persistence type we found in the records hash is not the same as the persistence
+    // type we are registering, then that means that it changed between the previous software
+    // version and the current version. If the metric changed to non-persistent, reset to
the
+    // new default value.
+    if ((r->stat_meta.persist_type == RECP_NULL ||r->stat_meta.persist_type == RECP_PERSISTENT)
&&
+        persist_type == RECP_NON_PERSISTENT) {
+      RecDebug(DL_Debug, "resetting default value for formerly persisted stat '%s'", r->name);
+      RecDataSet(r->data_type, &(r->data), &(data_default));
+    }
+
     r->stat_meta.persist_type = persist_type;
   } else {
     ink_assert(!"Can't register record!");
+    RecDebug(DL_Warning, "failed to register '%s' record", name);
   }
   ink_rwlock_unlock(&g_records_rwlock);
 
@@ -711,7 +754,7 @@ RecRegisterConfig(RecT rec_type, const char *name, RecDataT data_type,
 {
   RecRecord *r;
   ink_rwlock_wrlock(&g_records_rwlock);
-  if ((r = register_record(rec_type, name, data_type, data_default)) != NULL) {
+  if ((r = register_record(rec_type, name, data_type, data_default, RECP_NULL)) != NULL)
{
     // Note: do not modify 'record->config_meta.update_required'
     r->config_meta.update_type = update_type;
     r->config_meta.check_type = check_type;


Mime
View raw message