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.

Sign up for GitHub

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

Improve shards evictions in searchable snapshot cache service #67160

Merged
merged 19 commits into from
Jan 14, 2021
Merged

Improve shards evictions in searchable snapshot cache service #67160

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a48474a
Improve shards evictions in searchable snapshot cache service
tlrx Jan 7, 2021
fd1de88
Merge branch 'master' into improve-shards-evictions
tlrx Jan 8, 2021
44952bf
Merge branch 'master' into improve-shards-evictions
tlrx Jan 11, 2021
b5e5199
feedback
tlrx Jan 12, 2021
f9b9b2a
Merge branch 'master' into improve-shards-evictions
tlrx Jan 12, 2021
06f24af
Merge branch 'master' into improve-shards-evictions
tlrx Jan 12, 2021
9745747
fixes
tlrx Jan 13, 2021
0a0b260
always throw
tlrx Jan 13, 2021
8ad36ba
Merge branch 'master' into improve-shards-evictions
tlrx Jan 14, 2021
4e32df5
private
tlrx Jan 14, 2021
f2c6e21
assert generic
tlrx Jan 14, 2021
29f30fa
waitForPendingEvictions
tlrx Jan 14, 2021
8a6a536
testRepopulateCachevvvvv
tlrx Jan 14, 2021
5293eca
exists
tlrx Jan 14, 2021
b9cfded
tests
tlrx Jan 14, 2021
6c29e73
remove pendingFutures.isEmpty() == false
tlrx Jan 14, 2021
ba8a029
use mutex
tlrx Jan 14, 2021
1e5c965
unlock if success
tlrx Jan 14, 2021
aae2f78
deletions
tlrx Jan 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions ...csearch/xpack/searchablesnapshots/cache/SearchableSnapshotsPersistentCacheIntegTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ public void testCacheSurviveRestart() throws Exception {
}
assertFalse("no cache files found", cacheFiles.isEmpty());

CacheService cacheService = internalCluster().getInstance(CacheService.class, dataNode);
final CacheService cacheService = internalCluster().getInstance(CacheService.class, dataNode);
cacheService.synchronizeCache();

PersistentCache persistentCache = cacheService.getPersistentCache();
final PersistentCache persistentCache = cacheService.getPersistentCache();
assertThat(persistentCache.getNumDocs(), equalTo((long) cacheFiles.size()));

internalCluster().restartNode(dataNode, new InternalTestCluster.RestartCallback() {
Expand All @@ -142,21 +142,19 @@ public Settings onNodeStopped(String nodeName) {
}
});

cacheService = internalCluster().getInstance(CacheService.class, dataNode);
persistentCache = cacheService.getPersistentCache();
final CacheService cacheServiceAfterRestart = internalCluster().getInstance(CacheService.class, dataNode);
final PersistentCache persistentCacheAfterRestart = cacheServiceAfterRestart.getPersistentCache();
ensureGreen(restoredIndexName);

cacheFiles.forEach(cacheFile -> assertTrue(cacheFile + " should have survived node restart", Files.exists(cacheFile)));
assertThat("Cache files should be repopulated in cache", persistentCache.getNumDocs(), equalTo((long) cacheFiles.size()));
assertThat("Cache files should be loaded in cache", persistentCacheAfterRestart.getNumDocs(), equalTo((long) cacheFiles.size()));

assertAcked(client().admin().indices().prepareDelete(restoredIndexName));

assertBusy(() -> {
cacheFiles.forEach(cacheFile -> assertFalse(cacheFile + " should have been cleaned up", Files.exists(cacheFile)));
assertTrue(internalCluster().getInstance(CacheService.class, dataNode).getPersistentCache().hasDeletions());
cacheServiceAfterRestart.synchronizeCache();
assertThat(persistentCacheAfterRestart.getNumDocs(), equalTo(0L));
});
cacheService.synchronizeCache();

assertThat(persistentCache.getNumDocs(), equalTo(0L));
}
}
6 changes: 3 additions & 3 deletions ...le-snapshots/src/main/java/org/elasticsearch/index/store/SearchableSnapshotDirectory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public boolean loadSnapshot(RecoveryState recoveryState, ActionListener<Void> pr
this.snapshot = snapshotSupplier.get();
this.loaded = true;
cleanExistingRegularShardFiles();
cleanExistingCacheFiles();
evictCacheFilesIfNeeded();
this.recoveryState = (SearchableSnapshotRecoveryState) recoveryState;
prewarmCache(preWarmListener);
}
Expand Down Expand Up @@ -431,9 +431,9 @@ private void cleanExistingRegularShardFiles() {
* Evicts all cache files associated to the current searchable snapshot shard in case a
* previous instance of that same shard has been marked as evicted on this node.
*/
private void cleanExistingCacheFiles() {
private void evictCacheFilesIfNeeded() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: rename to waitForPendingEvictions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I pushed 29f30fa

assert Thread.holdsLock(this);
cacheService.runIfShardMarkedAsEvictedInCache(snapshotId, indexId, shardId, this::clearCache);
cacheService.evictCacheFilesIfNeeded(snapshotId.getUUID(), indexId.getName(), shardId);
}

private void prewarmCache(ActionListener<Void> listener) {
Expand Down
14 changes: 2 additions & 12 deletions ...ava/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshotIndexEventListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,14 @@
import org.elasticsearch.index.translog.Translog;
import org.elasticsearch.index.translog.TranslogException;
import org.elasticsearch.indices.cluster.IndicesClusterStateService.AllocatedIndices.IndexRemovalReason;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.xpack.searchablesnapshots.cache.CacheService;

import java.nio.file.Path;

import static org.elasticsearch.index.store.SearchableSnapshotDirectory.unwrapDirectory;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_ID_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_NAME_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_SNAPSHOT_ID_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_SNAPSHOT_NAME_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.isSearchableSnapshotStore;

public class SearchableSnapshotIndexEventListener implements IndexEventListener {
Expand Down Expand Up @@ -116,14 +112,8 @@ public void beforeIndexRemoved(IndexService indexService, IndexRemovalReason rea

logger.debug("{} marking shard as evicted in searchable snapshots cache (reason: {})", shardId, reason);
cacheService.markShardAsEvictedInCache(
new SnapshotId(
SNAPSHOT_SNAPSHOT_NAME_SETTING.get(indexSettings.getSettings()),
SNAPSHOT_SNAPSHOT_ID_SETTING.get(indexSettings.getSettings())
),
new IndexId(
SNAPSHOT_INDEX_NAME_SETTING.get(indexSettings.getSettings()),
SNAPSHOT_INDEX_ID_SETTING.get(indexSettings.getSettings())
),
SNAPSHOT_SNAPSHOT_ID_SETTING.get(indexSettings.getSettings()),
SNAPSHOT_INDEX_NAME_SETTING.get(indexSettings.getSettings()),
shardId
);
}
Expand Down
14 changes: 2 additions & 12 deletions ...asticsearch/xpack/searchablesnapshots/SearchableSnapshotIndexFoldersDeletionListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,14 @@
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.plugins.IndexStorePlugin;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.xpack.searchablesnapshots.cache.CacheService;

import java.nio.file.Path;
import java.util.Objects;
import java.util.function.Supplier;

import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_ID_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_INDEX_NAME_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_SNAPSHOT_ID_SETTING;
import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshots.SNAPSHOT_SNAPSHOT_NAME_SETTING;

/**
* This {@link IndexStorePlugin.IndexFoldersDeletionListener} is called when an index folder or a shard folder is deleted from the disk. If
Expand Down Expand Up @@ -62,14 +58,8 @@ private void markShardAsEvictedInCache(ShardId shardId, IndexSettings indexSetti

logger.debug("{} marking shard as evicted in searchable snapshots cache (reason: cache files deleted from disk)", shardId);
cacheService.markShardAsEvictedInCache(
new SnapshotId(
SNAPSHOT_SNAPSHOT_NAME_SETTING.get(indexSettings.getSettings()),
SNAPSHOT_SNAPSHOT_ID_SETTING.get(indexSettings.getSettings())
),
new IndexId(
SNAPSHOT_INDEX_NAME_SETTING.get(indexSettings.getSettings()),
SNAPSHOT_INDEX_ID_SETTING.get(indexSettings.getSettings())
),
SNAPSHOT_SNAPSHOT_ID_SETTING.get(indexSettings.getSettings()),
SNAPSHOT_INDEX_NAME_SETTING.get(indexSettings.getSettings()),
shardId
);
}
Expand Down
1 change: 1 addition & 0 deletions ...pshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/SearchableSnapshots.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ public List<Setting<?>> getSettings() {
CacheService.SNAPSHOT_CACHE_SYNC_INTERVAL_SETTING,
CacheService.SNAPSHOT_CACHE_MAX_FILES_TO_SYNC_AT_ONCE_SETTING,
CacheService.SNAPSHOT_CACHE_SYNC_SHUTDOWN_TIMEOUT,
CacheService.SNAPSHOT_CACHE_SHARD_EVICTIONS_SHUTDOWN_TIMEOUT,
SearchableSnapshotEnableAllocationDecider.SEARCHABLE_SNAPSHOTS_ALLOCATE_ON_ROLLING_RESTART
);
}
Expand Down
Loading