Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SOLR-16966: Add a first-class cache for OrdinalMaps #1899

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

magibney
Copy link
Contributor

@magibney magibney commented Sep 7, 2023

See: SOLR-16966

@magibney
Copy link
Contributor Author

magibney commented Sep 7, 2023

Tests are not built out yet; I wanted first to solicit feedback on the general approach, particularly around allowing entries to age out by tracking access time in cache entry metadata.

EDIT: now has a test suite; should be basically ready to commit, pending any feedback.

@magibney
Copy link
Contributor Author

magibney commented Sep 12, 2023

I considered adding the regenKeepAlive param to CaffeineCache more generally, but decided against it because:

  1. it makes things somewhat more complicated, and
  2. the problem that it addresses is quite particular to relatively small, low-turnover caches -- e.g., ordMapCache.

I'm hoping to commit this soon -- any feedback welcome!

@magibney
Copy link
Contributor Author

Proposed CHANGES.txt entry under Improvements for 9.4.0 release:

* SOLR-16966: Add a first-class cache for OrdinalMaps (Michael Gibney)

I plan to commit this early next week, pending feedback.

@magibney
Copy link
Contributor Author

The last three commits (through b812467) introduce a subtle but substantial change that abstracts all the "metadata wrapper" accounting so that application code (SolrIndexSearcher, other users of caches) know nothing about any potential metadata wrappers around cache value entries.

The cleanest place to do this is from the regenerator; I've added wrap() and unwrap() methods to CacheRegenerator, with a default passthrough implementation. Regenerators that require value-entry metadata accounting (such as access timestamp, individual entry hit count, etc.) may override wrap() to return a simple SolrCache<K, V> view of an underlying SolrCache<K, MetadataWrapper<V>> cache. It is the raw cache (unwrapped) that is autowarmed, providing the regenerator with the information it needs to make warming decisions, etc. But it is the wrapped cache that is presented for all normal cache interactions.

This approach works equally well for implementing more nuanced cache metrics (histograms, dumping cache contents, etc.). The overall approach is described in the javadocs here. It should be quite trivial to implement regenerators that implement custom metrics.

A wrapping instance will always know that it's wrapping, and be
capable of directly returning the backing (wrapped) cache via
SolrCache.toInternal().
Copy link

This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the [email protected] mailing list. Thank you for your contribution!

@github-actions github-actions bot added the stale PR not updated in 60 days label Jan 27, 2024
Copy link

github-actions bot commented Oct 7, 2024

This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again.

@github-actions github-actions bot added the closed-stale Closed after being stale for 60 days label Oct 7, 2024
@github-actions github-actions bot closed this Oct 7, 2024
@magibney magibney reopened this Oct 11, 2024
@github-actions github-actions bot added documentation Improvements or additions to documentation module:hdfs tests cat:search labels Oct 11, 2024
@github-actions github-actions bot added cat:index cat:schema module:hdfs tests cat:search and removed closed-stale Closed after being stale for 60 days stale PR not updated in 60 days labels Oct 11, 2024
Copy link

This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution!

@github-actions github-actions bot added the stale PR not updated in 60 days label Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cat:index cat:schema cat:search documentation Improvements or additions to documentation module:hdfs stale PR not updated in 60 days tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant