git commit: TS-2931: plugin metrics fail after a crash

[jp...@apache.org]

Repository: trafficserver
Updated Branches:
  refs/heads/master 06f5b3f8a -> d537357da


TS-2931: plugin metrics fail after a crash

If a plugin uses TSStatFindName followed by TSStatCreate, TSStatFindName
can return 0 for all the metric IDs after a traffic_server crash.

This will happen every time with the following conditions:
  1. traffic_manager has pulled the stat records from traffic_server
  2. traffic_server crashes

When traffic_server comes back up, it pulls the records from
traffic_manager. traffic_manager sends the records including the
rsb_id field. However, RecForceInsert() does not copy the rsb_id
field from the message to the actual record. Subsequent stat lookups
succeed because the stat is registered, but always receive a rsb_id
of 0.

The fix is to ensure that we copy the rsb_id field so that stat
lookups succeed.


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

Branch: refs/heads/master
Commit: d537357da69ee701c7165f0e1f07554e18324e1f
Parents: 06f5b3f
Author: James Peach <jp...@apache.org>
Authored: Wed Jul 9 13:27:01 2014 -0700
Committer: James Peach <jp...@apache.org>
Committed: Wed Jul 9 17:27:28 2014 -0700

----------------------------------------------------------------------
 CHANGES                  |  2 ++
 lib/records/P_RecCore.cc |  6 +++---
 lib/records/RecCore.cc   |  3 +++
 proxy/InkAPI.cc          | 10 +++++-----
 4 files changed, 13 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d537357d/CHANGES
----------------------------------------------------------------------
diff --git a/CHANGES b/CHANGES
index 5fa418b..d96ac08 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache Traffic Server 5.1.0
 
+  *) [TS-2931] Plugin metrics fail after a crash.
+
   *) [TS-2649] Use SSL certificate chain loading everywhere.
 
   *) [TS-2780] Core dump in SpdyRequest::clear() in production testing of SPDY

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d537357d/lib/records/P_RecCore.cc
----------------------------------------------------------------------
diff --git a/lib/records/P_RecCore.cc b/lib/records/P_RecCore.cc
index 3168741..5f0a8b3 100644
--- a/lib/records/P_RecCore.cc
+++ b/lib/records/P_RecCore.cc
@@ -113,7 +113,7 @@ send_push_message()
     if (i_am_the_record_owner(r->rec_type)) {
       if (r->sync_required & REC_PEER_SYNC_REQUIRED) {
         m = RecMessageMarshal_Realloc(m, r);
-        r->sync_required = r->sync_required & ~REC_PEER_SYNC_REQUIRED;
+        r->sync_required &= ~REC_PEER_SYNC_REQUIRED;
         send_msg = true;
       }
     }
@@ -157,10 +157,10 @@ send_pull_message(RecMessageT msg_type)
       r = &(g_records[i]);
       if (i_am_the_record_owner(r->rec_type) ||
           (REC_TYPE_IS_STAT(r->rec_type) && !(r->registered)) ||
-          (REC_TYPE_IS_STAT(r->rec_type) && !(r->stat_meta.persist_type != RECP_NON_PERSISTENT))) {
+          (REC_TYPE_IS_STAT(r->rec_type) && (r->stat_meta.persist_type == RECP_NON_PERSISTENT))) {
         rec_mutex_acquire(&(r->lock));
         m = RecMessageMarshal_Realloc(m, r);
-        r->sync_required = r->sync_required & ~REC_PEER_SYNC_REQUIRED;
+        r->sync_required &= ~REC_PEER_SYNC_REQUIRED;
         rec_mutex_release(&(r->lock));
       }
     }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d537357d/lib/records/RecCore.cc
----------------------------------------------------------------------
diff --git a/lib/records/RecCore.cc b/lib/records/RecCore.cc
index ca13f8a..863f497 100644
--- a/lib/records/RecCore.cc
+++ b/lib/records/RecCore.cc
@@ -837,7 +837,10 @@ RecForceInsert(RecRecord * record)
   // set the record value
   RecDataSet(r->data_type, &(r->data), &(record->data));
   RecDataSet(r->data_type, &(r->data_default), &(record->data_default));
+
   r->registered = record->registered;
+  r->rsb_id = record->rsb_id;
+
   if (REC_TYPE_IS_STAT(r->rec_type)) {
     r->stat_meta.persist_type = record->stat_meta.persist_type;
     r->stat_meta.data_raw = record->stat_meta.data_raw;

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/d537357d/proxy/InkAPI.cc
----------------------------------------------------------------------
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index 7b2d6aa..abc652f 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -82,14 +82,14 @@
     _HDR.m_mime = _HDR.m_http->m_fields_impl;
 
 // Globals for new librecords stats
-volatile int top_stat = 0;
-RecRawStatBlock *api_rsb;
+static volatile int api_rsb_index = 0;
+static RecRawStatBlock * api_rsb;
 
 // Library init functions needed for API.
 extern void ts_session_protocol_well_known_name_indices_init();
 
 // Globals for the Sessions/Transaction index registry
-volatile int next_argv_index = 0;
+static volatile int next_argv_index = 0;
 
 struct _STATE_ARG_TABLE {
   char* name;
@@ -5550,7 +5550,7 @@ TSHttpArgIndexReserve(const char* name, const char* description, int *arg_idx)
 {
   sdk_assert(sdk_sanity_check_null_ptr(arg_idx) == TS_SUCCESS);
 
-  int volatile ix = ink_atomic_increment(&next_argv_index, 1);
+  int ix = ink_atomic_increment(&next_argv_index, 1);
 
   if (ix < HTTP_SSN_TXN_MAX_USER_ARG) {
     state_arg_table[ix].name = ats_strdup(name);
@@ -6656,7 +6656,7 @@ TSCacheScan(TSCont contp, TSCacheKey key, int KB_per_second)
 int
 TSStatCreate(const char *the_name, TSRecordDataType the_type, TSStatPersistence persist, TSStatSync sync)
 {
-  int id = ink_atomic_increment(&top_stat, 1);
+  int id = ink_atomic_increment(&api_rsb_index, 1);
   RecRawStatSyncCb syncer = RecRawStatSyncCount;
 
   // TODO: This only supports "int" data types at this point, since the "Raw" stats