Skip to content

SearchableSnapshotDirectory should not evict cache files when closed#66264

Merged
tlrx merged 3 commits intoelastic:7.xfrom
tlrx:use-listeners-to-manage-cache-files-for-removed-shards-7.x
Dec 14, 2020
Merged

SearchableSnapshotDirectory should not evict cache files when closed#66264
tlrx merged 3 commits intoelastic:7.xfrom
tlrx:use-listeners-to-manage-cache-files-for-removed-shards-7.x

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Dec 14, 2020

This commit changes the SearchableSnapshotDirectory so
that it does not evict all its cache files at closing time, but
instead delegates this work to the CacheService.

This change is motivated by the fact that Lucene directories
are closed as the consequence of applying a new cluster
state and as such the closing is executed within the cluster
state applier thread; and we want to minimize disk IO
operations in such thread (like deleting a lot of evicted
cache files). It is also motivated by the future of the
searchable snapshot cache which should become persistent.

This change is built on top of the existing
SearchableSnapshotIndexEventListener and a new
SearchableSnapshotIndexFoldersDeletionListener
(see #65926) that are used to detect when a searchable
snapshot index (or searchable snapshot shard) is
removed from a data node.

When such a thing happens, the listeners notify the
CacheService that maintains an internal list of removed
shards. This list is used to evict the cache files associated
to these shards as soon as possible (but not in the cluster
state applier thread) or right before the same searchable
snapshot shard is being built again on the same node.

In other situations like opening/closing a searchable
snapshot shard then the cache files are not evicted
anymore and should be reused.

Backport of #66173 for 7.11

…lastic#66173)

This commit changes the SearchableSnapshotDirectory so
that it does not evict all its cache files at closing time, but
instead delegates this work to the CacheService.

This change is motivated by the fact that Lucene directories
are closed as the consequence of applying a new cluster
state and as such the closing is executed within the cluster
state applier thread; and we want to minimize disk IO
operations in such thread (like deleting a lot of evicted
cache files). It is also motivated by the future of the
searchable snapshot cache which should become persistent.

This change is built on top of the existing
SearchableSnapshotIndexEventListener and a new
 SearchableSnapshotIndexFoldersDeletionListener
(see elastic#65926) that are used to detect when a searchable
snapshot index (or searchable snapshot shard) is
removed from a data node.

When such a thing happens, the listeners notify the
 CacheService that maintains an internal list of removed
shards. This list is used to evict the cache files associated
to these shards as soon as possible (but not in the cluster
state applier thread) or right before the same searchable
snapshot shard is being built again on the same node.

In other situations like opening/closing a searchable
snapshot shard then the cache files are not evicted
anymore and should be reused.
@tlrx tlrx added >enhancement :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs backport v7.11.0 labels Dec 14, 2020
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Dec 14, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@tlrx tlrx merged commit 6046b1f into elastic:7.x Dec 14, 2020
@tlrx tlrx deleted the use-listeners-to-manage-cache-files-for-removed-shards-7.x branch December 14, 2020 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Distributed Meta label for distributed team. v7.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments