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

dedicated storedFields executor experiment #225

Open
wants to merge 2 commits into
base: fs/branch_9_3
Choose a base branch
from

Conversation

magibney
Copy link
Collaborator

@magibney magibney commented Oct 4, 2024

This is a very simple change (best viewed ignoring whitespace) that simply wraps the parts of the code that instantiate ThreadLocal storedFields and delegates them to a dedicated executor.

Consolidating in this way, and with an executor that allows its threads to die (at least for initial POC) should all but eliminate the "ThreadLocal storedFields cartesian product accumulation over time" problem, to hold us over until we have non-ThreadLocal storedFields in solr 9.7.

I determined these locations by analyzing GCP profiler (to identify code that in practice instantiates ThreadLocal storedFields).

@patsonluk
Copy link
Collaborator

patsonluk commented Oct 7, 2024

Just curious! For the threadlocal usage, is it something buried deep in the lucene end (was reading this PR)? And that it's triggered by SolrIndexSearcher#doc?

Therefore your proposed change just invokes the doc within threads with shorter lifespan (now managed by storedFieldsExecute), so the threadlocals will not accumulate as badly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants