[ 
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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 <ts/Map.h>
    +#include <ts/List.h>
    +
    +#include <cstdint>
    +// TODO: no vector?
    +#include <vector>
    +#include <unistd.h>
    +#include <string.h>
    +#include <sys/stat.h>
    +#include <fcntl.h>
    +#include <sys/mman.h>
    +#include <cmath>
    +#include <fstream>
    +
    +#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<ProxyMutex>
    +
    +// 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 C> class RefCountCacheItem : public RefCountCacheItemBase
    +{
    +public:
    +  using RefCountCacheItemBase::RefCountCacheItemBase;
    +  C *item();
    +};
    +
    +template <class C>
    +C *
    +RefCountCacheItem<C>::item()
    +{
    +  return (C *)this->iobuf;
    +}
    +
    +// Layer of inderection for the hashmap-- since it needs lots of things 
inside of it
    +class RefCountCacheHashingValue
    +{
    +public:
    +  Ptr<RefCountCacheItemBase> item;
    +  LINK(RefCountCacheHashingValue, item_link);
    +  RefCountCacheHashingValue(Ptr<RefCountCacheItemBase> 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<RefCountCacheItem<YourClass>>
    +// TODO: add metrics
    +// TODO: store partition number for debugging
    +template <class C> class RefCountCachePartition
    +{
    +public:
    +  RefCountCachePartition(int maxSize, int maxItems);
    +  RefCountCacheItem<C> *alloc(uint64_t key, int size = 0);
    +  Ptr<RefCountCacheItem<C>> 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, 
> 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)

Reply via email to