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

Simplify searchable snapshot CacheKey #66263

Merged
merged 2 commits into from
Dec 14, 2020
Merged

Simplify searchable snapshot CacheKey #66263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d286856
Simplify CacheKey
tlrx Dec 14, 2020
05a8925
nits
tlrx Dec 14, 2020
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
2 changes: 1 addition & 1 deletion ...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 @@ -344,7 +344,7 @@ protected IndexInputStats createIndexInputStats(final long fileLength) {
}

public CacheKey createCacheKey(String fileName) {
return new CacheKey(snapshotId, indexId, shardId, fileName);
return new CacheKey(snapshotId.getUUID(), indexId.getName(), shardId, fileName);
}

public CacheFile getCacheFile(CacheKey cacheKey, long fileLength) throws Exception {
Expand Down
36 changes: 21 additions & 15 deletions ...ugin/searchable-snapshots/src/main/java/org/elasticsearch/index/store/cache/CacheKey.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,29 @@
package org.elasticsearch.index.store.cache;

import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;

import java.util.Objects;

public class CacheKey {

private final SnapshotId snapshotId;
private final IndexId indexId;
private final String snapshotUUID;
private final String snapshotIndexName;
private final ShardId shardId;
private final String fileName;

public CacheKey(SnapshotId snapshotId, IndexId indexId, ShardId shardId, String fileName) {
this.snapshotId = Objects.requireNonNull(snapshotId);
this.indexId = Objects.requireNonNull(indexId);
public CacheKey(String snapshotUUID, String snapshotIndexName, ShardId shardId, String fileName) {
this.snapshotUUID = Objects.requireNonNull(snapshotUUID);
this.snapshotIndexName = Objects.requireNonNull(snapshotIndexName);
this.shardId = Objects.requireNonNull(shardId);
this.fileName = Objects.requireNonNull(fileName);
}

public SnapshotId getSnapshotId() {
return snapshotId;
public String getSnapshotUUID() {
return snapshotUUID;
}

public IndexId getIndexId() {
return indexId;
public String getSnapshotIndexName() {
return snapshotIndexName;
}

public ShardId getShardId() {
Expand All @@ -50,19 +48,27 @@ public boolean equals(Object o) {
return false;
}
final CacheKey cacheKey = (CacheKey) o;
return Objects.equals(snapshotId, cacheKey.snapshotId)
&& Objects.equals(indexId, cacheKey.indexId)
return Objects.equals(snapshotUUID, cacheKey.snapshotUUID)
&& Objects.equals(snapshotIndexName, cacheKey.snapshotIndexName)
&& Objects.equals(shardId, cacheKey.shardId)
&& Objects.equals(fileName, cacheKey.fileName);
}

@Override
public int hashCode() {
return Objects.hash(snapshotId, indexId, shardId, fileName);
return Objects.hash(snapshotUUID, snapshotIndexName, shardId, fileName);
}

@Override
public String toString() {
return "[" + "snapshotId=" + snapshotId + ", indexId=" + indexId + ", shardId=" + shardId + ", fileName='" + fileName + "']";
return "[snapshotUUID="
+ snapshotUUID
+ ", snapshotIndexName="
+ snapshotIndexName
+ ", shardId="
+ shardId
+ ", fileName='"
+ fileName
+ "']";
}
}
30 changes: 16 additions & 14 deletions ...apshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/CacheService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ public void removeFromCache(final CacheKey cacheKey) {
* @param shardId the {@link SnapshotId}
*/
public void markShardAsEvictedInCache(SnapshotId snapshotId, IndexId indexId, ShardId shardId) {
final ShardEviction shardEviction = new ShardEviction(snapshotId, indexId, shardId);
final ShardEviction shardEviction = new ShardEviction(snapshotId.getUUID(), indexId.getName(), shardId);
if (evictedShards.add(shardEviction)) {
threadPool.generic().submit(new AbstractRunnable() {
@Override
Expand Down Expand Up @@ -350,7 +350,7 @@ public void onFailure(Exception e) {
* @param runnable a runnable to execute
*/
public void runIfShardMarkedAsEvictedInCache(SnapshotId snapshotId, IndexId indexId, ShardId shardId, Runnable runnable) {
runIfShardMarkedAsEvictedInCache(new ShardEviction(snapshotId, indexId, shardId), runnable);
runIfShardMarkedAsEvictedInCache(new ShardEviction(snapshotId.getUUID(), indexId.getName(), shardId), runnable);
}

/**
Expand Down Expand Up @@ -458,7 +458,9 @@ protected void synchronizeCache() {
assert value >= 0 : value;

final CacheKey cacheKey = cacheFile.getCacheKey();
if (evictedShards.contains(new ShardEviction(cacheKey.getSnapshotId(), cacheKey.getIndexId(), cacheKey.getShardId()))) {
if (evictedShards.contains(
new ShardEviction(cacheKey.getSnapshotUUID(), cacheKey.getSnapshotIndexName(), cacheKey.getShardId())
)) {
logger.debug("cache file belongs to a shard marked as evicted, skipping synchronization for [{}]", cacheKey);
continue;
}
Expand Down Expand Up @@ -549,13 +551,13 @@ public String toString() {
*/
private static class ShardEviction {

private final SnapshotId snapshotId;
private final IndexId indexId;
private final String snapshotUUID;
private final String snapshotIndexName;
private final ShardId shardId;

private ShardEviction(SnapshotId snapshotId, IndexId indexId, ShardId shardId) {
this.snapshotId = snapshotId;
this.indexId = indexId;
private ShardEviction(String snapshotUUID, String snapshotIndexName, ShardId shardId) {
this.snapshotUUID = snapshotUUID;
this.snapshotIndexName = snapshotIndexName;
this.shardId = shardId;
}

Expand All @@ -564,24 +566,24 @@ public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ShardEviction that = (ShardEviction) o;
return Objects.equals(snapshotId, that.snapshotId)
&& Objects.equals(indexId, that.indexId)
return Objects.equals(snapshotUUID, that.snapshotUUID)
&& Objects.equals(snapshotIndexName, that.snapshotIndexName)
&& Objects.equals(shardId, that.shardId);
}

@Override
public int hashCode() {
return Objects.hash(snapshotId, indexId, shardId);
return Objects.hash(snapshotUUID, snapshotIndexName, shardId);
}

@Override
public String toString() {
return "[snapshotId=" + snapshotId + ", indexId=" + indexId + ", shardId=" + shardId + ']';
return "[snapshotUUID=" + snapshotUUID + ", snapshotIndexName=" + snapshotIndexName + ", shardId=" + shardId + ']';
}

boolean matches(CacheKey cacheKey) {
return Objects.equals(snapshotId, cacheKey.getSnapshotId())
&& Objects.equals(indexId, cacheKey.getIndexId())
return Objects.equals(snapshotUUID, cacheKey.getSnapshotUUID())
&& Objects.equals(snapshotIndexName, cacheKey.getSnapshotIndexName())
&& Objects.equals(shardId, cacheKey.getShardId());
}
}
Expand Down
22 changes: 5 additions & 17 deletions ...hots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/PersistentCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,7 @@ public String toString() {
private static final String CACHE_PATH_FIELD = "cache_path";
private static final String CACHE_RANGES_FIELD = "cache_ranges";
private static final String SNAPSHOT_ID_FIELD = "snapshot_id";
private static final String SNAPSHOT_NAME_FIELD = "snapshot_name";
private static final String INDEX_ID_FIELD = "index_id";
private static final String INDEX_NAME_FIELD = "index_name";
private static final String SNAPSHOT_INDEX_NAME_FIELD = "index_name";
private static final String SHARD_INDEX_NAME_FIELD = "shard_index_name";
private static final String SHARD_INDEX_ID_FIELD = "shard_index_id";
private static final String SHARD_ID_FIELD = "shard_id";
Expand All @@ -503,10 +501,6 @@ private static String buildId(Path path) {
return path.getFileName().toString();
}

private static Term buildTerm(CacheFile cacheFile) {
return buildTerm(buildId(cacheFile));
}

private static Term buildTerm(String cacheFileUuid) {
return new Term(CACHE_ID_FIELD, cacheFileUuid);
}
Expand All @@ -530,14 +524,8 @@ private static Document buildDocument(NodeEnvironment.NodePath nodePath, CacheFi
final CacheKey cacheKey = cacheFile.getCacheKey();
document.add(new StringField(FILE_NAME_FIELD, cacheKey.getFileName(), Field.Store.YES));
document.add(new StringField(FILE_LENGTH_FIELD, Long.toString(cacheFile.getLength()), Field.Store.YES));

final SnapshotId snapshotId = cacheKey.getSnapshotId();
document.add(new StringField(SNAPSHOT_NAME_FIELD, snapshotId.getName(), Field.Store.YES));
document.add(new StringField(SNAPSHOT_ID_FIELD, snapshotId.getUUID(), Field.Store.YES));

final IndexId indexId = cacheKey.getIndexId();
document.add(new StringField(INDEX_NAME_FIELD, indexId.getName(), Field.Store.YES));
document.add(new StringField(INDEX_ID_FIELD, indexId.getId(), Field.Store.YES));
document.add(new StringField(SNAPSHOT_ID_FIELD, cacheKey.getSnapshotUUID(), Field.Store.YES));
document.add(new StringField(SNAPSHOT_INDEX_NAME_FIELD, cacheKey.getSnapshotIndexName(), Field.Store.YES));

final ShardId shardId = cacheKey.getShardId();
document.add(new StringField(SHARD_INDEX_NAME_FIELD, shardId.getIndex().getName(), Field.Store.YES));
Expand All @@ -555,8 +543,8 @@ private static String getValue(Document document, String fieldName) {

private static CacheKey buildCacheKey(Document document) {
return new CacheKey(
new SnapshotId(getValue(document, SNAPSHOT_NAME_FIELD), getValue(document, SNAPSHOT_ID_FIELD)),
new IndexId(getValue(document, INDEX_NAME_FIELD), getValue(document, INDEX_ID_FIELD)),
getValue(document, SNAPSHOT_ID_FIELD),
getValue(document, SNAPSHOT_INDEX_NAME_FIELD),
new ShardId(
new Index(getValue(document, SHARD_INDEX_NAME_FIELD), getValue(document, SHARD_INDEX_ID_FIELD)),
Integer.parseInt(getValue(document, SHARD_ID_FIELD))
Expand Down
13 changes: 3 additions & 10 deletions ...earchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CacheFileTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.index.store.cache.CacheFile.EvictionListener;
import org.elasticsearch.index.store.cache.TestUtils.FSyncTrackingFileSystemProvider;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.threadpool.ThreadPool;
import org.hamcrest.Matcher;
Expand Down Expand Up @@ -49,17 +47,12 @@
public class CacheFileTests extends ESTestCase {

private static final Runnable NOOP = () -> {};
private static final CacheKey CACHE_KEY = new CacheKey(
new SnapshotId("_name", "_uuid"),
new IndexId("_name", "_uuid"),
new ShardId("_name", "_uuid", 0),
"_filename"
);
private static final CacheKey CACHE_KEY = new CacheKey("_snap_uuid", "_snap_index", new ShardId("_name", "_uuid", 0), "_filename");

public void testGetCacheKey() throws Exception {
final CacheKey cacheKey = new CacheKey(
new SnapshotId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
new IndexId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
UUIDs.randomBase64UUID(random()),
randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
new ShardId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random()), randomInt(5)),
randomAlphaOfLength(105).toLowerCase(Locale.ROOT)
);
Expand Down
35 changes: 14 additions & 21 deletions ...searchable-snapshots/src/test/java/org/elasticsearch/index/store/cache/CacheKeyTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,63 +5,56 @@
*/
package org.elasticsearch.index.store.cache;

import org.elasticsearch.common.UUIDs;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.shard.ShardId;
import org.elasticsearch.repositories.IndexId;
import org.elasticsearch.snapshots.SnapshotId;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.test.EqualsHashCodeTestUtils;

import java.util.Locale;

public class CacheKeyTests extends ESTestCase {

public void testEqualsAndHashCode() {
EqualsHashCodeTestUtils.checkEqualsAndHashCode(createInstance(), this::copy, this::mutate);
}

private SnapshotId randomSnapshotId() {
return new SnapshotId(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10));
private String randomSnapshotUUID() {
return UUIDs.randomBase64UUID(random());
}

private IndexId randomIndexId() {
return new IndexId(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10));
private String randomSnapshotIndexName() {
return randomAlphaOfLengthBetween(5, 10).toLowerCase(Locale.ROOT);
}

private ShardId randomShardId() {
return new ShardId(randomAlphaOfLengthBetween(5, 10), randomAlphaOfLengthBetween(5, 10), randomInt(5));
}

private CacheKey createInstance() {
return new CacheKey(randomSnapshotId(), randomIndexId(), randomShardId(), randomAlphaOfLengthBetween(5, 10));
return new CacheKey(randomSnapshotUUID(), randomSnapshotIndexName(), randomShardId(), randomAlphaOfLengthBetween(5, 10));
}

private CacheKey copy(final CacheKey origin) {
SnapshotId snapshotId = origin.getSnapshotId();
if (randomBoolean()) {
snapshotId = new SnapshotId(origin.getSnapshotId().getName(), origin.getSnapshotId().getUUID());
}
IndexId indexId = origin.getIndexId();
if (randomBoolean()) {
indexId = new IndexId(origin.getIndexId().getName(), origin.getIndexId().getId());
}
ShardId shardId = origin.getShardId();
if (randomBoolean()) {
shardId = new ShardId(new Index(shardId.getIndex().getName(), shardId.getIndex().getUUID()), shardId.id());
}
return new CacheKey(snapshotId, indexId, shardId, origin.getFileName());
return new CacheKey(origin.getSnapshotUUID(), origin.getSnapshotIndexName(), shardId, origin.getFileName());
}

private CacheKey mutate(CacheKey origin) {
SnapshotId snapshotId = origin.getSnapshotId();
IndexId indexId = origin.getIndexId();
String snapshotUUID = origin.getSnapshotUUID();
String snapshotIndexName = origin.getSnapshotIndexName();
ShardId shardId = origin.getShardId();
String fileName = origin.getFileName();

switch (randomInt(3)) {
case 0:
snapshotId = randomValueOtherThan(origin.getSnapshotId(), this::randomSnapshotId);
snapshotUUID = randomValueOtherThan(snapshotUUID, this::randomSnapshotUUID);
break;
case 1:
indexId = randomValueOtherThan(origin.getIndexId(), this::randomIndexId);
snapshotIndexName = randomValueOtherThan(snapshotIndexName, this::randomSnapshotIndexName);
break;
case 2:
shardId = randomValueOtherThan(origin.getShardId(), this::randomShardId);
Expand All @@ -72,6 +65,6 @@ private CacheKey mutate(CacheKey origin) {
default:
throw new AssertionError("Unsupported mutation");
}
return new CacheKey(snapshotId, indexId, shardId, fileName);
return new CacheKey(snapshotUUID, snapshotIndexName, shardId, fileName);
}
}
17 changes: 9 additions & 8 deletions ...ts/src/test/java/org/elasticsearch/xpack/searchablesnapshots/cache/CacheServiceTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ public void testCacheSynchronization() throws Exception {
assumeFalse("https://github.com/elastic/elasticsearch/issues/65543", Constants.WINDOWS);
final int numShards = randomIntBetween(1, 3);
final Index index = new Index(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random()));
final SnapshotId snapshotId = new SnapshotId("_snapshot_name", UUIDs.randomBase64UUID(random()));
final IndexId indexId = new IndexId("_index_name", UUIDs.randomBase64UUID(random()));
final String snapshotUUID = UUIDs.randomBase64UUID(random());
final String snapshotIndexName = UUIDs.randomBase64UUID(random());

logger.debug("--> creating shard cache directories on disk");
final Path[] shardsCacheDirs = new Path[numShards];
Expand All @@ -76,7 +76,7 @@ public void testCacheSynchronization() throws Exception {
assertFalse(Files.exists(shardDataPath));

logger.debug("--> creating directories [{}] for shard [{}]", shardDataPath.toAbsolutePath(), i);
shardsCacheDirs[i] = Files.createDirectories(resolveSnapshotCache(shardDataPath).resolve(snapshotId.getUUID()));
shardsCacheDirs[i] = Files.createDirectories(CacheService.resolveSnapshotCache(shardDataPath).resolve(snapshotUUID));
}

try (CacheService cacheService = defaultCacheService()) {
Expand Down Expand Up @@ -112,7 +112,7 @@ public void testCacheSynchronization() throws Exception {
for (int i = 0; i < between(1, 25); i++) {
final ShardId shardId = new ShardId(index, randomIntBetween(0, numShards - 1));
final String fileName = String.format(Locale.ROOT, "file_%d_%d", iteration, i);
final CacheKey cacheKey = new CacheKey(snapshotId, indexId, shardId, fileName);
final CacheKey cacheKey = new CacheKey(snapshotUUID, snapshotIndexName, shardId, fileName);
final CacheFile cacheFile = cacheService.get(cacheKey, randomIntBetween(0, 10_000), shardsCacheDirs[shardId.id()]);

final CacheFile.EvictionListener listener = evictedCacheFile -> {};
Expand Down Expand Up @@ -180,14 +180,14 @@ public void testPut() throws Exception {
try (CacheService cacheService = defaultCacheService()) {
final long fileLength = randomLongBetween(0L, 1000L);
final CacheKey cacheKey = new CacheKey(
new SnapshotId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
new IndexId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random())),
UUIDs.randomBase64UUID(random()),
randomAlphaOfLength(5).toLowerCase(Locale.ROOT),
new ShardId(randomAlphaOfLength(5).toLowerCase(Locale.ROOT), UUIDs.randomBase64UUID(random()), randomInt(5)),
randomAlphaOfLength(105).toLowerCase(Locale.ROOT)
);

final Path cacheDir = Files.createDirectories(
resolveSnapshotCache(randomShardPath(cacheKey.getShardId())).resolve(cacheKey.getSnapshotId().getUUID())
resolveSnapshotCache(randomShardPath(cacheKey.getShardId())).resolve(cacheKey.getSnapshotUUID())
);
final String cacheFileUuid = UUIDs.randomBase64UUID(random());
final SortedSet<Tuple<Long, Long>> cacheFileRanges = randomBoolean() ? randomRanges(fileLength) : emptySortedSet();
Expand Down Expand Up @@ -240,7 +240,8 @@ public void testRunIfShardMarkedAsEvictedInCache() throws Exception {
final PlainActionFuture<Void> waitForEviction = PlainActionFuture.newFuture();
final CacheFile.EvictionListener evictionListener = evicted -> waitForEviction.onResponse(null);

final CacheFile cacheFile = cacheService.get(new CacheKey(snapshotId, indexId, shardId, "_0.dvd"), 100, cacheDir);
final CacheKey cacheKey = new CacheKey(snapshotId.getUUID(), indexId.getName(), shardId, "_0.dvd");
final CacheFile cacheFile = cacheService.get(cacheKey, 100, cacheDir);
cacheFile.acquire(evictionListener);

cacheService.markShardAsEvictedInCache(snapshotId, indexId, shardId);
Expand Down
Loading