Repository: trafficserver
Updated Branches:
  refs/heads/master 21a692ba2 -> 26b566e74


TS-3848: Extend wait_for_cache to specify dependency on cache initialization.
This closes #282.


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

Branch: refs/heads/master
Commit: 26b566e74e0a62db4b8210f51a908c1309ae96f9
Parents: 21a692b
Author: Pushkar Pradhan <[email protected]>
Authored: Mon Aug 17 22:54:05 2015 +0000
Committer: Alan M. Carroll <[email protected]>
Committed: Thu Sep 3 09:05:35 2015 -0500

----------------------------------------------------------------------
 .../configuration/records.config.en.rst         | 28 ++++++++++
 iocore/cache/Cache.cc                           | 54 ++++++++++++++++----
 iocore/cache/I_Cache.h                          | 10 ++--
 iocore/cache/I_Store.h                          |  3 ++
 iocore/cache/Store.cc                           |  5 +-
 mgmt/RecordsConfig.cc                           |  5 +-
 proxy/Main.cc                                   |  6 +--
 7 files changed, 91 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26b566e7/doc/reference/configuration/records.config.en.rst
----------------------------------------------------------------------
diff --git a/doc/reference/configuration/records.config.en.rst 
b/doc/reference/configuration/records.config.en.rst
index 70ec156..b24c26f 100644
--- a/doc/reference/configuration/records.config.en.rst
+++ b/doc/reference/configuration/records.config.en.rst
@@ -2925,5 +2925,33 @@ Sockets
    exception being if you run Traffic Server with a protocol plugin, and would
    like for it to not support HTTP requests at all.
 
+.. ts:cv:: CONFIG proxy.config.http.wait_for_cache INT 0
+
+   Accepting inbound connections and starting the cache are independent 
operations in Traffic
+   Server. This variable controls the relative timing of these operations and 
Traffic Server
+   dependency on cache because if cache is required then inbound connection 
accepts should be
+   deferred until the validity of the cache requirement is determined. If 
cache initialization
+   failure causes a Traffic Server shutdown this will be logged in 
:file:`diags.log`.
+
+===== ====================
+Value Effect
+===== ====================
+0     Decouple inbound connections and cache initialization. Connections will 
be accepted as soon as
+      possible and Traffic Server will run regardless of the results of cache 
initialization.
+
+1     Do not accept inbound connections until cache initialization has 
finished. Traffic server will run
+      regardless of the results of cache initialization.
+
+2     Do not accept inbound connections until cache initialization has 
finished and been sufficiently
+      successful that cache is enabled. This means at least one cache span is 
usable. If there are no
+      spans in :configfile:`storage.config` or none of the spans can be 
successfully parsed and
+      initialized then Traffic Server will shut down.
+
+3     Do not accept inbound connections until cache initialization has 
finished and been completely
+      successful. This requires at least one cache span in 
:configfile:`storage.config` and that every
+      span specified is valid and successfully initialized. Any error will 
cause Traffic Server to shut
+      down.
+===== ====================
+
 .. _Traffic Shaping:
                  https://cwiki.apache.org/confluence/display/TS/Traffic+Shaping

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26b566e7/iocore/cache/Cache.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Cache.cc b/iocore/cache/Cache.cc
index d0b5917..2400adc 100644
--- a/iocore/cache/Cache.cc
+++ b/iocore/cache/Cache.cc
@@ -733,11 +733,24 @@ CacheProcessor::start_internal(int flags)
     }
   }
 
+  start_done = 1;
+
   if (gndisks == 0) {
-    Warning("unable to open cache disk(s): Cache Disabled\n");
-    return -1;
+    CacheProcessor::initialized = CACHE_INIT_FAILED;
+    // Have to do this here because no IO events were scheduled and so @c 
diskInitialized() won't be called.
+    if (cb_after_init) cb_after_init();
+
+    if (this->waitForCache() > 1) {
+      Fatal("Cache initialization failed - no disks available but cache 
required");
+    } else {
+      Warning("unable to open cache disk(s): Cache Disabled\n");
+      return -1; // pointless, AFAICT this is ignored.
+    }
+  } else if (this->waitForCache() == 3 && static_cast<unsigned int>(gndisks) < 
theCacheStore.n_disks_in_config) {
+    CacheProcessor::initialized = CACHE_INIT_FAILED;
+    if (cb_after_init) cb_after_init();
+    Fatal("Cache initialization failed - only %d out of %d disks were valid 
and all were required.", gndisks, theCacheStore.n_disks_in_config);
   }
-  start_done = 1;
 
   return 0;
 }
@@ -756,7 +769,16 @@ CacheProcessor::diskInitialized()
     }
 
     if (bad_disks != 0) {
-      // create a new array
+      // Check if this is a fatal error
+      if (this->waitForCache() == 3 || (bad_disks == gndisks && 
this->waitForCache() == 2)) {
+        // This could be passed off to @c cacheInitialized (as with volume 
config problems) but I think
+        // the more specific error message here is worth the extra code.
+        CacheProcessor::initialized = CACHE_INIT_FAILED;
+        if (cb_after_init) cb_after_init();
+        Fatal("Cache initialization failed - only %d of %d disks were 
available.", gndisks, theCacheStore.n_disks_in_config);
+      }
+
+      // still good, create a new array to hold the valid disks.
       CacheDisk **p_good_disks;
       if ((gndisks - bad_disks) > 0)
         p_good_disks = (CacheDisk **)ats_malloc((gndisks - bad_disks) * 
sizeof(CacheDisk *));
@@ -1039,6 +1061,7 @@ CacheProcessor::cacheInitialized()
       Warning("cache unable to open any vols, disabled");
   }
   if (cache_init_ok) {
+
     // Initialize virtual cache
     CacheProcessor::initialized = CACHE_INITIALIZED;
     CacheProcessor::cache_ready = caches_ready;
@@ -1053,9 +1076,15 @@ CacheProcessor::cacheInitialized()
     CacheProcessor::initialized = CACHE_INIT_FAILED;
     Note("cache disabled");
   }
+
   // Fire callback to signal initialization finished.
   if (cb_after_init)
     cb_after_init();
+
+  // TS-3848
+  if (CACHE_INIT_FAILED == CacheProcessor::initialized && 
cacheProcessor.waitForCache() > 1) {
+    Fatal("Cache initialization failed with cache required, exiting.");
+  }
 }
 
 void
@@ -2078,6 +2107,7 @@ Cache::open_done()
   Action *register_ShowCacheInternal(Continuation * c, HTTPHdr * h);
   statPagesManager.register_http("cache", register_ShowCache);
   statPagesManager.register_http("cache-internal", register_ShowCacheInternal);
+
   if (total_good_nvol == 0) {
     ready = CACHE_INIT_FAILED;
     cacheProcessor.cacheInitialized();
@@ -2091,6 +2121,12 @@ Cache::open_done()
     ready = CACHE_INIT_FAILED;
   else
     ready = CACHE_INITIALIZED;
+
+  // TS-3848
+  if (ready == CACHE_INIT_FAILED && cacheProcessor.waitForCache() >= 2) {
+    Fatal("Failed to initialize cache host table");
+  }
+
   cacheProcessor.cacheInitialized();
 
   return 0;
@@ -3233,17 +3269,13 @@ ink_cache_init(ModuleVersion v)
 
   register_cache_stats(cache_rsb, "proxy.process.cache");
 
+  REC_ReadConfigInteger(cacheProcessor.wait_for_cache, 
"proxy.config.http.wait_for_cache");
+
   const char *err = NULL;
   if ((err = theCacheStore.read_config())) {
-    printf("%s  failed\n", err);
+    printf("Failed to read cache storage configuration - %s\n", err);
     exit(1);
   }
-
-  if (theCacheStore.n_disks == 0) {
-    ats_scoped_str 
path(RecConfigReadConfigPath("proxy.config.cache.storage_filename", 
"storage.config"));
-    Warning("no cache disks specified in %s: cache disabled\n", (const char 
*)path);
-    // exit(1);
-  }
 }
 
 //----------------------------------------------------------------------------

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26b566e7/iocore/cache/I_Cache.h
----------------------------------------------------------------------
diff --git a/iocore/cache/I_Cache.h b/iocore/cache/I_Cache.h
index ecd5db4..d6c5cea 100644
--- a/iocore/cache/I_Cache.h
+++ b/iocore/cache/I_Cache.h
@@ -65,7 +65,8 @@ typedef HTTPInfo CacheHTTPInfo;
 struct CacheProcessor : public Processor {
   CacheProcessor()
     : min_stripe_version(CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION),
-      max_stripe_version(CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION), 
cb_after_init(0)
+      max_stripe_version(CACHE_DB_MAJOR_VERSION, CACHE_DB_MINOR_VERSION), 
cb_after_init(0),
+      wait_for_cache(0)
   {
   }
 
@@ -140,13 +141,15 @@ struct CacheProcessor : public Processor {
       to specific the callback type and passed to the callback
       function.
   */
-  void set_after_init_callback(CALLBACK_FUNC cb);
+  void afterInitCallbackSet(CALLBACK_FUNC cb);
 
   // private members
   void diskInitialized();
 
   void cacheInitialized();
 
+  int waitForCache() const { return wait_for_cache; }
+
   static volatile uint32_t cache_ready;
   static volatile int initialized;
   static volatile int start_done;
@@ -160,10 +163,11 @@ struct CacheProcessor : public Processor {
   VersionNumber max_stripe_version;
 
   CALLBACK_FUNC cb_after_init;
+  int wait_for_cache;
 };
 
 inline void
-CacheProcessor::set_after_init_callback(CALLBACK_FUNC cb)
+CacheProcessor::afterInitCallbackSet(CALLBACK_FUNC cb)
 {
   cb_after_init = cb;
 }

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26b566e7/iocore/cache/I_Store.h
----------------------------------------------------------------------
diff --git a/iocore/cache/I_Store.h b/iocore/cache/I_Store.h
index 7fed57b..3a11bf3 100644
--- a/iocore/cache/I_Store.h
+++ b/iocore/cache/I_Store.h
@@ -240,6 +240,9 @@ struct Store {
   Store();
   ~Store();
 
+  // The number of disks/paths defined in storage.config
+  unsigned n_disks_in_config;
+  // The number of disks/paths we could actually read and parse.
   unsigned n_disks;
   Span **disk;
   //

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26b566e7/iocore/cache/Store.cc
----------------------------------------------------------------------
diff --git a/iocore/cache/Store.cc b/iocore/cache/Store.cc
index 6d41e15..c148f20 100644
--- a/iocore/cache/Store.cc
+++ b/iocore/cache/Store.cc
@@ -71,7 +71,8 @@ span_file_typename(mode_t st_mode)
 }
 
 Ptr<ProxyMutex> tmp_p;
-Store::Store() : n_disks(0), disk(NULL)
+  Store::Store()
+  : n_disks_in_config(0), n_disks(0), disk(NULL)
 {
 }
 
@@ -353,6 +354,7 @@ Store::read_config()
 
     // parse
     Debug("cache_init", "Store::read_config: \"%s\"", path);
+    ++n_disks_in_config;
 
     int64_t size = -1;
     int volume_num = -1;
@@ -381,6 +383,7 @@ Store::read_config()
     }
 
     char *pp = Layout::get()->relative(path);
+
     ns = new Span;
     Debug("cache_init", "Store::read_config - ns = new Span; 
ns->init(\"%s\",%" PRId64 "), forced volume=%d%s%s", pp, size,
           volume_num, seed ? " id=" : "", seed ? seed : "");

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26b566e7/mgmt/RecordsConfig.cc
----------------------------------------------------------------------
diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc
index aa3c886..ea28cfc 100644
--- a/mgmt/RecordsConfig.cc
+++ b/mgmt/RecordsConfig.cc
@@ -377,7 +377,7 @@ static const RecordElement RecordsConfig[] =
   ,
   {RECT_CONFIG, "proxy.config.http.server_ports", RECD_STRING, "8080", 
RECU_RESTART_TM, RR_NULL, RECC_NULL, NULL, RECA_NULL}
   ,
-  {RECT_CONFIG, "proxy.config.http.wait_for_cache", RECD_INT, "0", 
RECU_RESTART_TM, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}
+  {RECT_CONFIG, "proxy.config.http.wait_for_cache", RECD_INT, "0", 
RECU_RESTART_TM, RR_NULL, RECC_INT, "[0-3]", RECA_NULL}
   ,
   {RECT_CONFIG, "proxy.config.http.insert_request_via_str", RECD_INT, "1", 
RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
   ,
@@ -2005,7 +2005,8 @@ static const RecordElement RecordsConfig[] =
   //# Temporary and esoteric values.
   //#
   //###########
-  {RECT_CONFIG, "proxy.config.cache.http.compatibility.4-2-0-fixup", RECD_INT, 
"1", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL}
+  {RECT_CONFIG, "proxy.config.cache.http.compatibility.4-2-0-fixup", RECD_INT, 
"1", RECU_DYNAMIC, RR_NULL, RECC_NULL, NULL, RECA_NULL},
+
 };
 // clang-format on
 

http://git-wip-us.apache.org/repos/asf/trafficserver/blob/26b566e7/proxy/Main.cc
----------------------------------------------------------------------
diff --git a/proxy/Main.cc b/proxy/Main.cc
index 1ed6c54..b1e19e0 100644
--- a/proxy/Main.cc
+++ b/proxy/Main.cc
@@ -598,7 +598,7 @@ cmd_check_internal(char * /* cmd ATS_UNUSED */, bool fix = 
false)
   hd.reset();
 #endif
 
-  cacheProcessor.set_after_init_callback(&CB_cmd_cache_check);
+  cacheProcessor.afterInitCallbackSet(&CB_cmd_cache_check);
   if (cacheProcessor.start_internal(PROCESSOR_CHECK) < 0) {
     printf("\nbad cache configuration, %s failed\n", n);
     return CMD_FAILED;
@@ -663,7 +663,7 @@ cmd_clear(char *cmd)
   if (c_all || c_cache) {
     Note("Clearing Cache");
 
-    cacheProcessor.set_after_init_callback(&CB_cmd_cache_clear);
+    cacheProcessor.afterInitCallbackSet(&CB_cmd_cache_clear);
     if (cacheProcessor.start_internal(PROCESSOR_RECONFIGURE) < 0) {
       Note("unable to open Cache, CLEAR failed");
       return CMD_FAILED;
@@ -1699,7 +1699,7 @@ main(int /* argc ATS_UNUSED */, const char **argv)
 
     pmgmt->registerPluginCallbacks(global_config_cbs);
 
-    cacheProcessor.set_after_init_callback(&CB_After_Cache_Init);
+    cacheProcessor.afterInitCallbackSet(&CB_After_Cache_Init);
     cacheProcessor.start();
 
     // UDP net-threads are turned off by default.

Reply via email to