Fewer ThreadLocals in version/seqno resolver #85896
Conversation
Today we create a new `ThreadLocal` for every `IndexReader` encountered during non-append-only indexing (effectively every refresh on every shard). Each thread keeps a table of all its live thread-locals, where "live" effectively means "has not been garbage-collected". Some of these `IndexReader` instances have lifespans long enough for the corresponding thread-local to escape the young generation which means that it may not be garbage-collected for a very long time, preventing their slots in the table from being re-used. This causes the thread-local table for `write` threads to grow rather large, which egregiously slows down other operations on unrelated thread-locals on these threads. This commit changes the map-of-thread-locals to a thread-local-of-maps, reducing the churn of thread-locals and releasing the resolver in each thread's thread-local map as soon as the corresponding `IndexReader` is closed. Closes elastic#56766
DaveCTurner
added
>bug
:Distributed/CRUD
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Hi @DaveCTurner, I've created a changelog YAML for you. |
| assert lookupState[leaf.ord] == null; | ||
| lookupState[leaf.ord] = new PerThreadIDVersionAndSeqNoLookup(leaf.reader(), uidField); | ||
| } | ||
| cacheHelper.addClosedListener(this::removeLookup); |
There was a problem hiding this comment.
NB this may have performance impact since it means on close (i.e. refresh) we remove the entry from the map of every thread that touched it. Previously we'd have done most of that cleanup later on during GC.
| } | ||
| ctl.set(lookupState); | ||
| } | ||
| private final Map<IndexReader.CacheKey, PerThreadIDVersionAndSeqNoLookup[]> lookupsByReader = ConcurrentCollections |
There was a problem hiding this comment.
Not sure about using a CHM here, nor about its exact config. We only put and read on the owning thread, the thread-safety is just needed for the cleanup operations which should be much less common. Maybe a synchronized map would be fine/better?
|
@elasticmachine please run elasticsearch-ci/bwc (don't think it actually failed, looks like something went wrong after completion somehow) |
Today we create a new
ThreadLocalfor everyIndexReaderencounteredduring non-append-only indexing (effectively every refresh on every
shard). Each thread keeps a table of all its live thread-locals, where
"live" effectively means "has not been garbage-collected". Some of these
IndexReaderinstances have lifespans long enough for the correspondingthread-local to escape the young generation which means that it may not
be garbage-collected for a very long time, preventing their slots in the
table from being re-used. This causes the thread-local table for
writethreads to grow rather large, which egregiously slows down other
operations on unrelated thread-locals on these threads.
This commit changes the map-of-thread-locals to a thread-local-of-maps,
reducing the churn of thread-locals and releasing the resolver in each
thread's thread-local map as soon as the corresponding
IndexReaderisclosed.
Closes #56766