[jira] [Commented] (TS-4483) NetAccept & SSLNetAccept Optimize

[Oknet Xu (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4483?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15305195#comment-15305195
 ] 

Oknet Xu commented on TS-4483:
--

This optimize also fix a bug in SSLNetAccept::init_accept_per_thread(bool 
isTransparent)

{code}
PollDescriptor *pd = get_PollDescriptor(t);
if (ep.start(pd, this, EVENTIO_READ) < 0)   // ==> should be  
a->ep.start(pd, a, EVENTIO_READ)
  Debug("iocore_net", "error starting EventIO");
a->mutex = get_NetHandler(t)->mutex;
t->schedule_every(a, period, etype);
{code}

> NetAccept & SSLNetAccept Optimize
> -
>
> Key: TS-4483
> URL: https://issues.apache.org/jira/browse/TS-4483
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: Core
>Reporter: Oknet Xu
>Assignee: Oknet Xu
> Fix For: 7.0.0
>
>
> replace getEtype() with member etype.
> NetAccept has a member named 'etype' and it is set by upgradeEtype before 
> NetAccept running.
> Thus, we can replace getEtype() with member etype and make the SSLNetAccept 
> codes clearly.
> Should we remote the getEtype() method ? It is called by none after the patch.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304896#comment-15304896
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jacksontj commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64972616
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
+  void copy(std::vector *items);
+
+  typedef typename TSHashTable::iterator 
iteratortype;
+  typedef typename TSHashTable::self hashtype;
+  typedef typename TSHashTable::Location 
locationtype;
+  TSHashTable *getMap();
+
+  PtrMutex lock; // Lock
+
+private:
+  int maxSize;
+  int maxItems;
+
+  int size;
+  int items;
+
+  hashtype itemMap;
+};
+
+template  RefCountCachePartition::RefCountCachePartition(int 
maxSize, int maxItems)
+{
+  this->maxSize = maxSize;
+  this->maxItems = maxItems;
+  this->size = 0;
+  this->items = 0;
+
+  // Initialize lock
+  this->lock = new_ProxyMutex();
+}
+
+// TODO: error somehow if you asked for something too big!
+// Allocate space for an item `C` as well as `size` extra space.
+template 
+RefCountCacheItem *
+RefCountCachePartition::alloc(uint64_t key, int size)
+{
+  size += sizeof(C);
+  // Remove any colliding entries
+  this->del(key);
+
+  // Now allocat a sufficiently large iobuf to store our data
+  // as well as the intermediate layers we have
+  int allocSize = size + sizeof(RefCountCacheItem) + 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304876#comment-15304876
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jacksontj commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64971899
  
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -0,0 +1,178 @@
+#define _test_RefCountCache_cc
+
+#include 
+#include "P_RefCountCache.h"
+#include "I_EventSystem.h"
+#include "ts/I_Layout.h"
+#include "diags.i"
+
+struct ExampleStruct {
+  int idx;
+  int name_offset; // pointer addr to name
+
+  // Return the char* to the name (TODO: cleaner interface??)
+  char *
+  name()
+  {
+return (char *)this + this->name_offset;
+  }
+};
+
+void
+fillCache(RefCountCache *cache, int start, int end)
+{
+  // TODO: name per?
+  std::string name = "foobar";
+  int allocSize = name.size() + 1;
+
+  for (int i = start; i < end; i++) {
+ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)i, 
allocSize)->iobuf;
+
+tmp->idx = i;
+tmp->name_offset = sizeof(ExampleStruct);
+memcpy(tmp->name(), name.c_str(), name.size());
+// NULL terminate the string
+*(tmp->name() + name.size()) = '\0';
+
+// Print out the struct we put in there
+// printf("New ExampleStruct%d idx=%d name=%s allocSize=%d\n", i, 
tmp->idx, name.c_str(), allocSize);
+  }
+  printf("Loading complete! Cache now has %d items.\n\n", 
cache->numItems());
+}
+
+int
+verifyCache(RefCountCache *cache, int start, int end)
+{
+  // Re-query all the structs to make sure they are there and accurate
+  for (int i = start; i < end; i++) {
+Ptr ccitem = cache->get(i);
+if (ccitem.get() == NULL) {
+  continue;
+}
+ExampleStruct *tmp = (ExampleStruct *)ccitem->iobuf;
+if (tmp == NULL) {
+  // printf("ExampleStruct %d missing, skipping\n", i);
+  continue;
+}
+// printf("Get (%p) ExampleStruct%d idx=%d name=%s\n", tmp, i, 
tmp->idx, tmp->name());
+
+// Check that idx is correct
+if (tmp->idx != i) {
+  printf("IDX of ExampleStruct%d incorrect!\n", i);
+  return 1; // TODO: spin over all?
+}
+
+// check that the name is correct
+// if (strcmp(tmp->name, name.c_str())){
+//  printf("Name of ExampleStruct%d incorrect! %s %s\n", i, tmp->name, 
name.c_str());
+//  exit(1);
+//}
+  }
+  return 0;
+}
+
+// TODO: check that the memory was actually free-d better
+int
+testRefcounting()
+{
+  int ret = 0;
+
+  RefCountCache *cache = new 
RefCountCache(4);
+
+  // Set an item in the cache
+  ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)1)->iobuf;
+  tmp->idx = 1;
+
+  // Grab a pointer to item 1
+  Ptr ccitem = cache->get((uint64_t)1);
+
+  Ptr tmpAfter = cache->get((uint64_t)1);
+
+  // Delete a single item
+  cache->del(1);
+  // verify that it still isn't in there
+  ret |= cache->get(1).get() != NULL;
+  ret |= tmpAfter.get()->item()->idx != 1;
+
+  (ExampleStruct *)cache->alloc((uint64_t)2);
+  cache->del((uint64_t)2);
+
+  return ret;
+}
+
+int
+main()
--- End diff --

Actually, what I'm doing here is what is supported in the ATS source tree 
for unit tests-- if you just go in that directory and run `make check` it has 
all pretty output etc.

That being said, I do have some (and will have more) regression tests, but 
I think the standalone tests for the cache are just as valuable.


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304860#comment-15304860
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jacksontj commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64970971
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
--- End diff --

since these are variable size-- we can't simply multiply the count to get 
the actual used size, so I think we'll want to expose both.


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304857#comment-15304857
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64970668
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
--- End diff --

The number of items you have is ``size``, the number of items you have 
current storage for is ``capacity``.


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304856#comment-15304856
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jacksontj commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64970519
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
--- End diff --

There are 2 numbers we need to expose-- number of items and size (memory 
wise). So, do you think `size` and `count` are good? 


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 

Build failed in Jenkins: clang-analyzer #2347

[jenkins]

See 

Changes:

[James Peach] Fix ssl_cert_loader libtool linking.

--
[...truncated 2820 lines...]
reading sources... [ 53%] developer-guide/api/functions/TSMimeHdrFieldGet.en
reading sources... [ 53%] 
developer-guide/api/functions/TSMimeHdrFieldLengthGet.en
reading sources... [ 53%] developer-guide/api/functions/TSMimeHdrFieldNameGet.en
reading sources... [ 53%] developer-guide/api/functions/TSMimeHdrFieldNameSet.en
reading sources... [ 54%] developer-guide/api/functions/TSMimeHdrFieldNext.en
reading sources... [ 54%] developer-guide/api/functions/TSMimeHdrFieldNextDup.en
reading sources... [ 54%] developer-guide/api/functions/TSMimeHdrFieldRemove.en
reading sources... [ 54%] 
developer-guide/api/functions/TSMimeHdrFieldValueAppend.en
reading sources... [ 54%] 
developer-guide/api/functions/TSMimeHdrFieldValueDateInsert.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueDateSet.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueIntSet.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueStringGet.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueStringInsert.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValueStringSet.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValueUintInsert.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValueUintSet.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValuesClear.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValuesCount.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrFieldsClear.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrFieldsCount.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrLengthGet.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrParse.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeHdrPrint.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeParserClear.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeParserCreate.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeParserDestroy.en
reading sources... [ 58%] developer-guide/api/functions/TSMutexCreate.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexDestroy.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexLock.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexLockTry.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexUnlock.en
reading sources... [ 60%] developer-guide/api/functions/TSNetAccept.en
reading sources... [ 60%] 
developer-guide/api/functions/TSNetAcceptNamedProtocol.en
reading sources... [ 60%] developer-guide/api/functions/TSNetConnect.en
reading sources... [ 60%] developer-guide/api/functions/TSPluginInit.en
reading sources... [ 60%] developer-guide/api/functions/TSRemap.en
reading sources... [ 61%] developer-guide/api/functions/TSSslContextFindBy.en
reading sources... [ 61%] 
developer-guide/api/functions/TSSslServerContextCreate.en
reading sources... [ 61%] developer-guide/api/functions/TSTextLogObjectCreate.en
reading sources... [ 61%] developer-guide/api/functions/TSThreadCreate.en
reading sources... [ 62%] developer-guide/api/functions/TSThreadDestroy.en
reading sources... [ 62%] developer-guide/api/functions/TSThreadInit.en
reading sources... [ 62%] developer-guide/api/functions/TSThreadSelf.en
reading sources... [ 62%] 
developer-guide/api/functions/TSTrafficServerVersionGet.en
reading sources... [ 63%] developer-guide/api/functions/TSTransformCreate.en
reading sources... [ 63%] 
developer-guide/api/functions/TSTransformOutputVConnGet.en
reading sources... [ 63%] developer-guide/api/functions/TSTypes.en
reading sources... [ 63%] developer-guide/api/functions/TSUrlCreate.en
reading sources... [ 63%] developer-guide/api/functions/TSUrlDestroy.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlFtpTypeGet.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlFtpTypeSet.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlHostGet.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlHostSet.en
reading sources... [ 65%] developer-guide/api/functions/TSUrlPercentEncode.en
reading sources... [ 65%] developer-guide/api/functions/TSUrlStringGet.en
reading sources... [ 65%] developer-guide/api/functions/TSVConnAbort.en
reading sources... [ 65%] 
developer-guide/api/functions/TSVConnCacheObjectSizeGet.en
reading sources... [ 65%] developer-guide/api/functions/TSVConnClose.en
reading sources... [ 66%] developer-guide/api/functions/TSVConnClosedGet.en
reading sources... [ 66%] developer-guide/api/functions/TSVConnFdCreate.en
reading sources... [ 66%] developer-guide/api/functions/TSVConnIsSsl.en
reading 

[jira] [Comment Edited] (TS-4309) Reduced SSL upload/download speed after event loop change (TS-4260)

[Leif Hedstrom (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4309?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304579#comment-15304579
 ] 

Leif Hedstrom edited comment on TS-4309 at 5/27/16 7:07 PM:


Ah, never mind, this is 7.0.0 specific only right ?


was (Author: zwoop):
Is this a 6.2.0 candidate?

> Reduced SSL upload/download speed after event loop change (TS-4260)
> ---
>
> Key: TS-4309
> URL: https://issues.apache.org/jira/browse/TS-4309
> Project: Traffic Server
>  Issue Type: Bug
>  Components: Core
>Reporter: Susan Hinrichs
>Assignee: Susan Hinrichs
> Fix For: 7.0.0
>
>
> With the Eventloop changes of TS-4260 there are fewer spurious event loop 
> kicks.  This reveals stalls during upload and download on a lightly loaded 
> system.  Tidying up the net read/write loops and reactivating a never 
> triggered signal fixed the problem.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4309) Reduced SSL upload/download speed after event loop change (TS-4260)

[Leif Hedstrom (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4309?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304579#comment-15304579
 ] 

Leif Hedstrom commented on TS-4309:
---

Is this a 6.2.0 candidate?

> Reduced SSL upload/download speed after event loop change (TS-4260)
> ---
>
> Key: TS-4309
> URL: https://issues.apache.org/jira/browse/TS-4309
> Project: Traffic Server
>  Issue Type: Bug
>  Components: Core
>Reporter: Susan Hinrichs
>Assignee: Susan Hinrichs
> Fix For: 7.0.0
>
>
> With the Eventloop changes of TS-4260 there are fewer spurious event loop 
> kicks.  This reveals stalls during upload and download on a lightly loaded 
> system.  Tidying up the net read/write loops and reactivating a never 
> triggered signal fixed the problem.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304334#comment-15304334
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64935087
  
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -0,0 +1,178 @@
+#define _test_RefCountCache_cc
+
+#include 
+#include "P_RefCountCache.h"
+#include "I_EventSystem.h"
+#include "ts/I_Layout.h"
+#include "diags.i"
+
+struct ExampleStruct {
+  int idx;
+  int name_offset; // pointer addr to name
+
+  // Return the char* to the name (TODO: cleaner interface??)
+  char *
+  name()
+  {
+return (char *)this + this->name_offset;
+  }
+};
+
+void
+fillCache(RefCountCache *cache, int start, int end)
+{
+  // TODO: name per?
+  std::string name = "foobar";
+  int allocSize = name.size() + 1;
+
+  for (int i = start; i < end; i++) {
+ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)i, 
allocSize)->iobuf;
+
+tmp->idx = i;
+tmp->name_offset = sizeof(ExampleStruct);
+memcpy(tmp->name(), name.c_str(), name.size());
+// NULL terminate the string
+*(tmp->name() + name.size()) = '\0';
+
+// Print out the struct we put in there
+// printf("New ExampleStruct%d idx=%d name=%s allocSize=%d\n", i, 
tmp->idx, name.c_str(), allocSize);
+  }
+  printf("Loading complete! Cache now has %d items.\n\n", 
cache->numItems());
+}
+
+int
+verifyCache(RefCountCache *cache, int start, int end)
+{
+  // Re-query all the structs to make sure they are there and accurate
+  for (int i = start; i < end; i++) {
+Ptr ccitem = cache->get(i);
+if (ccitem.get() == NULL) {
+  continue;
+}
+ExampleStruct *tmp = (ExampleStruct *)ccitem->iobuf;
+if (tmp == NULL) {
+  // printf("ExampleStruct %d missing, skipping\n", i);
+  continue;
+}
+// printf("Get (%p) ExampleStruct%d idx=%d name=%s\n", tmp, i, 
tmp->idx, tmp->name());
+
+// Check that idx is correct
+if (tmp->idx != i) {
+  printf("IDX of ExampleStruct%d incorrect!\n", i);
+  return 1; // TODO: spin over all?
+}
+
+// check that the name is correct
+// if (strcmp(tmp->name, name.c_str())){
+//  printf("Name of ExampleStruct%d incorrect! %s %s\n", i, tmp->name, 
name.c_str());
+//  exit(1);
+//}
+  }
+  return 0;
+}
+
+// TODO: check that the memory was actually free-d better
+int
+testRefcounting()
+{
+  int ret = 0;
+
+  RefCountCache *cache = new 
RefCountCache(4);
+
+  // Set an item in the cache
+  ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)1)->iobuf;
+  tmp->idx = 1;
+
+  // Grab a pointer to item 1
+  Ptr ccitem = cache->get((uint64_t)1);
+
+  Ptr tmpAfter = cache->get((uint64_t)1);
+
+  // Delete a single item
+  cache->del(1);
+  // verify that it still isn't in there
+  ret |= cache->get(1).get() != NULL;
+  ret |= tmpAfter.get()->item()->idx != 1;
+
+  (ExampleStruct *)cache->alloc((uint64_t)2);
+  cache->del((uint64_t)2);
+
+  return ret;
+}
+
+int
+main()
--- End diff --

Because regressions gives you a way to run the tests, determine failure and 
format output already. Let's not reinvent that or do things differently 
everywhere. It's really easy to use the basic framework for everything you are 
doing by hand here.

eg. https://github.com/apache/trafficserver/blob/master/lib/ts/test_Ptr.cc


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around 

Build failed in Jenkins: clang-analyzer #2346

[jenkins]

See 

Changes:

[shinrich] TS-4309: Simplify read/write loops to address upload/download speed

--
[...truncated 2820 lines...]
reading sources... [ 53%] developer-guide/api/functions/TSMimeHdrFieldGet.en
reading sources... [ 53%] 
developer-guide/api/functions/TSMimeHdrFieldLengthGet.en
reading sources... [ 53%] developer-guide/api/functions/TSMimeHdrFieldNameGet.en
reading sources... [ 53%] developer-guide/api/functions/TSMimeHdrFieldNameSet.en
reading sources... [ 54%] developer-guide/api/functions/TSMimeHdrFieldNext.en
reading sources... [ 54%] developer-guide/api/functions/TSMimeHdrFieldNextDup.en
reading sources... [ 54%] developer-guide/api/functions/TSMimeHdrFieldRemove.en
reading sources... [ 54%] 
developer-guide/api/functions/TSMimeHdrFieldValueAppend.en
reading sources... [ 54%] 
developer-guide/api/functions/TSMimeHdrFieldValueDateInsert.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueDateSet.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueIntSet.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueStringGet.en
reading sources... [ 55%] 
developer-guide/api/functions/TSMimeHdrFieldValueStringInsert.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValueStringSet.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValueUintInsert.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValueUintSet.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValuesClear.en
reading sources... [ 56%] 
developer-guide/api/functions/TSMimeHdrFieldValuesCount.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrFieldsClear.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrFieldsCount.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrLengthGet.en
reading sources... [ 57%] developer-guide/api/functions/TSMimeHdrParse.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeHdrPrint.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeParserClear.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeParserCreate.en
reading sources... [ 58%] developer-guide/api/functions/TSMimeParserDestroy.en
reading sources... [ 58%] developer-guide/api/functions/TSMutexCreate.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexDestroy.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexLock.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexLockTry.en
reading sources... [ 59%] developer-guide/api/functions/TSMutexUnlock.en
reading sources... [ 60%] developer-guide/api/functions/TSNetAccept.en
reading sources... [ 60%] 
developer-guide/api/functions/TSNetAcceptNamedProtocol.en
reading sources... [ 60%] developer-guide/api/functions/TSNetConnect.en
reading sources... [ 60%] developer-guide/api/functions/TSPluginInit.en
reading sources... [ 60%] developer-guide/api/functions/TSRemap.en
reading sources... [ 61%] developer-guide/api/functions/TSSslContextFindBy.en
reading sources... [ 61%] 
developer-guide/api/functions/TSSslServerContextCreate.en
reading sources... [ 61%] developer-guide/api/functions/TSTextLogObjectCreate.en
reading sources... [ 61%] developer-guide/api/functions/TSThreadCreate.en
reading sources... [ 62%] developer-guide/api/functions/TSThreadDestroy.en
reading sources... [ 62%] developer-guide/api/functions/TSThreadInit.en
reading sources... [ 62%] developer-guide/api/functions/TSThreadSelf.en
reading sources... [ 62%] 
developer-guide/api/functions/TSTrafficServerVersionGet.en
reading sources... [ 63%] developer-guide/api/functions/TSTransformCreate.en
reading sources... [ 63%] 
developer-guide/api/functions/TSTransformOutputVConnGet.en
reading sources... [ 63%] developer-guide/api/functions/TSTypes.en
reading sources... [ 63%] developer-guide/api/functions/TSUrlCreate.en
reading sources... [ 63%] developer-guide/api/functions/TSUrlDestroy.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlFtpTypeGet.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlFtpTypeSet.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlHostGet.en
reading sources... [ 64%] developer-guide/api/functions/TSUrlHostSet.en
reading sources... [ 65%] developer-guide/api/functions/TSUrlPercentEncode.en
reading sources... [ 65%] developer-guide/api/functions/TSUrlStringGet.en
reading sources... [ 65%] developer-guide/api/functions/TSVConnAbort.en
reading sources... [ 65%] 
developer-guide/api/functions/TSVConnCacheObjectSizeGet.en
reading sources... [ 65%] developer-guide/api/functions/TSVConnClose.en
reading sources... [ 66%] developer-guide/api/functions/TSVConnClosedGet.en
reading sources... [ 66%] developer-guide/api/functions/TSVConnFdCreate.en
reading sources... [ 66%] 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304286#comment-15304286
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64932061
  
--- Diff: iocore/hostdb/RefCountCache.cc ---
@@ -0,0 +1,145 @@
+// The goal here is to create a proof-of-concept cache struct which will 
cache an item
+// for at least N seconds (configurable) and alloc a given amount of space 
for the given
+// struct-- this way all of the dynamic pieces (strings, arrays, etc.) can 
be allocated
+// within the same block.
+
+#include "ts/ink_platform.h"
+#include "ts/I_Layout.h"
+#include "P_HostDB.h"
+#include "P_RefCountCache.h"
+#include "P_EventSystem.h" // FIXME: need to have this in I_* header files.
+#include "ts/ink_file.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// TODO: remove? Config?
+// static const int MC_SYNC_MIN_PAUSE_TIME = HRTIME_MSECONDS(200); // 
Pause for at least 200ms
+
+void
+RefCountCacheBase::sync_partitions(Continuation *cont)
+{
+  eventProcessor.schedule_imm(new RefCountCacheSync(cont, this), ET_CALL);
+}
+
+int
+RefCountCacheSync::copyPartition(int event, Event *e)
+{
+  (void)event;
+  if (partition >= cc->partition_count()) {
+// TODO: check return
+this->finalizeSync();
+cont->handleEvent(REFCOUNT_CACHE_EVENT_SYNC, 0);
+Debug("multicache", "RefCountCacheSync done");
+delete this;
+return EVENT_DONE;
+  }
+  Debug("hostdb", "sync partition=%d/%d", partition, 
cc->partition_count());
+  // copy the partition into our buffer, then we'll let `pauseEvent` write 
it out
+  this->partitionItems.reserve(cc->partition_itemcount(partition));
+  cc->copy_partition(partition, >partitionItems);
+  partition++;
+  SET_HANDLER((MCacheSyncHandler)::writePartition);
+  mutex = e->ethread->mutex;
+  e->schedule_imm();
+
+  return EVENT_CONT;
+}
+
+int
+RefCountCacheSync::writePartition(int event, Event *e)
+{
+  // write the partition to disk
+  // for item in this->partitionItems
+  // write to disk with headers per item
+  for (std::vector::iterator it = 
this->partitionItems.begin(); it != this->partitionItems.end();
+   ++it) {
+// TODO: figure out a cleaner way, dereferencing and taking the ptr 
seems terrible!
+this->outfile.write((char *)&*it->get(), sizeof(*it->get()));
+// write the contents
+this->outfile.write(it->get()->iobuf, it->get()->size);
+  }
+
+  // Clear partition-- for the next user
+  this->partitionItems.clear();
+
+  SET_HANDLER((MCacheSyncHandler)::pauseEvent);
+
+  // TODO: no sleep between partitions?
+  // TODO: keep track of how long it took and sleep if we were faster than 
expected
+  // TODO: figure out how to get access to hostdb_sync_frequency (probaby 
should be passed in instead of this global magic)
+  // e->schedule_in(MAX(MC_SYNC_MIN_PAUSE_TIME, 
HRTIME_SECONDS(hostdb_sync_frequency - 5) / MULTI_CACHE_PARTITIONS));
+  // e->schedule_in(MC_SYNC_MIN_PAUSE_TIME);
+  e->schedule_imm();
+  return EVENT_CONT;
+}
+
+int
+RefCountCacheSync::pauseEvent(int event, Event *e)
+{
+  (void)event;
+  (void)e;
+
+  // Schedule up the next partition
+  if (partition < cc->partition_count())
+mutex = cc->lock_for_partition(partition);
+  else
+mutex = cont->mutex;
+  SET_HANDLER((MCacheSyncHandler)::copyPartition);
+  e->schedule_imm();
+  return EVENT_CONT;
+}
+
+// Open the tmp file, etc.
+int
+RefCountCacheSync::initializeStorage(int event, Event *e)
+{
+  this->filename = this->cc->get_filepath();
+  this->tmp_filename = this->filename + ".syncing";
+
+  this->outfile.open(this->tmp_filename.c_str(), std::ios::out | 
std::ios::binary);
+
+  if (!this->outfile.is_open()) {
+Warning("Unable to create temporyary file %s, unable to persist 
hostdb\n", this->tmp_filename.c_str());
+delete this;
+return EVENT_DONE;
+  }
+
+  // Write out the header
+  this->outfile.write((char *)this->cc->get_header(), 
sizeof(RefCountCacheHeader));
+
+  SET_HANDLER((MCacheSyncHandler)::copyPartition);
+  e->schedule_imm();
+  return EVENT_CONT;
+}
+// do the final mv and close of file handle
+int
+RefCountCacheSync::finalizeSync()
+{
+  // move the file
+  int ret;
+  ret = rename(this->tmp_filename.c_str(), this->filename.c_str());
+
+  if (ret != 0) {

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304284#comment-15304284
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64931947
  
--- Diff: iocore/hostdb/RefCountCache.cc ---
@@ -0,0 +1,145 @@
+// The goal here is to create a proof-of-concept cache struct which will 
cache an item
+// for at least N seconds (configurable) and alloc a given amount of space 
for the given
+// struct-- this way all of the dynamic pieces (strings, arrays, etc.) can 
be allocated
+// within the same block.
+
+#include "ts/ink_platform.h"
+#include "ts/I_Layout.h"
+#include "P_HostDB.h"
+#include "P_RefCountCache.h"
+#include "P_EventSystem.h" // FIXME: need to have this in I_* header files.
+#include "ts/ink_file.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// TODO: remove? Config?
+// static const int MC_SYNC_MIN_PAUSE_TIME = HRTIME_MSECONDS(200); // 
Pause for at least 200ms
+
+void
+RefCountCacheBase::sync_partitions(Continuation *cont)
+{
+  eventProcessor.schedule_imm(new RefCountCacheSync(cont, this), ET_CALL);
+}
+
+int
+RefCountCacheSync::copyPartition(int event, Event *e)
+{
+  (void)event;
+  if (partition >= cc->partition_count()) {
+// TODO: check return
+this->finalizeSync();
+cont->handleEvent(REFCOUNT_CACHE_EVENT_SYNC, 0);
+Debug("multicache", "RefCountCacheSync done");
+delete this;
+return EVENT_DONE;
+  }
+  Debug("hostdb", "sync partition=%d/%d", partition, 
cc->partition_count());
+  // copy the partition into our buffer, then we'll let `pauseEvent` write 
it out
+  this->partitionItems.reserve(cc->partition_itemcount(partition));
+  cc->copy_partition(partition, >partitionItems);
+  partition++;
+  SET_HANDLER((MCacheSyncHandler)::writePartition);
+  mutex = e->ethread->mutex;
+  e->schedule_imm();
+
+  return EVENT_CONT;
+}
+
+int
+RefCountCacheSync::writePartition(int event, Event *e)
+{
+  // write the partition to disk
+  // for item in this->partitionItems
+  // write to disk with headers per item
+  for (std::vector::iterator it = 
this->partitionItems.begin(); it != this->partitionItems.end();
+   ++it) {
+// TODO: figure out a cleaner way, dereferencing and taking the ptr 
seems terrible!
+this->outfile.write((char *)&*it->get(), sizeof(*it->get()));
+// write the contents
+this->outfile.write(it->get()->iobuf, it->get()->size);
--- End diff --

Check whether the data was written?


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two types of storage (static-- blocks and dynamic-- 
> alloc). The static side of the cache can hold N HostDBInfo structs (the 
> results of DNS queries). The dynamic side is used to store the round robin 
> records and various strings associated with the record. The size of this 
> dynamic space is defined as (N x [estimated_heap_bytes_per_entry. The basic 
> problem we are running into is that we are putting too much preassure on the 
> dynamic heap-- such that the heap is getting re-used while people still have 
> references to items in that space.
> So, I've actually been working on re-writing MultiCache to allocate the 
> entire required block at once (so we don't have this problem where the parent 
> exists but not the children), but I'm not certain if we want such a change to 
> go into the 6.x branch (I'm willing to discuss if we want). If we aren't 
> comfortable with such a large change I 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304283#comment-15304283
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64931897
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
+  void copy(std::vector *items);
+
+  typedef typename TSHashTable::iterator 
iteratortype;
+  typedef typename TSHashTable::self hashtype;
+  typedef typename TSHashTable::Location 
locationtype;
+  TSHashTable *getMap();
+
+  PtrMutex lock; // Lock
+
+private:
+  int maxSize;
+  int maxItems;
+
+  int size;
+  int items;
+
+  hashtype itemMap;
+};
+
+template  RefCountCachePartition::RefCountCachePartition(int 
maxSize, int maxItems)
+{
+  this->maxSize = maxSize;
+  this->maxItems = maxItems;
+  this->size = 0;
+  this->items = 0;
+
+  // Initialize lock
+  this->lock = new_ProxyMutex();
+}
+
+// TODO: error somehow if you asked for something too big!
+// Allocate space for an item `C` as well as `size` extra space.
+template 
+RefCountCacheItem *
+RefCountCachePartition::alloc(uint64_t key, int size)
+{
+  size += sizeof(C);
+  // Remove any colliding entries
+  this->del(key);
+
+  // Now allocat a sufficiently large iobuf to store our data
+  // as well as the intermediate layers we have
+  int allocSize = size + sizeof(RefCountCacheItem) + 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304279#comment-15304279
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64931485
  
--- Diff: iocore/hostdb/RefCountCache.cc ---
@@ -0,0 +1,145 @@
+// The goal here is to create a proof-of-concept cache struct which will 
cache an item
+// for at least N seconds (configurable) and alloc a given amount of space 
for the given
+// struct-- this way all of the dynamic pieces (strings, arrays, etc.) can 
be allocated
+// within the same block.
+
+#include "ts/ink_platform.h"
+#include "ts/I_Layout.h"
+#include "P_HostDB.h"
+#include "P_RefCountCache.h"
+#include "P_EventSystem.h" // FIXME: need to have this in I_* header files.
+#include "ts/ink_file.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+// TODO: remove? Config?
+// static const int MC_SYNC_MIN_PAUSE_TIME = HRTIME_MSECONDS(200); // 
Pause for at least 200ms
+
+void
+RefCountCacheBase::sync_partitions(Continuation *cont)
+{
+  eventProcessor.schedule_imm(new RefCountCacheSync(cont, this), ET_CALL);
+}
+
+int
+RefCountCacheSync::copyPartition(int event, Event *e)
+{
+  (void)event;
+  if (partition >= cc->partition_count()) {
+// TODO: check return
+this->finalizeSync();
+cont->handleEvent(REFCOUNT_CACHE_EVENT_SYNC, 0);
+Debug("multicache", "RefCountCacheSync done");
+delete this;
+return EVENT_DONE;
+  }
+  Debug("hostdb", "sync partition=%d/%d", partition, 
cc->partition_count());
+  // copy the partition into our buffer, then we'll let `pauseEvent` write 
it out
+  this->partitionItems.reserve(cc->partition_itemcount(partition));
+  cc->copy_partition(partition, >partitionItems);
+  partition++;
+  SET_HANDLER((MCacheSyncHandler)::writePartition);
+  mutex = e->ethread->mutex;
+  e->schedule_imm();
+
+  return EVENT_CONT;
+}
+
+int
+RefCountCacheSync::writePartition(int event, Event *e)
+{
+  // write the partition to disk
+  // for item in this->partitionItems
+  // write to disk with headers per item
+  for (std::vector::iterator it = 
this->partitionItems.begin(); it != this->partitionItems.end();
+   ++it) {
+// TODO: figure out a cleaner way, dereferencing and taking the ptr 
seems terrible!
+this->outfile.write((char *)&*it->get(), sizeof(*it->get()));
+// write the contents
+this->outfile.write(it->get()->iobuf, it->get()->size);
+  }
+
+  // Clear partition-- for the next user
+  this->partitionItems.clear();
+
+  SET_HANDLER((MCacheSyncHandler)::pauseEvent);
+
+  // TODO: no sleep between partitions?
+  // TODO: keep track of how long it took and sleep if we were faster than 
expected
+  // TODO: figure out how to get access to hostdb_sync_frequency (probaby 
should be passed in instead of this global magic)
+  // e->schedule_in(MAX(MC_SYNC_MIN_PAUSE_TIME, 
HRTIME_SECONDS(hostdb_sync_frequency - 5) / MULTI_CACHE_PARTITIONS));
+  // e->schedule_in(MC_SYNC_MIN_PAUSE_TIME);
+  e->schedule_imm();
+  return EVENT_CONT;
+}
+
+int
+RefCountCacheSync::pauseEvent(int event, Event *e)
+{
+  (void)event;
+  (void)e;
+
+  // Schedule up the next partition
+  if (partition < cc->partition_count())
+mutex = cc->lock_for_partition(partition);
+  else
+mutex = cont->mutex;
+  SET_HANDLER((MCacheSyncHandler)::copyPartition);
+  e->schedule_imm();
+  return EVENT_CONT;
+}
+
+// Open the tmp file, etc.
+int
+RefCountCacheSync::initializeStorage(int event, Event *e)
+{
+  this->filename = this->cc->get_filepath();
+  this->tmp_filename = this->filename + ".syncing";
+
+  this->outfile.open(this->tmp_filename.c_str(), std::ios::out | 
std::ios::binary);
+
+  if (!this->outfile.is_open()) {
+Warning("Unable to create temporyary file %s, unable to persist 
hostdb\n", this->tmp_filename.c_str());
--- End diff --

* temporary


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304278#comment-15304278
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jacksontj commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64931455
  
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -0,0 +1,178 @@
+#define _test_RefCountCache_cc
+
+#include 
+#include "P_RefCountCache.h"
+#include "I_EventSystem.h"
+#include "ts/I_Layout.h"
+#include "diags.i"
+
+struct ExampleStruct {
+  int idx;
+  int name_offset; // pointer addr to name
+
+  // Return the char* to the name (TODO: cleaner interface??)
+  char *
+  name()
+  {
+return (char *)this + this->name_offset;
+  }
+};
+
+void
+fillCache(RefCountCache *cache, int start, int end)
+{
+  // TODO: name per?
+  std::string name = "foobar";
+  int allocSize = name.size() + 1;
+
+  for (int i = start; i < end; i++) {
+ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)i, 
allocSize)->iobuf;
+
+tmp->idx = i;
+tmp->name_offset = sizeof(ExampleStruct);
+memcpy(tmp->name(), name.c_str(), name.size());
+// NULL terminate the string
+*(tmp->name() + name.size()) = '\0';
+
+// Print out the struct we put in there
+// printf("New ExampleStruct%d idx=%d name=%s allocSize=%d\n", i, 
tmp->idx, name.c_str(), allocSize);
+  }
+  printf("Loading complete! Cache now has %d items.\n\n", 
cache->numItems());
+}
+
+int
+verifyCache(RefCountCache *cache, int start, int end)
+{
+  // Re-query all the structs to make sure they are there and accurate
+  for (int i = start; i < end; i++) {
+Ptr ccitem = cache->get(i);
+if (ccitem.get() == NULL) {
+  continue;
+}
+ExampleStruct *tmp = (ExampleStruct *)ccitem->iobuf;
+if (tmp == NULL) {
+  // printf("ExampleStruct %d missing, skipping\n", i);
+  continue;
+}
+// printf("Get (%p) ExampleStruct%d idx=%d name=%s\n", tmp, i, 
tmp->idx, tmp->name());
+
+// Check that idx is correct
+if (tmp->idx != i) {
+  printf("IDX of ExampleStruct%d incorrect!\n", i);
+  return 1; // TODO: spin over all?
+}
+
+// check that the name is correct
+// if (strcmp(tmp->name, name.c_str())){
+//  printf("Name of ExampleStruct%d incorrect! %s %s\n", i, tmp->name, 
name.c_str());
+//  exit(1);
+//}
+  }
+  return 0;
+}
+
+// TODO: check that the memory was actually free-d better
+int
+testRefcounting()
+{
+  int ret = 0;
+
+  RefCountCache *cache = new 
RefCountCache(4);
+
+  // Set an item in the cache
+  ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)1)->iobuf;
+  tmp->idx = 1;
+
+  // Grab a pointer to item 1
+  Ptr ccitem = cache->get((uint64_t)1);
+
+  Ptr tmpAfter = cache->get((uint64_t)1);
+
+  // Delete a single item
+  cache->del(1);
+  // verify that it still isn't in there
+  ret |= cache->get(1).get() != NULL;
+  ret |= tmpAfter.get()->item()->idx != 1;
+
+  (ExampleStruct *)cache->alloc((uint64_t)2);
+  cache->del((uint64_t)2);
+
+  return ret;
+}
+
+int
+main()
--- End diff --

Why? These are tests on the generic cache (not even using ATS` real 
structs), to me this seems to make more sense here-- since there are actual 
regression tests (albeit not enough-- TODO is marked above).


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304274#comment-15304274
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64931096
  
--- Diff: iocore/hostdb/test_RefCountCache.cc ---
@@ -0,0 +1,178 @@
+#define _test_RefCountCache_cc
+
+#include 
+#include "P_RefCountCache.h"
+#include "I_EventSystem.h"
+#include "ts/I_Layout.h"
+#include "diags.i"
+
+struct ExampleStruct {
+  int idx;
+  int name_offset; // pointer addr to name
+
+  // Return the char* to the name (TODO: cleaner interface??)
+  char *
+  name()
+  {
+return (char *)this + this->name_offset;
+  }
+};
+
+void
+fillCache(RefCountCache *cache, int start, int end)
+{
+  // TODO: name per?
+  std::string name = "foobar";
+  int allocSize = name.size() + 1;
+
+  for (int i = start; i < end; i++) {
+ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)i, 
allocSize)->iobuf;
+
+tmp->idx = i;
+tmp->name_offset = sizeof(ExampleStruct);
+memcpy(tmp->name(), name.c_str(), name.size());
+// NULL terminate the string
+*(tmp->name() + name.size()) = '\0';
+
+// Print out the struct we put in there
+// printf("New ExampleStruct%d idx=%d name=%s allocSize=%d\n", i, 
tmp->idx, name.c_str(), allocSize);
+  }
+  printf("Loading complete! Cache now has %d items.\n\n", 
cache->numItems());
+}
+
+int
+verifyCache(RefCountCache *cache, int start, int end)
+{
+  // Re-query all the structs to make sure they are there and accurate
+  for (int i = start; i < end; i++) {
+Ptr ccitem = cache->get(i);
+if (ccitem.get() == NULL) {
+  continue;
+}
+ExampleStruct *tmp = (ExampleStruct *)ccitem->iobuf;
+if (tmp == NULL) {
+  // printf("ExampleStruct %d missing, skipping\n", i);
+  continue;
+}
+// printf("Get (%p) ExampleStruct%d idx=%d name=%s\n", tmp, i, 
tmp->idx, tmp->name());
+
+// Check that idx is correct
+if (tmp->idx != i) {
+  printf("IDX of ExampleStruct%d incorrect!\n", i);
+  return 1; // TODO: spin over all?
+}
+
+// check that the name is correct
+// if (strcmp(tmp->name, name.c_str())){
+//  printf("Name of ExampleStruct%d incorrect! %s %s\n", i, tmp->name, 
name.c_str());
+//  exit(1);
+//}
+  }
+  return 0;
+}
+
+// TODO: check that the memory was actually free-d better
+int
+testRefcounting()
+{
+  int ret = 0;
+
+  RefCountCache *cache = new 
RefCountCache(4);
+
+  // Set an item in the cache
+  ExampleStruct *tmp = (ExampleStruct *)cache->alloc((uint64_t)1)->iobuf;
+  tmp->idx = 1;
+
+  // Grab a pointer to item 1
+  Ptr ccitem = cache->get((uint64_t)1);
+
+  Ptr tmpAfter = cache->get((uint64_t)1);
+
+  // Delete a single item
+  cache->del(1);
+  // verify that it still isn't in there
+  ret |= cache->get(1).get() != NULL;
+  ret |= tmpAfter.get()->item()->idx != 1;
+
+  (ExampleStruct *)cache->alloc((uint64_t)2);
+  cache->del((uint64_t)2);
+
+  return ret;
+}
+
+int
+main()
--- End diff --

These tests should use the Regression harness.


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two types of storage (static-- blocks and dynamic-- 
> alloc). The static side of the cache can hold N HostDBInfo structs (the 
> results of DNS queries). The dynamic 

[jira] [Commented] (TS-4309) Reduced SSL upload/download speed after event loop change (TS-4260)

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4309?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304271#comment-15304271
 ] 

ASF GitHub Bot commented on TS-4309:


Github user asfgit closed the pull request at:

https://github.com/apache/trafficserver/pull/629


> Reduced SSL upload/download speed after event loop change (TS-4260)
> ---
>
> Key: TS-4309
> URL: https://issues.apache.org/jira/browse/TS-4309
> Project: Traffic Server
>  Issue Type: Bug
>  Components: Core
>Reporter: Susan Hinrichs
>Assignee: Susan Hinrichs
> Fix For: 7.0.0
>
>
> With the Eventloop changes of TS-4260 there are fewer spurious event loop 
> kicks.  This reveals stalls during upload and download on a lightly loaded 
> system.  Tidying up the net read/write loops and reactivating a never 
> triggered signal fixed the problem.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4309) Reduced SSL upload/download speed after event loop change (TS-4260)

[ASF subversion and git services (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4309?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304270#comment-15304270
 ] 

ASF subversion and git services commented on TS-4309:
-

Commit a647412f20f3a8858c3fa4fc1cc4e9c623b68e92 in trafficserver's branch 
refs/heads/master from [~shinrich]
[ https://git-wip-us.apache.org/repos/asf?p=trafficserver.git;h=a647412 ]

TS-4309: Simplify read/write loops to address upload/download speed problems.  
This closes #629.


> Reduced SSL upload/download speed after event loop change (TS-4260)
> ---
>
> Key: TS-4309
> URL: https://issues.apache.org/jira/browse/TS-4309
> Project: Traffic Server
>  Issue Type: Bug
>  Components: Core
>Reporter: Susan Hinrichs
>Assignee: Susan Hinrichs
> Fix For: 7.0.0
>
>
> With the Eventloop changes of TS-4260 there are fewer spurious event loop 
> kicks.  This reveals stalls during upload and download on a lightly loaded 
> system.  Tidying up the net read/write loops and reactivating a never 
> triggered signal fixed the problem.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304221#comment-15304221
 ] 

ASF GitHub Bot commented on TS-4488:


Github user jpeach commented on the pull request:

https://github.com/apache/trafficserver/pull/674#issuecomment-222182844
  
What's the trigger for this bug? Is it reproducible? Can we add a 
regression test?


> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304220#comment-15304220
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64925841
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
--- End diff --

The STL design pattern is to associate the allocator with the container. In 
this approach knowledge of the allocator is distributed across the container 
and the contained objects. It would be much better to isolate this and have the 
container return Ptr directly, hiding all the internal implementation 
details.


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two types of storage (static-- blocks and dynamic-- 
> alloc). The static side of the cache can hold N HostDBInfo structs (the 
> results of DNS queries). The dynamic side is used to store the round robin 
> records and various strings associated with the record. The size of this 
> dynamic space is defined as (N x [estimated_heap_bytes_per_entry. The basic 
> problem we are running into is that we are putting too much preassure on the 
> dynamic heap-- such that the heap is getting re-used while people still have 
> references to items in that space.
> So, I've actually been working on re-writing MultiCache to allocate the 
> entire required block at once (so we don't have this problem where the parent 
> exists but not the children), but I'm not certain if we want such a change to 
> go into the 6.x branch (I'm willing to discuss if we want). If we aren't 
> comfortable with such a large change I suggest just accounting for the 
> hostname size in the estimated_heap_bytes_per_entry as a stopgap solution. 
> The maximum allowable size is 253 (so 254 with null terminator), but we could 
> pick a smaller number (~120 or so seems to be more reasonable). Alternatively 
> you can increase the number of records in hostdb (and the size accordingly) 
> to increase the dynamic heap size.
> TLDR; almost done with the long term solution, but I'm not sure if we want to 
> merge that into 6.x-- alternatively we can do a simple workaround in 6.x 
> (https://github.com/apache/trafficserver/pull/553)
> {quote}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304211#comment-15304211
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64924636
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
+  void copy(std::vector *items);
+
+  typedef typename TSHashTable::iterator 
iteratortype;
+  typedef typename TSHashTable::self hashtype;
+  typedef typename TSHashTable::Location 
locationtype;
+  TSHashTable *getMap();
+
+  PtrMutex lock; // Lock
+
+private:
+  int maxSize;
+  int maxItems;
+
+  int size;
+  int items;
+
+  hashtype itemMap;
+};
+
+template  RefCountCachePartition::RefCountCachePartition(int 
maxSize, int maxItems)
+{
+  this->maxSize = maxSize;
+  this->maxItems = maxItems;
+  this->size = 0;
+  this->items = 0;
+
+  // Initialize lock
+  this->lock = new_ProxyMutex();
+}
+
+// TODO: error somehow if you asked for something too big!
+// Allocate space for an item `C` as well as `size` extra space.
+template 
+RefCountCacheItem *
+RefCountCachePartition::alloc(uint64_t key, int size)
+{
+  size += sizeof(C);
+  // Remove any colliding entries
+  this->del(key);
+
+  // Now allocat a sufficiently large iobuf to store our data
+  // as well as the intermediate layers we have
+  int allocSize = size + sizeof(RefCountCacheItem) + 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304192#comment-15304192
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64923198
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
--- End diff --

I think you meant ``typedef``. Elsewhere we don't use a typedef for 
``Ptr``, so we should either use it everywhere or nowhere.


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two types of storage (static-- blocks and dynamic-- 
> alloc). The static side of the cache can hold N HostDBInfo structs (the 
> results of DNS queries). The dynamic side is used to store the round robin 
> records and various strings associated with the record. The size of this 
> dynamic space is defined as (N x [estimated_heap_bytes_per_entry. The basic 
> problem we are running into is that we are putting too much preassure on the 
> dynamic heap-- such that the heap is getting re-used while people still have 
> references to items in that space.
> So, I've actually been working on re-writing MultiCache to allocate the 
> entire required block at once (so we don't have this problem where the parent 
> exists but not the children), but I'm not certain if we want such a change to 
> go into the 6.x branch (I'm willing to discuss if we want). If we aren't 
> comfortable with such a large change I suggest just accounting for the 
> hostname size in the estimated_heap_bytes_per_entry as a stopgap solution. 
> The maximum allowable size is 253 (so 254 with null terminator), but we could 
> pick a smaller number (~120 or so seems to be more reasonable). Alternatively 
> you can increase the number of records in hostdb (and the size accordingly) 
> to increase the dynamic heap size.
> TLDR; almost done with the long term solution, but I'm not sure if we want to 
> merge that into 6.x-- alternatively we can do a simple workaround in 6.x 
> (https://github.com/apache/trafficserver/pull/553)
> {quote}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304188#comment-15304188
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64922689
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
+  void copy(std::vector *items);
+
+  typedef typename TSHashTable::iterator 
iteratortype;
--- End diff --

``iterator_type``


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304187#comment-15304187
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64922490
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
+  void clear();
+
+  int numItems();
--- End diff --

``size()``


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two types of storage (static-- blocks and dynamic-- 
> alloc). The static side of the cache can 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304185#comment-15304185
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64922440
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
+class RefCountCacheHashingValue
+{
+public:
+  Ptr item;
+  LINK(RefCountCacheHashingValue, item_link);
+  RefCountCacheHashingValue(Ptr i) : item(i){};
+};
+
+// TODO: template?
+struct RefCountCacheHashing {
+  typedef uint64_t ID;
+  typedef uint64_t const Key;
+  typedef RefCountCacheHashingValue Value;
+  typedef DList(RefCountCacheHashingValue, item_link) ListHead;
+
+  static ID
+  hash(Key key)
+  {
+return key;
+  }
+  static Key
+  key(Value const *value)
+  {
+return value->item->key;
+  }
+  static bool
+  equal(Key lhs, Key rhs)
+  {
+return lhs == rhs;
+  }
+};
+
+// The RefCountCachePartition is simply a map of key -> 
Ptr
+// TODO: add metrics
+// TODO: store partition number for debugging
+template  class RefCountCachePartition
+{
+public:
+  RefCountCachePartition(int maxSize, int maxItems);
+  RefCountCacheItem *alloc(uint64_t key, int size = 0);
+  Ptr get(uint64_t key);
+  void del(uint64_t key);
--- End diff --

Prefer to use STL terminology where it makes sense. ``del`` would be 
``erase``.


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two types of storage (static-- blocks and dynamic-- 
> alloc). The static 

[jira] [Commented] (TS-4331) Hostdb consistency problems due to MultiCache

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304183#comment-15304183
 ] 

ASF GitHub Bot commented on TS-4331:


Github user jpeach commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/653#discussion_r64922290
  
--- Diff: iocore/hostdb/P_RefCountCache.h ---
@@ -0,0 +1,572 @@
+#ifndef _P_RefCountCache_h_
+#define _P_RefCountCache_h_
+
+#include "I_EventSystem.h"
+
+#include 
+#include 
+
+#include 
+// TODO: no vector?
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "ts/I_Version.h"
+
+#define REFCOUNT_CACHE_EVENT_SYNC REFCOUNT_CACHE_EVENT_EVENTS_START
+
+#define REFCOUNTCACHE_MAGIC_NUMBER 0x0BAD2D9
+#define REFCOUNTCACHE_MAJOR_VERSION 1
+#define REFCOUNTCACHE_MINOR_VERSION 0
+
+// TODO: elsewhere?
+#define PtrMutex Ptr
+
+// In the cache we want to recount our items
+// This class will handle memory allocations (on the freelist) and
+// take care of implementing RefCountObj-- so that the items we want in
+// the cache don't have to inherit from it (which makes 
serialization/deserialization much easier)
+class RefCountCacheItemBase : public RefCountObj
+{
+public:
+  RefCountCacheItemBase(uint64_t key = 0, unsigned int size = 0, char 
*iobuf = NULL, int iobuffer_index = 0)
+  {
+this->key = key;
+this->size = size;
+this->iobuf = iobuf;
+this->iobuffer_index = iobuffer_index;
+  }
+  void
+  free()
+  {
+if (this->size > 0) {
+  // free the block
+  ioBufAllocator[iobuffer_index].free_void((void *)(iobuf));
+}
+  }
+  uint64_t key;  // Key of this item
+  unsigned int size; // how much space does this guy get
+  int64_t iobuffer_index;
+  char *iobuf;
+};
+
+// Template to the particular class, so we can caste the item ptr correctly
+template  class RefCountCacheItem : public RefCountCacheItemBase
+{
+public:
+  using RefCountCacheItemBase::RefCountCacheItemBase;
+  C *item();
+};
+
+template 
+C *
+RefCountCacheItem::item()
+{
+  return (C *)this->iobuf;
+}
+
+// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
--- End diff --

*indirection


> Hostdb consistency problems due to MultiCache
> -
>
> Key: TS-4331
> URL: https://issues.apache.org/jira/browse/TS-4331
> Project: Traffic Server
>  Issue Type: Bug
>  Components: HostDB
>Reporter: Thomas Jackson
>Assignee: Thomas Jackson
> Fix For: 7.0.0
>
>
> This ticket is for the correct long term fix to TS-4207
> pulled from a comment, which wraps up the issue
> {quote}
> Leif Hedstrom I have spent a decent amount of time on this while I was OOO on 
> vacation the last couple of weeks. It seems that the root cause of this issue 
> has always existed, and that the addition of always doing hostname storing 
> (https://github.com/apache/trafficserver/commit/0e703e1e) we are just causing 
> the issue to happen all the time.
> To understand the issue I'll give a little background in how hostdb is 
> currently working. Basically hostdb is just a wrapper around this templated 
> struct called MultiCache. MultiCache is "multi" not because it is templated, 
> but because it has two types of storage (static-- blocks and dynamic-- 
> alloc). The static side of the cache can hold N HostDBInfo structs (the 
> results of DNS queries). The dynamic side is used to store the round robin 
> records and various strings associated with the record. The size of this 
> dynamic space is defined as (N x [estimated_heap_bytes_per_entry. The basic 
> problem we are running into is that we are putting too much preassure on the 
> dynamic heap-- such that the heap is getting re-used while people still have 
> references to items in that space.
> So, I've actually been working on re-writing MultiCache to allocate the 
> entire required block at once (so we don't have this problem where the parent 
> exists but not the children), but I'm not certain if we want such a change to 
> go into the 6.x branch (I'm willing to discuss if we want). If we aren't 
> comfortable with such a large change I suggest just accounting for the 
> hostname size in the estimated_heap_bytes_per_entry as a stopgap solution. 
> The maximum allowable size is 253 (so 254 with null terminator), but we could 
> pick a smaller number (~120 or so seems to be more reasonable). Alternatively 
> you can increase the number of records in hostdb (and the size accordingly) 
> to increase the dynamic heap 

[jira] [Commented] (TS-4487) Don't reschedule read depend on needs & did not check the change of lock at the return callback with wbe.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304177#comment-15304177
 ] 

ASF GitHub Bot commented on TS-4487:


Github user shinrich commented on the pull request:

https://github.com/apache/trafficserver/pull/673#issuecomment-222175909
  
I think the right thing to do is push PR #629.  That moves the read/write 
loops to use the IOBuffer interface instead of the IOBlock interface.  I've 
been holding off on this one until the event loop changes landed since it 
solved a real problem I had seen.  But if other folks are seeing problems in 
this area  the IOBuffer interface code is much easier to reason about and 
debug. 


> Don't reschedule read depend on needs & did not check the change of lock at 
> the return callback with wbe.
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {  // should check needs==0
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}
> another issue in write_to_net_io(): did not check the change of lock at the 
> return callback with wbe.
> {code}
>   if (s->vio.ntodo() <= 0) {
> write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
> return;
>   } else if (signalled && (wbe_event != vc->write_buffer_empty_event)) {
> // @a signalled means we won't send an event, and the event values 
> differing means we
> // had a write buffer trap and cleared it, so we need to send it now.
> if (write_signal_and_update(wbe_event, vc) != EVENT_CONT)
>   return;
> // > did not check the change of lock at the return callback 
> with wbe.
>   } else if (!signalled) {
> if (write_signal_and_update(VC_EVENT_WRITE_READY, vc) != EVENT_CONT) {
>   return;
> }
> // change of lock... don't look at shared variables!
> if (lock.get_mutex() != s->vio.mutex.get()) {
>   write_reschedule(nh, vc);
>   return;
> }
>   }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304160#comment-15304160
 ] 

ASF GitHub Bot commented on TS-4488:


Github user yatsukhnenko commented on the pull request:

https://github.com/apache/trafficserver/pull/674#issuecomment-222170387
  
fixed


> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304156#comment-15304156
 ] 

ASF GitHub Bot commented on TS-4488:


Github user zwoop commented on the pull request:

https://github.com/apache/trafficserver/pull/674#issuecomment-222169540
  
Failed clang-format this time :)

https://ci.trafficserver.apache.org/job/Github-Linux/43/console


> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304141#comment-15304141
 ] 

ASF GitHub Bot commented on TS-4488:


Github user yatsukhnenko commented on the pull request:

https://github.com/apache/trafficserver/pull/674#issuecomment-222164906
  
Moved variable declaration to top of function, try to build again, pls


> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4487) Don't reschedule read depend on needs & did not check the change of lock at the return callback with wbe.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304127#comment-15304127
 ] 

ASF GitHub Bot commented on TS-4487:


Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/673#discussion_r64913857
  
--- Diff: iocore/net/UnixNetVConnection.cc ---
@@ -545,57 +545,61 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection 
*vc, EThread *thread)
 vc->write.triggered = 0;
 write_signal_error(nh, vc, (int)-r);
 return;
-  } else {
-int wbe_event = vc->write_buffer_empty_event; // save so we can clear 
if needed.
+  }
 
-NET_SUM_DYN_STAT(net_write_bytes_stat, r);
+  int wbe_event = vc->write_buffer_empty_event; // save so we can clear if 
needed.
 
-// Remove data from the buffer and signal continuation.
-ink_assert(buf.reader()->read_avail() >= r);
-buf.reader()->consume(r);
-ink_assert(buf.reader()->read_avail() >= 0);
-s->vio.ndone += r;
+  NET_SUM_DYN_STAT(net_write_bytes_stat, r);
 
-// If the empty write buffer trap is set, clear it.
-if (!(buf.reader()->is_read_avail_more_than(0)))
-  vc->write_buffer_empty_event = 0;
+  // Remove data from the buffer and signal continuation.
+  ink_assert(buf.reader()->read_avail() >= r);
--- End diff --

Use `is_read_avail_more_than(r)` and `is_read_avail_more_than(0)`.


> Don't reschedule read depend on needs & did not check the change of lock at 
> the return callback with wbe.
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {  // should check needs==0
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}
> another issue in write_to_net_io(): did not check the change of lock at the 
> return callback with wbe.
> {code}
>   if (s->vio.ntodo() <= 0) {
> write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
> return;
>   } else if (signalled && (wbe_event != vc->write_buffer_empty_event)) {
> // @a signalled means we won't send an event, and the event values 
> differing means we
> // had a write buffer trap and cleared it, so we need to send it now.
> if (write_signal_and_update(wbe_event, vc) != EVENT_CONT)
>   return;
> // > did not check the change of lock at the return callback 
> with wbe.
>   } else if (!signalled) {
> if (write_signal_and_update(VC_EVENT_WRITE_READY, vc) != EVENT_CONT) {
>   return;
> }
> // change of lock... don't look at shared variables!
> if (lock.get_mutex() != s->vio.mutex.get()) {
>   write_reschedule(nh, vc);
>   return;
> }
>   }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4487) Don't reschedule read depend on needs & did not check the change of lock at the return callback with wbe.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304125#comment-15304125
 ] 

ASF GitHub Bot commented on TS-4487:


Github user SolidWallOfCode commented on a diff in the pull request:

https://github.com/apache/trafficserver/pull/673#discussion_r64913617
  
--- Diff: iocore/net/UnixNetVConnection.cc ---
@@ -545,57 +545,61 @@ write_to_net_io(NetHandler *nh, UnixNetVConnection 
*vc, EThread *thread)
 vc->write.triggered = 0;
 write_signal_error(nh, vc, (int)-r);
 return;
-  } else {
-int wbe_event = vc->write_buffer_empty_event; // save so we can clear 
if needed.
+  }
 
-NET_SUM_DYN_STAT(net_write_bytes_stat, r);
+  int wbe_event = vc->write_buffer_empty_event; // save so we can clear if 
needed.
 
-// Remove data from the buffer and signal continuation.
-ink_assert(buf.reader()->read_avail() >= r);
-buf.reader()->consume(r);
-ink_assert(buf.reader()->read_avail() >= 0);
-s->vio.ndone += r;
+  NET_SUM_DYN_STAT(net_write_bytes_stat, r);
 
-// If the empty write buffer trap is set, clear it.
-if (!(buf.reader()->is_read_avail_more_than(0)))
-  vc->write_buffer_empty_event = 0;
+  // Remove data from the buffer and signal continuation.
+  ink_assert(buf.reader()->read_avail() >= r);
+  buf.reader()->consume(r);
+  ink_assert(buf.reader()->read_avail() >= 0);
+  s->vio.ndone += r;
 
-net_activity(vc, thread);
-// If there are no more bytes to write, signal write complete,
-ink_assert(ntodo >= 0);
-if (s->vio.ntodo() <= 0) {
-  write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
-  return;
-} else if (signalled && (wbe_event != vc->write_buffer_empty_event)) {
+  // If the empty write buffer trap is set, clear it.
+  if (!(buf.reader()->is_read_avail_more_than(0)))
+vc->write_buffer_empty_event = 0;
+
+  net_activity(vc, thread);
+  // If there are no more bytes to write, signal write complete,
+  ink_assert(ntodo >= 0);
+  int e = 0;
+  if (s->vio.ntodo() <= 0) {
+write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
+return;
+  } else {
+if (!signalled) {
+  e = VC_EVENT_WRITE_READY;
+} else if (wbe_event != vc->write_buffer_empty_event) {
   // @a signalled means we won't send an event, and the event values 
differing means we
   // had a write buffer trap and cleared it, so we need to send it now.
-  if (write_signal_and_update(wbe_event, vc) != EVENT_CONT)
-return;
-} else if (!signalled) {
-  if (write_signal_and_update(VC_EVENT_WRITE_READY, vc) != EVENT_CONT) 
{
-return;
-  }
-
-  // change of lock... don't look at shared variables!
-  if (lock.get_mutex() != s->vio.mutex.get()) {
-write_reschedule(nh, vc);
-return;
-  }
+  e = wbe_event;
 }
-
-if (!buf.reader()->read_avail()) {
-  write_disable(nh, vc);
+  }
+  if (e) {
+if (write_signal_and_update(e, vc) != EVENT_CONT) {
   return;
 }
 
-if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
+// change of lock... don't look at shared variables!
+if (lock.get_mutex() != s->vio.mutex.get()) {
   write_reschedule(nh, vc);
+  return;
 }
-if ((needs & EVENTIO_READ) == EVENTIO_READ) {
-  read_reschedule(nh, vc);
-}
+  }
+
+  if (needs==0 && !buf.reader()->read_avail()) {
--- End diff --

Should use `read_avail_more_than(0)` here for performance as the exact 
number of bytes is irrelevant.


> Don't reschedule read depend on needs & did not check the change of lock at 
> the return callback with wbe.
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {  // should check needs==0
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}
> another issue in write_to_net_io(): did not check the change of lock at the 
> return callback with wbe.
> {code}
>   if (s->vio.ntodo() <= 

[jira] [Commented] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304112#comment-15304112
 ] 

ASF GitHub Bot commented on TS-4488:


Github user zwoop commented on the pull request:

https://github.com/apache/trafficserver/pull/674#issuecomment-222159292
  
Note that this fails the unit checks, please see details on

https://ci.trafficserver.apache.org/job/Github-Linux/42/console


> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304098#comment-15304098
 ] 

ASF GitHub Bot commented on TS-4488:


Github user atsci commented on the pull request:

https://github.com/apache/trafficserver/pull/674#issuecomment-222156833
  
This is a test message.


> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Closed] (TS-4455) remove isEosRcvd() and eosRcvd

[Susan Hinrichs (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4455?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Susan Hinrichs closed TS-4455.
--
Resolution: Fixed

> remove isEosRcvd() and eosRcvd
> --
>
> Key: TS-4455
> URL: https://issues.apache.org/jira/browse/TS-4455
> Project: Traffic Server
>  Issue Type: Bug
>  Components: SSL
>Reporter: Oknet Xu
> Fix For: 7.0.0
>
>
> the isEosRcvd() is used by HttpSM.
> It is check SSLNetVConnection->eosRcvd while received VC_EVENT_READ_READY or 
> VC_EVENT_READ_COMPLETE event, and rewrite the event to VC_EVENT_EOS.
> {code}
> int
> HttpSM::state_read_client_request_header(int event, void *data)
> {
> .
> .
> .
>   // check to see if there was an EOS received on the SSL connection
>   SSLNetVConnection *ssl_vc = dynamic_cast *>(ua_session->get_netvc());
>   if (ssl_vc && ssl_vc->isEosRcvd()) {
> DebugSM("http", "EOS for ssl vc %p at read_first_btye state", 
> ua_session->get_netvc());
> event = VC_EVENT_EOS;
>   }
>   switch (event) {
>   case VC_EVENT_READ_READY:
>   case VC_EVENT_READ_COMPLETE:
> // More data to parse
> break;
>   case VC_EVENT_EOS:
> ua_entry->eos = true;
> if ((client_request_hdr_bytes > 0) && 
> is_transparent_passthrough_allowed() && (ua_raw_buffer_reader != NULL)) {
>   break;
> }
> .
> .
> .
> {code}
> the eosRcvd is set to true only in SSLNetVConnection::net_read_io()
> {code}
> // changed by YTS Team, yamsat
> void
> SSLNetVConnection::net_read_io(NetHandler *nh, EThread *lthread)
> {
> .
> .
> .
>   case SSL_READ_EOS:
> // close the connection if we have SSL_READ_EOS, this is the return value 
> from ssl_read_from_net() if we get an
> // SSL_ERROR_ZERO_RETURN from SSL_get_error()
> // SSL_ERROR_ZERO_RETURN means that the origin server closed the SSL 
> connection
> eosRcvd = true;
> read.triggered = 0;
> readSignalDone(VC_EVENT_EOS, nh);
> if (bytes > 0) {
>   Debug("ssl", "read_from_net, read finished - EOS");
> } else {
>   Debug("ssl", "read_from_net, read finished - 0 useful bytes read, bytes 
> used by SSL layer");
> }
> break;
> .
> .
> .
> {code}
> It is also signal VC_EVENT_EOS to SM after set eosRcvd to true.
> thus, the eosRcvd and isEosRcvd() is no longer necessary.
> suggest remove them.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15304067#comment-15304067
 ] 

ASF GitHub Bot commented on TS-4488:


Github user shinrich commented on the pull request:

https://github.com/apache/trafficserver/pull/674#issuecomment-222147656
  
Looks reasonable to me.


> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4488) Segmentation fault at HttpSM::tunnel_handler_ua

[Pavlo Yatsukhnenko (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4488?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Pavlo Yatsukhnenko updated TS-4488:
---
  Flags: Patch
Description: From Github PR #674
Summary: Segmentation fault at HttpSM::tunnel_handler_ua  (was: 
Segmentation fault)

> Segmentation fault at HttpSM::tunnel_handler_ua
> ---
>
> Key: TS-4488
> URL: https://issues.apache.org/jira/browse/TS-4488
> Project: Traffic Server
>  Issue Type: Bug
>Reporter: Pavlo Yatsukhnenko
>
> From Github PR #674



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (TS-4488) Segmentation fault

[Pavlo Yatsukhnenko (JIRA)]

Pavlo Yatsukhnenko created TS-4488:
--

 Summary: Segmentation fault
 Key: TS-4488
 URL: https://issues.apache.org/jira/browse/TS-4488
 Project: Traffic Server
  Issue Type: Bug
Reporter: Pavlo Yatsukhnenko






--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (TS-4487) Don't reschedule read depend on needs & did not check the change of lock at the return callback with wbe.

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15303891#comment-15303891
 ] 

ASF GitHub Bot commented on TS-4487:


GitHub user oknet opened a pull request:

https://github.com/apache/trafficserver/pull/673

TS-4487: fix write_to_net_io issues



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/oknet/trafficserver patch-13

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/trafficserver/pull/673.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #673


commit 1f7b4e670bc70e5583958e6c25e473fbd683228d
Author: Oknet 
Date:   2016-05-27T10:26:19Z

TS-4487: fix write_to_net_io issues




> Don't reschedule read depend on needs & did not check the change of lock at 
> the return callback with wbe.
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {  // should check needs==0
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}
> another issue in write_to_net_io(): did not check the change of lock at the 
> return callback with wbe.
> {code}
>   if (s->vio.ntodo() <= 0) {
> write_signal_done(VC_EVENT_WRITE_COMPLETE, nh, vc);
> return;
>   } else if (signalled && (wbe_event != vc->write_buffer_empty_event)) {
> // @a signalled means we won't send an event, and the event values 
> differing means we
> // had a write buffer trap and cleared it, so we need to send it now.
> if (write_signal_and_update(wbe_event, vc) != EVENT_CONT)
>   return;
> // > did not check the change of lock at the return callback 
> with wbe.
>   } else if (!signalled) {
> if (write_signal_and_update(VC_EVENT_WRITE_READY, vc) != EVENT_CONT) {
>   return;
> }
> // change of lock... don't look at shared variables!
> if (lock.get_mutex() != s->vio.mutex.get()) {
>   write_reschedule(nh, vc);
>   return;
> }
>   }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't reschedule read depend on needs & did not check the change of lock at the return callback with wbe.

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Summary: Don't reschedule read depend on needs & did not check the change 
of lock at the return callback with wbe.  (was: Don't reschedule read depend on 
needs)

> Don't reschedule read depend on needs & did not check the change of lock at 
> the return callback with wbe.
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {  // should check needs==0
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't reschedule read depend on needs

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Description: 
the code:
{code}
int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, buf, 
needs);
{code}

At the end of write_to_net_io, 

{code}
if (!buf.reader()->read_avail()) {  // should check needs==0
  write_disable(nh, vc);
  return;
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}

  was:
the code:
{code}
int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, buf, 
needs);
{code}

At the end of write_to_net_io, 

{code}
if (!buf.reader()->read_avail()) {  // should check needs==0
  write_disable(nh, vc);
  return;
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
//  here r>0, don't need to check the needs.
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}


> Don't reschedule read depend on needs
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {  // should check needs==0
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't reschedule read depend on needs

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Description: 
the code:
{code}
int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, buf, 
needs);
{code}

At the end of write_to_net_io, 

{code}
if (!buf.reader()->read_avail()) {  // should check needs==0
  write_disable(nh, vc);
  return;
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
//  here r>0, don't need to check the needs.
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}

  was:
the code:
{code}
int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, buf, 
needs);
{code}

In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
EVENTIO_WRITE on r>0.

At the end of write_to_net_io, 

{code}
if (!buf.reader()->read_avail()) {
  write_disable(nh, vc);
  return;
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
//  here r>0, don't need to check the needs.
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}


> Don't reschedule read depend on needs
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {  // should check needs==0
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> //  here r>0, don't need to check the needs.
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't reschedule read depend on needs

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Summary: Don't reschedule read depend on needs  (was: Don't need reschedule 
read depend on needs)

> Don't reschedule read depend on needs
> -
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
> EVENTIO_WRITE on r>0.
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> //  here r>0, don't need to check the needs.
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't need reschedule read depend on needs

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Description: 
the code:
{code}
int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, buf, 
needs);
{code}

In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
EVENTIO_WRITE on r>0.

At the end of write_to_net_io, 

{code}
if (!buf.reader()->read_avail()) {
  write_disable(nh, vc);
  return;
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
//  here r>0, don't need to check the needs.
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}

  was:
the code:
{code}
int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, buf, 
needs);
{code}

In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
EVENTIO_WRITE on r>0.

At the end of write_to_net_io, 

{code}
if (!buf.reader()->read_avail()) {
  write_disable(nh, vc);
  return;  <-- return from here, but don't reschedule read
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}


> Don't need reschedule read depend on needs
> --
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
> EVENTIO_WRITE on r>0.
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {
>   write_disable(nh, vc);
>   return;
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> //  here r>0, don't need to check the needs.
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't need reschedule read depend on needs

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Issue Type: Improvement  (was: Bug)

> Don't need reschedule read depend on needs
> --
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Improvement
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
> EVENTIO_WRITE on r>0.
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {
>   write_disable(nh, vc);
>   return;  <-- return from here, but don't reschedule read
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't reschedule read depend on needs while write buffer empty

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Description: 
the code:
{code}
int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, buf, 
needs);
{code}

In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
EVENTIO_WRITE on r>0.

At the end of write_to_net_io, 

{code}
if (!buf.reader()->read_avail()) {
  write_disable(nh, vc);
  return;  <-- return from here, but don't reschedule read
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}

  was:
At the end of write_to_net_io
{code}
if (!buf.reader()->read_avail()) {
  write_disable(nh, vc);
  return;  <-- return from here, but don't reschedule read
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}


> Don't reschedule read depend on needs while write buffer empty
> --
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Bug
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
> EVENTIO_WRITE on r>0.
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {
>   write_disable(nh, vc);
>   return;  <-- return from here, but don't reschedule read
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (TS-4487) Don't need reschedule read depend on needs

[Oknet Xu (JIRA)]

 [ 
https://issues.apache.org/jira/browse/TS-4487?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Oknet Xu updated TS-4487:
-
Summary: Don't need reschedule read depend on needs  (was: Don't reschedule 
read depend on needs while write buffer empty)

> Don't need reschedule read depend on needs
> --
>
> Key: TS-4487
> URL: https://issues.apache.org/jira/browse/TS-4487
> Project: Traffic Server
>  Issue Type: Bug
>  Components: SSL
>Reporter: Oknet Xu
>
> the code:
> {code}
> int64_t r = vc->load_buffer_and_write(towrite, wattempted, total_written, 
> buf, needs);
> {code}
> In the SSLNetVConnection::load_buffer_and_write(), only set needs |= 
> EVENTIO_WRITE on r>0.
> At the end of write_to_net_io, 
> {code}
> if (!buf.reader()->read_avail()) {
>   write_disable(nh, vc);
>   return;  <-- return from here, but don't reschedule read
> }
> if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
>   write_reschedule(nh, vc);
> }
> if ((needs & EVENTIO_READ) == EVENTIO_READ) {
>   read_reschedule(nh, vc);
> }
> return;
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (TS-4487) Don't reschedule read depend on needs while write buffer empty

[Oknet Xu (JIRA)]

Oknet Xu created TS-4487:


 Summary: Don't reschedule read depend on needs while write buffer 
empty
 Key: TS-4487
 URL: https://issues.apache.org/jira/browse/TS-4487
 Project: Traffic Server
  Issue Type: Bug
  Components: SSL
Reporter: Oknet Xu


At the end of write_to_net_io
{code}
if (!buf.reader()->read_avail()) {
  write_disable(nh, vc);
  return;  <-- return from here, but don't reschedule read
}

if ((needs & EVENTIO_WRITE) == EVENTIO_WRITE) {
  write_reschedule(nh, vc);
}
if ((needs & EVENTIO_READ) == EVENTIO_READ) {
  read_reschedule(nh, vc);
}
return;
{code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)