[
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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 <iostream>
+#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<ExampleStruct> *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<ExampleStruct> *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<RefCountCacheItem<ExampleStruct>> 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<ExampleStruct> *cache = new
RefCountCache<ExampleStruct>(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<RefCountCacheItem<ExampleStruct>> ccitem = cache->get((uint64_t)1);
+
+ Ptr<RefCountCacheItem<ExampleStruct>> 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 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)
