[
https://issues.apache.org/jira/browse/TS-4331?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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 <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 --
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 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)
