From f8636b1b99f6bc13d5f87fb9d49c71158c0ea345 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Mon, 14 May 2018 18:43:47 +0200 Subject: [PATCH 1/4] Move caching of the size of a directory to `StoreDirectory`. In spite of the existing caching, I have seen a number of nodes hot threads where one thread had been spending all its cpu on computing the size of a directory. I am proposing to move the computation of the size of the directory to `StoreDirectory` in order to skip recomputing the size of the directory if no changes have been made. This should help with users that have read-only indices, which is very common for time-based indices. --- .../common/util/SingleObjectCache.java | 5 + .../index/store/ByteSizeCachingDirectory.java | 144 ++++++++++++++++++ .../org/elasticsearch/index/store/Store.java | 54 ++----- .../store/ByteSizeCachingDirectoryTests.java | 77 ++++++++++ 4 files changed, 238 insertions(+), 42 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java create mode 100644 server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java diff --git a/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java b/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java index f3d710dab8c79..fb0d3951aa36e 100644 --- a/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java +++ b/server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java @@ -64,6 +64,11 @@ public T getOrRefresh() { return cached; } + /** Return the potentially stale cached entry. */ + protected final T getNoRefresh() { + return cached; + } + /** * Returns a new instance to cache */ diff --git a/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java new file mode 100644 index 0000000000000..62fe8d79895c6 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java @@ -0,0 +1,144 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.store; + +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FilterDirectory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexOutput; +import org.elasticsearch.common.lucene.store.FilterIndexOutput; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.common.util.SingleObjectCache; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.AccessDeniedException; +import java.nio.file.NoSuchFileException; +import java.util.concurrent.atomic.AtomicLong; + +final class ByteSizeCachingDirectory extends FilterDirectory { + + private static class SizeAndModCount { + final long size; + final long modCount; + + SizeAndModCount(long length, long modCount) { + this.size = length; + this.modCount = modCount; + } + } + + private static long estimateSize(Directory directory) throws IOException { + long estimatedSize = 0; + String[] files = directory.listAll(); + for (String file : files) { + try { + estimatedSize += directory.fileLength(file); + } catch (NoSuchFileException | FileNotFoundException | AccessDeniedException e) { + // ignore, the file is not there no more; on Windows, if one thread concurrently deletes a file while + // calling Files.size, you can also sometimes hit AccessDeniedException + } + } + return estimatedSize; + } + + private final AtomicLong modCount = new AtomicLong(); + private final SingleObjectCache size; + + ByteSizeCachingDirectory(Directory in, TimeValue refreshInterval) { + super(in); + size = new SingleObjectCache(refreshInterval, new SizeAndModCount(0L, 0L)) { + @Override + protected SizeAndModCount refresh() { + // Compute modCount first so that updates that happen while the size + // is being computed invalidate the length + final long modCount = ByteSizeCachingDirectory.this.modCount.get(); + final long size; + try { + size = estimateSize(getDelegate()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return new SizeAndModCount(size, modCount); + } + + @Override + protected boolean needsRefresh() { + if (getNoRefresh().modCount == modCount.get()) { + // no updates to the directory since the last refresh + return false; + } + return super.needsRefresh(); + } + }; + } + + /** Return the cumulative size of all files in this directory. */ + long estimateSize() throws IOException { + try { + return size.getOrRefresh().size; + } catch (UncheckedIOException e) { + // we wrapped in the cache and unwrap here + throw e.getCause(); + } + } + + @Override + public IndexOutput createOutput(String name, IOContext context) throws IOException { + return wrapIndexOutput(super.createOutput(name, context)); + } + + @Override + public IndexOutput createTempOutput(String prefix, String suffix, IOContext context) throws IOException { + return wrapIndexOutput(super.createTempOutput(prefix, suffix, context)); + } + + private IndexOutput wrapIndexOutput(IndexOutput out) { + return new FilterIndexOutput(out.toString(), out) { + @Override + public void writeBytes(byte[] b, int length) throws IOException { + super.writeBytes(b, length); + modCount.incrementAndGet(); + } + + @Override + public void writeByte(byte b) throws IOException { + super.writeByte(b); + modCount.incrementAndGet(); + } + + @Override + public void close() throws IOException { + // Close might cause some data to be flushed from in-memory buffers, so + // increment the modification counter too. + super.close(); + modCount.incrementAndGet(); + } + }; + } + + @Override + public void deleteFile(String name) throws IOException { + super.deleteFile(name); + modCount.incrementAndGet(); + } + +} diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java index de29386022cc6..fabf64cf7a16f 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -50,7 +50,6 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.BytesRefBuilder; import org.apache.lucene.util.Version; -import org.elasticsearch.ElasticsearchException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.common.UUIDs; import org.elasticsearch.common.bytes.BytesReference; @@ -67,7 +66,6 @@ import org.elasticsearch.common.settings.Setting.Property; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.common.util.SingleObjectCache; import org.elasticsearch.common.util.concurrent.AbstractRefCounted; import org.elasticsearch.common.util.concurrent.RefCounted; import org.elasticsearch.common.util.iterable.Iterables; @@ -91,7 +89,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; -import java.nio.file.AccessDeniedException; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.util.ArrayList; @@ -146,7 +143,6 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref private final ReentrantReadWriteLock metadataLock = new ReentrantReadWriteLock(); private final ShardLock shardLock; private final OnClose onClose; - private final SingleObjectCache statsCache; private final AbstractRefCounted refCounter = new AbstractRefCounted("store") { @Override @@ -164,12 +160,13 @@ public Store(ShardId shardId, IndexSettings indexSettings, DirectoryService dire OnClose onClose) throws IOException { super(shardId, indexSettings); final Settings settings = indexSettings.getSettings(); - this.directory = new StoreDirectory(directoryService.newDirectory(), Loggers.getLogger("index.store.deletes", settings, shardId)); - this.shardLock = shardLock; - this.onClose = onClose; + Directory dir = directoryService.newDirectory(); final TimeValue refreshInterval = indexSettings.getValue(INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING); - this.statsCache = new StoreStatsCache(refreshInterval, directory); logger.debug("store stats are refreshed with refresh_interval [{}]", refreshInterval); + ByteSizeCachingDirectory sizeCachingDir = new ByteSizeCachingDirectory(dir, refreshInterval); + this.directory = new StoreDirectory(sizeCachingDir, Loggers.getLogger("index.store.deletes", settings, shardId)); + this.shardLock = shardLock; + this.onClose = onClose; assert onClose != null; assert shardLock != null; @@ -377,7 +374,7 @@ public void exorciseIndex(CheckIndex.Status status) throws IOException { public StoreStats stats() throws IOException { ensureOpen(); - return statsCache.getOrRefresh(); + return new StoreStats(directory.estimateSize()); } /** @@ -731,11 +728,16 @@ static final class StoreDirectory extends FilterDirectory { private final Logger deletesLogger; - StoreDirectory(Directory delegateDirectory, Logger deletesLogger) throws IOException { + StoreDirectory(ByteSizeCachingDirectory delegateDirectory, Logger deletesLogger) throws IOException { super(delegateDirectory); this.deletesLogger = deletesLogger; } + /** Estimate the cumulative size of all files in this directory in bytes. */ + long estimateSize() throws IOException { + return ((ByteSizeCachingDirectory) getDelegate()).estimateSize(); + } + @Override public void close() throws IOException { assert false : "Nobody should close this directory except of the Store itself"; @@ -1428,38 +1430,6 @@ public void accept(ShardLock Lock) { }; } - private static class StoreStatsCache extends SingleObjectCache { - private final Directory directory; - - StoreStatsCache(TimeValue refreshInterval, Directory directory) throws IOException { - super(refreshInterval, new StoreStats(estimateSize(directory))); - this.directory = directory; - } - - @Override - protected StoreStats refresh() { - try { - return new StoreStats(estimateSize(directory)); - } catch (IOException ex) { - throw new ElasticsearchException("failed to refresh store stats", ex); - } - } - - private static long estimateSize(Directory directory) throws IOException { - long estimatedSize = 0; - String[] files = directory.listAll(); - for (String file : files) { - try { - estimatedSize += directory.fileLength(file); - } catch (NoSuchFileException | FileNotFoundException | AccessDeniedException e) { - // ignore, the file is not there no more; on Windows, if one thread concurrently deletes a file while - // calling Files.size, you can also sometimes hit AccessDeniedException - } - } - return estimatedSize; - } - } - /** * creates an empty lucene index and a corresponding empty translog. Any existing data will be deleted. */ diff --git a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java new file mode 100644 index 0000000000000..d1a46aa3d5ee0 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java @@ -0,0 +1,77 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.store; + +import org.apache.lucene.store.Directory; +import org.apache.lucene.store.FilterDirectory; +import org.apache.lucene.store.IOContext; +import org.apache.lucene.store.IndexOutput; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.test.ESTestCase; + +import java.io.IOException; + +public class ByteSizeCachingDirectoryTests extends ESTestCase { + + private static class LengthCountingDirectory extends FilterDirectory { + + int numFileLengthCalls; + + LengthCountingDirectory(Directory in) { + super(in); + } + + @Override + public long fileLength(String name) throws IOException { + numFileLengthCalls++; + return super.fileLength(name); + } + } + + public void testBasics() throws IOException { + try (Directory dir = newDirectory()) { + LengthCountingDirectory countingDir = new LengthCountingDirectory(dir); + ByteSizeCachingDirectory cachingDir = new ByteSizeCachingDirectory(countingDir, new TimeValue(0)); + assertEquals(0, cachingDir.estimateSize()); + assertEquals(0, cachingDir.estimateSize()); + assertEquals(0, countingDir.numFileLengthCalls); + + try (IndexOutput out = cachingDir.createOutput("foo", IOContext.DEFAULT)) { + out.writeBytes(new byte[5], 5); + } + + assertEquals(5, cachingDir.estimateSize()); + assertEquals(1, countingDir.numFileLengthCalls); + assertEquals(5, cachingDir.estimateSize()); + assertEquals(1, countingDir.numFileLengthCalls); + + try (IndexOutput out = cachingDir.createTempOutput("bar", "baz", IOContext.DEFAULT)) { + out.writeBytes(new byte[4], 4); + } + + assertEquals(9, cachingDir.estimateSize()); + // +2 because there are two files + assertEquals(3, countingDir.numFileLengthCalls); + assertEquals(9, cachingDir.estimateSize()); + assertEquals(3, countingDir.numFileLengthCalls); + } + } + +} From a18910a97adfb52f47bacba9f1c2d1afc9a98480 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 18 May 2018 14:30:25 +0200 Subject: [PATCH 2/4] Fix size on init. --- .../index/store/ByteSizeCachingDirectory.java | 2 +- .../store/ByteSizeCachingDirectoryTests.java | 26 +++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java index 62fe8d79895c6..e2cf2642db3d2 100644 --- a/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java +++ b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java @@ -65,7 +65,7 @@ private static long estimateSize(Directory directory) throws IOException { ByteSizeCachingDirectory(Directory in, TimeValue refreshInterval) { super(in); - size = new SingleObjectCache(refreshInterval, new SizeAndModCount(0L, 0L)) { + size = new SingleObjectCache(refreshInterval, new SizeAndModCount(0L, -1L)) { @Override protected SizeAndModCount refresh() { // Compute modCount first so that updates that happen while the size diff --git a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java index d1a46aa3d5ee0..1259af5bbf543 100644 --- a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java +++ b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java @@ -47,30 +47,34 @@ public long fileLength(String name) throws IOException { public void testBasics() throws IOException { try (Directory dir = newDirectory()) { + try (IndexOutput out = dir.createOutput("quux", IOContext.DEFAULT)) { + out.writeBytes(new byte[11], 11); + } LengthCountingDirectory countingDir = new LengthCountingDirectory(dir); + ByteSizeCachingDirectory cachingDir = new ByteSizeCachingDirectory(countingDir, new TimeValue(0)); - assertEquals(0, cachingDir.estimateSize()); - assertEquals(0, cachingDir.estimateSize()); - assertEquals(0, countingDir.numFileLengthCalls); + assertEquals(11, cachingDir.estimateSize()); + assertEquals(11, cachingDir.estimateSize()); + assertEquals(1, countingDir.numFileLengthCalls); try (IndexOutput out = cachingDir.createOutput("foo", IOContext.DEFAULT)) { out.writeBytes(new byte[5], 5); } - assertEquals(5, cachingDir.estimateSize()); - assertEquals(1, countingDir.numFileLengthCalls); - assertEquals(5, cachingDir.estimateSize()); - assertEquals(1, countingDir.numFileLengthCalls); + assertEquals(16, cachingDir.estimateSize()); + assertEquals(3, countingDir.numFileLengthCalls); + assertEquals(16, cachingDir.estimateSize()); + assertEquals(3, countingDir.numFileLengthCalls); try (IndexOutput out = cachingDir.createTempOutput("bar", "baz", IOContext.DEFAULT)) { out.writeBytes(new byte[4], 4); } - assertEquals(9, cachingDir.estimateSize()); + assertEquals(20, cachingDir.estimateSize()); // +2 because there are two files - assertEquals(3, countingDir.numFileLengthCalls); - assertEquals(9, cachingDir.estimateSize()); - assertEquals(3, countingDir.numFileLengthCalls); + assertEquals(6, countingDir.numFileLengthCalls); + assertEquals(20, cachingDir.estimateSize()); + assertEquals(6, countingDir.numFileLengthCalls); } } From 3b94e67dddae7a22418e8a9569a7ce0dd88c6d24 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Fri, 18 May 2018 14:40:30 +0200 Subject: [PATCH 3/4] iter --- .../index/store/ByteSizeCachingDirectoryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java index 1259af5bbf543..9960b480fb3e9 100644 --- a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java +++ b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java @@ -71,7 +71,7 @@ public void testBasics() throws IOException { } assertEquals(20, cachingDir.estimateSize()); - // +2 because there are two files + // +3 because there are 3 files assertEquals(6, countingDir.numFileLengthCalls); assertEquals(20, cachingDir.estimateSize()); assertEquals(6, countingDir.numFileLengthCalls); From d438c5651fbbc362b4d40768602fb0fd2b5e3371 Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Thu, 24 May 2018 11:44:42 +0200 Subject: [PATCH 4/4] Avoid modifying atomic longs on every byte write. --- .../index/store/ByteSizeCachingDirectory.java | 79 ++++++++++++++----- .../org/elasticsearch/index/store/Store.java | 2 +- .../store/ByteSizeCachingDirectoryTests.java | 41 +++++++--- 3 files changed, 91 insertions(+), 31 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java index e2cf2642db3d2..3b0a912c2df79 100644 --- a/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java +++ b/server/src/main/java/org/elasticsearch/index/store/ByteSizeCachingDirectory.java @@ -32,21 +32,22 @@ import java.io.UncheckedIOException; import java.nio.file.AccessDeniedException; import java.nio.file.NoSuchFileException; -import java.util.concurrent.atomic.AtomicLong; final class ByteSizeCachingDirectory extends FilterDirectory { private static class SizeAndModCount { final long size; final long modCount; + final boolean pendingWrite; - SizeAndModCount(long length, long modCount) { + SizeAndModCount(long length, long modCount, boolean pendingWrite) { this.size = length; this.modCount = modCount; + this.pendingWrite = pendingWrite; } } - private static long estimateSize(Directory directory) throws IOException { + private static long estimateSizeInBytes(Directory directory) throws IOException { long estimatedSize = 0; String[] files = directory.listAll(); for (String file : files) { @@ -60,39 +61,61 @@ private static long estimateSize(Directory directory) throws IOException { return estimatedSize; } - private final AtomicLong modCount = new AtomicLong(); private final SingleObjectCache size; + // Both these variables need to be accessed under `this` lock. + private long modCount = 0; + private long numOpenOutputs = 0; ByteSizeCachingDirectory(Directory in, TimeValue refreshInterval) { super(in); - size = new SingleObjectCache(refreshInterval, new SizeAndModCount(0L, -1L)) { + size = new SingleObjectCache(refreshInterval, new SizeAndModCount(0L, -1L, true)) { @Override protected SizeAndModCount refresh() { - // Compute modCount first so that updates that happen while the size - // is being computed invalidate the length - final long modCount = ByteSizeCachingDirectory.this.modCount.get(); + // It is ok for the size of the directory to be more recent than + // the mod count, we would just recompute the size of the + // directory on the next call as well. However the opposite + // would be bad as we would potentially have a stale cache + // entry for a long time. So we fetch the values of modCount and + // numOpenOutputs BEFORE computing the size of the directory. + final long modCount; + final boolean pendingWrite; + synchronized(ByteSizeCachingDirectory.this) { + modCount = ByteSizeCachingDirectory.this.modCount; + pendingWrite = ByteSizeCachingDirectory.this.numOpenOutputs != 0; + } final long size; try { - size = estimateSize(getDelegate()); + // Compute this OUTSIDE of the lock + size = estimateSizeInBytes(getDelegate()); } catch (IOException e) { throw new UncheckedIOException(e); } - return new SizeAndModCount(size, modCount); + return new SizeAndModCount(size, modCount, pendingWrite); } @Override protected boolean needsRefresh() { - if (getNoRefresh().modCount == modCount.get()) { - // no updates to the directory since the last refresh + if (super.needsRefresh() == false) { + // The size was computed recently, don't recompute return false; } - return super.needsRefresh(); + SizeAndModCount cached = getNoRefresh(); + if (cached.pendingWrite) { + // The cached entry was generated while there were pending + // writes, so the size might be stale: recompute. + return true; + } + synchronized(ByteSizeCachingDirectory.this) { + // If there are pending writes or if new files have been + // written/deleted since last time: recompute + return numOpenOutputs != 0 || cached.modCount != modCount; + } } }; } /** Return the cumulative size of all files in this directory. */ - long estimateSize() throws IOException { + long estimateSizeInBytes() throws IOException { try { return size.getOrRefresh().size; } catch (UncheckedIOException e) { @@ -112,33 +135,49 @@ public IndexOutput createTempOutput(String prefix, String suffix, IOContext cont } private IndexOutput wrapIndexOutput(IndexOutput out) { + synchronized (this) { + numOpenOutputs++; + } return new FilterIndexOutput(out.toString(), out) { @Override public void writeBytes(byte[] b, int length) throws IOException { + // Don't write to atomicXXX here since it might be called in + // tight loops and memory barriers are costly super.writeBytes(b, length); - modCount.incrementAndGet(); } @Override public void writeByte(byte b) throws IOException { + // Don't write to atomicXXX here since it might be called in + // tight loops and memory barriers are costly super.writeByte(b); - modCount.incrementAndGet(); } @Override public void close() throws IOException { // Close might cause some data to be flushed from in-memory buffers, so // increment the modification counter too. - super.close(); - modCount.incrementAndGet(); + try { + super.close(); + } finally { + synchronized (this) { + numOpenOutputs--; + modCount++; + } + } } }; } @Override public void deleteFile(String name) throws IOException { - super.deleteFile(name); - modCount.incrementAndGet(); + try { + super.deleteFile(name); + } finally { + synchronized (this) { + modCount++; + } + } } } diff --git a/server/src/main/java/org/elasticsearch/index/store/Store.java b/server/src/main/java/org/elasticsearch/index/store/Store.java index 61679423d994e..001e263ea8ffb 100644 --- a/server/src/main/java/org/elasticsearch/index/store/Store.java +++ b/server/src/main/java/org/elasticsearch/index/store/Store.java @@ -735,7 +735,7 @@ static final class StoreDirectory extends FilterDirectory { /** Estimate the cumulative size of all files in this directory in bytes. */ long estimateSize() throws IOException { - return ((ByteSizeCachingDirectory) getDelegate()).estimateSize(); + return ((ByteSizeCachingDirectory) getDelegate()).estimateSizeInBytes(); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java index 9960b480fb3e9..25d783d25315f 100644 --- a/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java +++ b/server/src/test/java/org/elasticsearch/index/store/ByteSizeCachingDirectoryTests.java @@ -53,28 +53,49 @@ public void testBasics() throws IOException { LengthCountingDirectory countingDir = new LengthCountingDirectory(dir); ByteSizeCachingDirectory cachingDir = new ByteSizeCachingDirectory(countingDir, new TimeValue(0)); - assertEquals(11, cachingDir.estimateSize()); - assertEquals(11, cachingDir.estimateSize()); + assertEquals(11, cachingDir.estimateSizeInBytes()); + assertEquals(11, cachingDir.estimateSizeInBytes()); assertEquals(1, countingDir.numFileLengthCalls); try (IndexOutput out = cachingDir.createOutput("foo", IOContext.DEFAULT)) { out.writeBytes(new byte[5], 5); + + cachingDir.estimateSizeInBytes(); + // +2 because there are 3 files + assertEquals(3, countingDir.numFileLengthCalls); + // An index output is open so no caching + cachingDir.estimateSizeInBytes(); + assertEquals(5, countingDir.numFileLengthCalls); } - assertEquals(16, cachingDir.estimateSize()); - assertEquals(3, countingDir.numFileLengthCalls); - assertEquals(16, cachingDir.estimateSize()); - assertEquals(3, countingDir.numFileLengthCalls); + assertEquals(16, cachingDir.estimateSizeInBytes()); + assertEquals(7, countingDir.numFileLengthCalls); + assertEquals(16, cachingDir.estimateSizeInBytes()); + assertEquals(7, countingDir.numFileLengthCalls); try (IndexOutput out = cachingDir.createTempOutput("bar", "baz", IOContext.DEFAULT)) { out.writeBytes(new byte[4], 4); + + cachingDir.estimateSizeInBytes(); + assertEquals(10, countingDir.numFileLengthCalls); + // An index output is open so no caching + cachingDir.estimateSizeInBytes(); + assertEquals(13, countingDir.numFileLengthCalls); } - assertEquals(20, cachingDir.estimateSize()); + assertEquals(20, cachingDir.estimateSizeInBytes()); // +3 because there are 3 files - assertEquals(6, countingDir.numFileLengthCalls); - assertEquals(20, cachingDir.estimateSize()); - assertEquals(6, countingDir.numFileLengthCalls); + assertEquals(16, countingDir.numFileLengthCalls); + assertEquals(20, cachingDir.estimateSizeInBytes()); + assertEquals(16, countingDir.numFileLengthCalls); + + cachingDir.deleteFile("foo"); + + assertEquals(15, cachingDir.estimateSizeInBytes()); + // +2 because there are 2 files now + assertEquals(18, countingDir.numFileLengthCalls); + assertEquals(15, cachingDir.estimateSizeInBytes()); + assertEquals(18, countingDir.numFileLengthCalls); } }