SOLR-16654: Add support for node-level caches

[magibney]

See: SOLR-16654

[magibney]

apologies for the force-pushes, I won't do any more of them -- was just quick-fixing trivial precommit issues and trying to keep clarity of commit history.

[dsmiley]

so warmupTime is in milliseconds but apparently warmingStartTime is nanoseconds? I recommend a units suffix so that this is more obvious like "Ms" at the end or "Ns".

[dsmiley]

Pretty cool!

[noblepaul]

May be we need a bit more clarity on how to use this cache can be used by a core

[magibney]

May be we need a bit more clarity on how to use this cache can be used by a core

Yes, sorry. Ideally this would be built out with more test cases, but for now I went light on tests for the proof-of-concept/reference-implementation ThinCache, as it was intended at this point mainly to illustrate the value of the "node-level cache" approach.

You'd configure the node-level backing cache in solr.xml:

<solr>
  <caches>
    <cache name="myNodeLevelCache"
      class="solr.ThinCache$NodeLevelCache"
      maxRamMB="1024"
      />
  </caches>
</solr>

... and the missing piece is that you'd configure the ThinCache in the same way you would any core-level cache (in solrconfig.xml), providing the name of the node-level backing cache as a config attribute:

        <filterCache class="solr.ThinCache"
            nodeLevelCacheName="myNodeLevelCache"
            autowarmCount="10"
            />

NodeLevelCache extends solr.CaffeineCache, and can take any config param that CaffeineCache takes (wrt eviction policies, etc., though due to having a different lifecycle than a core-based cache, "autowarm" is not directly applicable at the node level).

ThinCache has basically two params: nodeLevelCacheName, and autowarmCount, because autowarm is the one cache attribute that's still evaluated at the per-core level.

[noblepaul]

@magibney

nodeLevelCacheName="myNodeLevelCache"

Is this support added ?

[magibney]

Is this support added ?

Yes. ThinCache class (which is suppressed as a new file with relatively many lines in the diff in github) should be a complete working implementation of the behavior we're discussing. It's a bit straightforward/naive in its current state, but enough to demonstrate the approach enabled by node-level cache and cache initialization.

Class javadocs for ThinCache describe its usage decently well.

[noblepaul]

is globalCache a better name ?

[magibney]

Sure; or perhaps parentCache or backingCache, since those express the relationship between "this" ThinCache and the named cache, which is arguably more useful?

[magibney]

There are several possibilities with how to proceed here:

1. We consider node-level cache support to be fundamentally valuable enough in its own right to commit this as a "hook", initially without the ThinCache proof-of-concept that leverages the functionality, updating the refguide to cover node-level cache configuration.
2. We proceed to commit this with the current proof-of-concept ThinCache implementation (building out tests and refguide accordingly).
3. We close this PR, and come back to it when it can be introduced alongside a less naive implementation leveraging this functionality.

My preferred approach would be the first of these: to revert 916806b39e0f39424fec0a6946e85486a65fc422 (the introduction of the ThinCache proof-of-concept) and only commit the minor infrastructural changes that enable configuring node-level caches, and enable searcher-aware core caches (a prerequisite for core/searcher-scoped keys at the node level). The benefit of this approach would be that it would support independent experimentation with pluggable cache impls leveraging a node-level approach, allowing implementations to be more fully developed before if/when being contributed to Solr core.

The downside of option 2 would be that if people start using the naive ThinCache proof-of-concept, it makes upgrade paths more challenging (and comes with the risk of presenting a relatively new implementation when there's very little downside to providing the opportunity to approach the addition of new cache impls in a more deliberate way). The pluggable nature of the ThinCache impl means that anyone who wants to run it could easily compile it themselves (as a plugin).

Option 3 is totally fine too, and would probably be my second choice (after option 1). The upside of this approach would be that if the interface needs to change (for some reason?) to support a more robust cache implementation, the node-level and searcher-aware changes could arrive more fully-formed (though tbh what's needed here is quite straightforward and I'd be very surprised if it needed to change). The downside of option 3 is that (obviously) folks who wanted to experiment with this approach would have to do so on a custom branch of Solr, making experimentation with a node-level cache approach more difficult for many people (and potentially resulting in divergent approaches to common configuration, which would need to be reconciled).

[dsmiley]

Option 1 looks good. Hopefully something simple will come for users eventually but I can see it maybe isn't ready yet.

[noblepaul]

If we go with # 1 , we do not have a way to use this feature at all. OTOH if we go with # 2 , atleast somebody will use it

[magibney]

It just depends on what one considers "the feature" to be. Option 1 considers the feature to be "standard hooks to support the implementation of pluggable node-level caches". That's no small thing, and I definitely think there would be folks interested in leveraging that feature.

There are some features/concrete implementations that simply can't be developed in a pluggable way, or where what's needed in terms of hooks to enable pluggability is not immediately clear in the absence of a concrete implementation.

I'd argue that neither is the case here. The interfaces needed to support the desired functionality are clear, and introducing those interfaces will enable interested parties to iterate on cache designs, and contribute those implementations to core once they're robust enough to be less a risk of becoming a backcompat headache. We still see people using solr.LFUCache and solr.LRUCache now, simply because their configs haven't been updated to take advantage of the new default.

[magibney]

I'm proposing to go with "Option 4", which hadn't occurred to me at the time I wrote the above comment:

Move ThinCache impl to Solr test codebase. The idea is to allow for "Option 1", but with a proof-of-concept implementation to:

1. Allow for testing the new hooks
2. Demonstrate how this could be useful
3. Use as a starting point for plugin implementations

... all without the back-compat risk of premature adoption of a concrete cache implementation.

[justinrsweeney]

I like Option 4, I think having the hook added as its own PR + demonstration of use is enough for a single PR and opens up the ability to use this via plugins.

If we were to go the route of supporting implementations, I think that could be a follow on PR because it opens up a different discussion of what are good supported implementations to be baked into Solr. I think the hook alone has clear value and should be merged as its own PR.

[dsmiley]

+1 Option 4.

[magibney]

I'm still pursuing the mailing list discussion -- ideally I would not have had to force-push, but I didn't want to hold this PR up any further.

Pending any feedback and/or CI test runs, etc., I plan to merge this tomorrow.

[magibney]

Ah, just noticed the test suite is not quite ready to commit.

[magibney]

Just wanted to call this out -- am I missing something, is there a better way to do this? I'd be surprised if this were the first time anyone wanted to actually build and query a core in a way that calls for custom functionality in solr.xml ... but I haven't been able to find any precedent.

There's lots of precedent for directly building a NodeConfig; but it looks like the way TestHarness builds its NodeConfig (like, to support actual indexing and querying) ignores solr.xml here in buildTestNodeConfig().

Aside from this last minor question, this should now be ready to commit.

[dsmiley]

FYI I'm trying to get us off of using TestHarness; it's very legacy. I recommend avoiding it for new tests.
https://issues.apache.org/jira/browse/SOLR-11872

[magibney]

Sorry about that! I'm curious, if it's not too much trouble: is there a test you'd recommend as demonstrating a more modern pattern -- particularly one that modifies solr.xml and goes all the way through indexing/querying? It was hard enough for me to track down any way to do this at all, let alone the right way.

I don't mind circling back to improve this, since it's in the front of my mind now.

[dsmiley]

See #1886