TS-4331: Major re-write of hostdb #653
Conversation
jacksontj
changed the title
jacksontj
force-pushed
the
hostdb_cleanup
branch
4 times, most recently
from
6d5bb74
to
8df86d6
Compare
| int cause_of_death_errno; // in | ||
| HostDBInfo host_db_info; // in | ||
| int cause_of_death_errno; // in | ||
| Ptr<RefCountCacheItem<HostDBInfo>> *hostdb_entry; // Pointer to the entry we are referencing in hostdb-- to keep our ref |
There was a problem hiding this comment.
hostdb_entry doesn't need to be allocated, it should be Ptr<RefCountCacheItem<HostDBInfo>>. Consider making a typedef for RefCountCacheItem<HostDBInfo> to reduce the verbosity.
jacksontj
force-pushed
the
hostdb_cleanup
branch
4 times, most recently
from
0ad71c3
to
dcbb672
Compare
| ats_scoped_str rundir(RecConfigReadRuntimeDir()); | ||
| ats_scoped_str config(Layout::relative_to(rundir, "hostdb.config")); | ||
| // Combine the path and name | ||
| char full_path[2 * PATH_NAME_MAX]; |
There was a problem hiding this comment.
Presumably if you really need more than PATH_NAME_MAX it's not going to work anyway.
There was a problem hiding this comment.
Yea, TBH I'm not sure why thats like that-- it was before (in a different part of the code-- moved to here. Presumably we actually know what the size is-- since we have the filename and path... I can definitely clean that :)
|
The IPv4 and IPv6 addresses are stored in the same hash table because that was easiest at the time. There's certainly no fundamental reason to do that, although maybe it helps with keeping all the addresses for a host entry with both in the same container. |
| unsigned int num_partitions; | ||
|
|
||
| std::string filepath; | ||
| std::string dirname; |
There was a problem hiding this comment.
You should not need to store this again here. HostDBSync has a copy and RefCountCacheSync has a copy, we should be able to without this one.
There was a problem hiding this comment.
Correct, must have missed them in the cleanup ;)
|
@jpeach that should cover all but the mac build case (i'll fix that once I'm in the office). So, once the mac builds are working it sounds like we might be ready for a merge. Note: i will squash these commits before merging, keeping them separate for now to hopefully confuse github less during the review :) |
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/349/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/243/ for details. |
jacksontj
force-pushed
the
hostdb_cleanup
branch
from
ab73fc9
to
795f7b1
Compare
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/255/ for details. |
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/361/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/256/ for details. |
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/362/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/257/ for details. |
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/363/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/258/ for details. |
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/364/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/259/ for details. |
|
👍 |
jacksontj
force-pushed
the
hostdb_cleanup
branch
from
4e45c31
to
3dcd78a
Compare
Before doing the rewrite of hostdb (TS-4331) this commit cleans up someo f the method names etc. to make this subsystem less confusing
jacksontj
force-pushed
the
hostdb_cleanup
branch
from
3dcd78a
to
873e517
Compare
|
FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/365/ for details. |
|
Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/260/ for details. |
This is still a WIP, but I figure it's about time to get some feedback on the code here.
The primary goal of this is to fix the crashing issues described in TS-4331, where basically multicache wasn't keeping track of who referenced various items within the cache.
At this point my TODO list still is:
HOST_DB_MAX_ROUND_ROBIN_INFOas a compile time constant-- make it configurable (this will probably get bumped into a separate PR-- since it requires some work down in the dns processor as well).Nice-to-have:
I'm sure more will come up, I'll do my best to keep this list ^^ up-to-date
Note: the plan for this PR is to get something better that works in, I don't want this PR to drag out forever due to scope creep ;) At this point I think we have enough features complete to merge this.
After testing and rolling this change out internally, we see a significant performance improvement even over multicache with no persistance (this is the latency of an in-memory fileserver, Blue is "control-- specifically normal multicache with syncing, Red is this PR, green is "control with no syncing"-- the spikes are the crashes from multicache) :
