dnsdist: Moka cache

[edmonds]

Description

Hi,

This is a draft PR for discussion purposes because there are definitely some things that are broken or weird in this branch.

This branch replaces the underlying data store for the DNSDistPacketCache with the Rust moka crate, a "fast, concurrent cache library for Rust". cxx::bridge is used to create a binding from Rust to C++, wrapping the moka::sync::SegmentedCache object, specialized for the key and value data types required by the packet cache.

The key type stored in the moka cache is Rust's Vec<u8>. These keys are in Rust-allocated memory and store the bytes extracted by dnsdist's DNSDistPacketCache::getKey() parser, concatenated together into a contiguous buffer. Because this buffer is in Rust-allocated memory, it can be inserted into the moka cache, taking ownership of the key instead of copying it. In the C++ portion of the dnsdist code base, a new CacheKey struct has been introduced that wraps the Rust-owned Vec<u8> buffer. The parts of the code base that previously operated on Jenkins hash values directly have been updated to use this new CacheKey type to accumulate the bytes of the key. (The moka crate internally uses the aHash crate to hash keys which probably compares favorably to the Jenkins lookup2 hash.)

Adding this CacheKey abstraction and giving the bytes of the actual query key directly to the container to store seems to simplify things, because it removes the need for a post-lookup step that checks for collisions, and as a result at least half of the members of CacheValue would seem to be unnecessary and could be removed, which then makes it unnecessary to pass these fields around as function parameters.

The value type stored in the moka cache is C++'s std::shared_ptr<CacheValue>. cxx::bridge has built-in support for std::shared_ptr, which lets us allocate cache value objects on the C++ side and insert them directly into the moka cache. If I understand correctly, the refcount is automatically increased when inserting into the cache, so when moka decides to evict a cache entry it may or may not be deallocated immediately, depending on if another worker thread has taken out a reference. This is a big difference from the original implementation which stores CacheValue directly in the container and copies out the required elements from the cache value directly into the response packet while holding a read lock on the container.

Because moka handles eviction incrementally, it does not have its own background maintenance thread, and the "cache cleaning" work that was being done by dnsdist's maintenance thread is unnecessary in this branch.

Discussion

This is an early draft submitted for feedback purposes. There is another draft PR pending (#16692) that adds LRU and SIEVE into the existing implementation, but my branch is a lot more intrusive since it introduces a hard dependency on Rust and reworks how keys/values are stored in the cache.

My primary interest is in getting improved caching admission/eviction algorithms in dnsdist that have "scan resistance", or being robust against "one hit wonders", i.e. it would be very nice if a pseudorandom subdomain attack could not trivially empty the packet cache by just sending more queries than the size of the cache in a short amount of time.

I have done some initial performance tests and found that this implementation does use a few percent more CPU cycles than the original implementation, which makes sense given the additional work needed to implement the algorithms that moka uses. The tradeoff of slightly more CPU usage may be worth it if it results in a better cache hit ratio.

There are several commits marked with XXX in the commit subject that indicate where more work is needed, for instance there are some ancillary operations (dumping the cache, expunging records, searching for cached records, etc.) that the new implementation would need to support that it doesn't yet. There is also pdns/dnsdistdist/packetcache.hh which is a symlink that I broke so that I could test out the dnsdist-specific changes in this branch without affecting the other products, but there is probably a better way to do this.

Checklist

I have:

• read the CONTRIBUTING.md document
• read and accepted the Developer Certificate of Origin document, including the AI Policy, and added a "Signed-off-by" to my commits
• compiled this code
• tested this code
• included documentation (including possible behaviour changes)
• documented the code
• added or modified regression test(s)
• added or modified unit test(s)

[rgacogne]

Thanks a lot for contributing this! I have two main concerns at the moment:

• we don't want to make Rust a hard requirement for DNSdist (yet?) so any code using Rust has to be optionally built
• I'm not familiar with TinyLFU so I read the TinyLFU: A Highly Efficient Cache Admission Policy paper and https://docs.rs/moka/latest/moka/#tinylfu, and I'm worried about the actual performance under heavy load with a lot of concurrent reads. Since dnsdist is pretty much always deployed in front of a backend that has its own cache(s), we want it to be really fast even if the trade-off is a lower cache-hit ratio.

So I guess we need to come up with a realistic benchmark setup to evaluate the impact of both this PR and #16692.

[edmonds]

Thanks a lot for contributing this! I have two main concerns at the moment:

* we don't want to make Rust a hard requirement for DNSdist (yet?) so any code using Rust has to be optionally built

* I'm not familiar with TinyLFU so I read the `TinyLFU: A Highly Efficient Cache Admission Policy` paper and https://docs.rs/moka/latest/moka/#tinylfu, and I'm worried about the actual performance under heavy load with a lot of concurrent reads. Since dnsdist is pretty much always deployed in front of a backend that has its own cache(s), we want it to be really fast even if the trade-off is a lower cache-hit ratio.

So I guess we need to come up with a realistic benchmark setup to evaluate the impact of both this PR and #16692.

Thanks for taking a look!

For my use case I don't necessarily have the backends co-located with the dnsdist frontends, so I care a lot more about having a higher cache-hit rate in dnsdist's packet cache, and also about maintaining a good cache-hit rate in the face of abusive traffic like pseudorandom subdomain attacks.

Regarding performance under concurrent reads, I did make sure to use moka::sync::SegmentedCache and configure the number of segments using the existing CacheSettings::d_shardCount parameter. Looking at the moka source, it looks like SegmentedCache performs a hash of the key and uses the hash to index into an array of cache segments (which is very similar to dnsdist's existing cache sharding scheme which basically splits the cache into an array of std::unordered_map's). Or is your concern regarding the performance of concurrent reads that hash to the same cache shard?

Any thoughts on the non-algorithmic changes in this branch? I actually spent a bit more time on refactoring how DNSDistPacketCache::getKey works (returning the keying elements rather than a burtle hash of the keying elements) and getting rid of DNSDistPacketCache::cachedValueMatches than I did on the change to swap in moka as the underlying data store. I suspect this is a very old performance hack borrowed from PowerDNS 😃