You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2014/01/27 21:41:56 UTC
[26/28] git commit: TS-2519: handle stat persistence type changes
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 <jp...@apache.org>
Authored: Thu Jan 23 16:21:25 2014 -0800
Committer: James Peach <jp...@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;