dnsdist: add generic object cache - #17561
Conversation
|
This was mostly implemented to work with: #17387, but it could be useful otherwise too I think. |
b81fd63 to
4f59f25
Compare
|
Thanks for opening this PR! I'll do a high-level review soon to see if this is something that are interested in merging, please don't spend more time on this until we have reached a decision! |
Thanks. That makes sense. |
Coverage Report for CI Build 28880705225Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-1.7%) to 69.36%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions3420 previously-covered lines in 67 files lost coverage.
Coverage Stats💛 - Coveralls |
|
I'm wondering if we could somehow merge this work with the one happening in #16692, by implementing a generic enough cache class and then specializing it for DNS packets. That way we would (hopefully) avoid duplicating a lot of Lua bindings / YAML settings, if we manage to make them generic enough to apply to the packet cache and this generic object cache. Or am I too optimistic? :-) |
|
Well it seems like it could be possible, considering this cache is supposed to be generic. I need to take a deeper look into that PR, but at first glance it seems possible. |
What is the status on that PR? I believe I could integrate this work into that (with some minor modifications maybe, not sure), but I just wanted to know if that is a solid foundation that I could build on or if it is something that will change a lot. |
That's a very good question! Perhaps @karelbilek can chime in? One possibility might be to define a generic enough interface that you could based your work on, and even if it turns out that we need to change a few things later that shouldn't be too hard. Note that we could also approach this the other way around: use the existing interface as the basis for your generic object cache, merge it before #16692, and adapt it later if needed for the changes that #16692 wants to introduce. |
That second option sounds good too. Maybe better to not base my work off of another draft PR. We can easily adjust later I think. |
|
Just to confirm - the existing interface refers to |
Yes, it doesn't have to be fully compatible, of course! |
4f59f25 to
7b79aac
Compare
Signed-off-by: Ensar Sarajčić <dev@ensarsarajcic.com>
Signed-off-by: Ensar Sarajčić <dev@ensarsarajcic.com>
7b79aac to
f6c4ed5
Compare
Signed-off-by: Ensar Sarajčić <dev@ensarsarajcic.com>
|
@rgacogne did you mean something like this? I haven't been able to use the same iinterface exactly (because of some packet cache specific things), but the generic cache is largely based on the packet cache and I have just changed the packet cache to use the generic cache internally. |
That's pretty close to what I had in mind, yes, thank you! I did not review all the changes yet but at a quick glance this looks great, code duplication is very low and the new interfaces will likely make it easier to implement S3-FIFO/LRU/etc versions of the packet cache in the future. |
|
If this approach makes sense, should I start adding tests and documentation for this, to get this PR ready for review? |
|
That sounds great, please do :) |
Short description
This adds a new object usable from Lua -
GenericCache- which can be used to store any values under string keys and retrieve them later on.Checklist
I have: