Skip to content

Commit 03dcf22

Browse files
authored
Move caching of the size of a directory to StoreDirectory. (#30581)
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.
1 parent 984523d commit 03dcf22

File tree

4 files changed

+302
-42
lines changed

4 files changed

+302
-42
lines changed

server/src/main/java/org/elasticsearch/common/util/SingleObjectCache.java

+5
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ public T getOrRefresh() {
6464
return cached;
6565
}
6666

67+
/** Return the potentially stale cached entry. */
68+
protected final T getNoRefresh() {
69+
return cached;
70+
}
71+
6772
/**
6873
* Returns a new instance to cache
6974
*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.index.store;
21+
22+
import org.apache.lucene.store.Directory;
23+
import org.apache.lucene.store.FilterDirectory;
24+
import org.apache.lucene.store.IOContext;
25+
import org.apache.lucene.store.IndexOutput;
26+
import org.elasticsearch.common.lucene.store.FilterIndexOutput;
27+
import org.elasticsearch.common.unit.TimeValue;
28+
import org.elasticsearch.common.util.SingleObjectCache;
29+
30+
import java.io.FileNotFoundException;
31+
import java.io.IOException;
32+
import java.io.UncheckedIOException;
33+
import java.nio.file.AccessDeniedException;
34+
import java.nio.file.NoSuchFileException;
35+
36+
final class ByteSizeCachingDirectory extends FilterDirectory {
37+
38+
private static class SizeAndModCount {
39+
final long size;
40+
final long modCount;
41+
final boolean pendingWrite;
42+
43+
SizeAndModCount(long length, long modCount, boolean pendingWrite) {
44+
this.size = length;
45+
this.modCount = modCount;
46+
this.pendingWrite = pendingWrite;
47+
}
48+
}
49+
50+
private static long estimateSizeInBytes(Directory directory) throws IOException {
51+
long estimatedSize = 0;
52+
String[] files = directory.listAll();
53+
for (String file : files) {
54+
try {
55+
estimatedSize += directory.fileLength(file);
56+
} catch (NoSuchFileException | FileNotFoundException | AccessDeniedException e) {
57+
// ignore, the file is not there no more; on Windows, if one thread concurrently deletes a file while
58+
// calling Files.size, you can also sometimes hit AccessDeniedException
59+
}
60+
}
61+
return estimatedSize;
62+
}
63+
64+
private final SingleObjectCache<SizeAndModCount> size;
65+
// Both these variables need to be accessed under `this` lock.
66+
private long modCount = 0;
67+
private long numOpenOutputs = 0;
68+
69+
ByteSizeCachingDirectory(Directory in, TimeValue refreshInterval) {
70+
super(in);
71+
size = new SingleObjectCache<SizeAndModCount>(refreshInterval, new SizeAndModCount(0L, -1L, true)) {
72+
@Override
73+
protected SizeAndModCount refresh() {
74+
// It is ok for the size of the directory to be more recent than
75+
// the mod count, we would just recompute the size of the
76+
// directory on the next call as well. However the opposite
77+
// would be bad as we would potentially have a stale cache
78+
// entry for a long time. So we fetch the values of modCount and
79+
// numOpenOutputs BEFORE computing the size of the directory.
80+
final long modCount;
81+
final boolean pendingWrite;
82+
synchronized(ByteSizeCachingDirectory.this) {
83+
modCount = ByteSizeCachingDirectory.this.modCount;
84+
pendingWrite = ByteSizeCachingDirectory.this.numOpenOutputs != 0;
85+
}
86+
final long size;
87+
try {
88+
// Compute this OUTSIDE of the lock
89+
size = estimateSizeInBytes(getDelegate());
90+
} catch (IOException e) {
91+
throw new UncheckedIOException(e);
92+
}
93+
return new SizeAndModCount(size, modCount, pendingWrite);
94+
}
95+
96+
@Override
97+
protected boolean needsRefresh() {
98+
if (super.needsRefresh() == false) {
99+
// The size was computed recently, don't recompute
100+
return false;
101+
}
102+
SizeAndModCount cached = getNoRefresh();
103+
if (cached.pendingWrite) {
104+
// The cached entry was generated while there were pending
105+
// writes, so the size might be stale: recompute.
106+
return true;
107+
}
108+
synchronized(ByteSizeCachingDirectory.this) {
109+
// If there are pending writes or if new files have been
110+
// written/deleted since last time: recompute
111+
return numOpenOutputs != 0 || cached.modCount != modCount;
112+
}
113+
}
114+
};
115+
}
116+
117+
/** Return the cumulative size of all files in this directory. */
118+
long estimateSizeInBytes() throws IOException {
119+
try {
120+
return size.getOrRefresh().size;
121+
} catch (UncheckedIOException e) {
122+
// we wrapped in the cache and unwrap here
123+
throw e.getCause();
124+
}
125+
}
126+
127+
@Override
128+
public IndexOutput createOutput(String name, IOContext context) throws IOException {
129+
return wrapIndexOutput(super.createOutput(name, context));
130+
}
131+
132+
@Override
133+
public IndexOutput createTempOutput(String prefix, String suffix, IOContext context) throws IOException {
134+
return wrapIndexOutput(super.createTempOutput(prefix, suffix, context));
135+
}
136+
137+
private IndexOutput wrapIndexOutput(IndexOutput out) {
138+
synchronized (this) {
139+
numOpenOutputs++;
140+
}
141+
return new FilterIndexOutput(out.toString(), out) {
142+
@Override
143+
public void writeBytes(byte[] b, int length) throws IOException {
144+
// Don't write to atomicXXX here since it might be called in
145+
// tight loops and memory barriers are costly
146+
super.writeBytes(b, length);
147+
}
148+
149+
@Override
150+
public void writeByte(byte b) throws IOException {
151+
// Don't write to atomicXXX here since it might be called in
152+
// tight loops and memory barriers are costly
153+
super.writeByte(b);
154+
}
155+
156+
@Override
157+
public void close() throws IOException {
158+
// Close might cause some data to be flushed from in-memory buffers, so
159+
// increment the modification counter too.
160+
try {
161+
super.close();
162+
} finally {
163+
synchronized (this) {
164+
numOpenOutputs--;
165+
modCount++;
166+
}
167+
}
168+
}
169+
};
170+
}
171+
172+
@Override
173+
public void deleteFile(String name) throws IOException {
174+
try {
175+
super.deleteFile(name);
176+
} finally {
177+
synchronized (this) {
178+
modCount++;
179+
}
180+
}
181+
}
182+
183+
}

server/src/main/java/org/elasticsearch/index/store/Store.java

+12-42
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
import org.apache.lucene.util.BytesRef;
5151
import org.apache.lucene.util.BytesRefBuilder;
5252
import org.apache.lucene.util.Version;
53-
import org.elasticsearch.ElasticsearchException;
5453
import org.elasticsearch.ExceptionsHelper;
5554
import org.elasticsearch.common.UUIDs;
5655
import org.elasticsearch.common.bytes.BytesReference;
@@ -67,7 +66,6 @@
6766
import org.elasticsearch.common.settings.Setting.Property;
6867
import org.elasticsearch.common.settings.Settings;
6968
import org.elasticsearch.common.unit.TimeValue;
70-
import org.elasticsearch.common.util.SingleObjectCache;
7169
import org.elasticsearch.common.util.concurrent.AbstractRefCounted;
7270
import org.elasticsearch.common.util.concurrent.RefCounted;
7371
import org.elasticsearch.common.util.iterable.Iterables;
@@ -91,7 +89,6 @@
9189
import java.io.IOException;
9290
import java.io.InputStream;
9391
import java.io.PrintStream;
94-
import java.nio.file.AccessDeniedException;
9592
import java.nio.file.NoSuchFileException;
9693
import java.nio.file.Path;
9794
import java.util.ArrayList;
@@ -146,7 +143,6 @@ public class Store extends AbstractIndexShardComponent implements Closeable, Ref
146143
private final ReentrantReadWriteLock metadataLock = new ReentrantReadWriteLock();
147144
private final ShardLock shardLock;
148145
private final OnClose onClose;
149-
private final SingleObjectCache<StoreStats> statsCache;
150146

151147
private final AbstractRefCounted refCounter = new AbstractRefCounted("store") {
152148
@Override
@@ -164,12 +160,13 @@ public Store(ShardId shardId, IndexSettings indexSettings, DirectoryService dire
164160
OnClose onClose) throws IOException {
165161
super(shardId, indexSettings);
166162
final Settings settings = indexSettings.getSettings();
167-
this.directory = new StoreDirectory(directoryService.newDirectory(), Loggers.getLogger("index.store.deletes", settings, shardId));
168-
this.shardLock = shardLock;
169-
this.onClose = onClose;
163+
Directory dir = directoryService.newDirectory();
170164
final TimeValue refreshInterval = indexSettings.getValue(INDEX_STORE_STATS_REFRESH_INTERVAL_SETTING);
171-
this.statsCache = new StoreStatsCache(refreshInterval, directory);
172165
logger.debug("store stats are refreshed with refresh_interval [{}]", refreshInterval);
166+
ByteSizeCachingDirectory sizeCachingDir = new ByteSizeCachingDirectory(dir, refreshInterval);
167+
this.directory = new StoreDirectory(sizeCachingDir, Loggers.getLogger("index.store.deletes", settings, shardId));
168+
this.shardLock = shardLock;
169+
this.onClose = onClose;
173170

174171
assert onClose != null;
175172
assert shardLock != null;
@@ -377,7 +374,7 @@ public void exorciseIndex(CheckIndex.Status status) throws IOException {
377374

378375
public StoreStats stats() throws IOException {
379376
ensureOpen();
380-
return statsCache.getOrRefresh();
377+
return new StoreStats(directory.estimateSize());
381378
}
382379

383380
/**
@@ -731,11 +728,16 @@ static final class StoreDirectory extends FilterDirectory {
731728

732729
private final Logger deletesLogger;
733730

734-
StoreDirectory(Directory delegateDirectory, Logger deletesLogger) {
731+
StoreDirectory(ByteSizeCachingDirectory delegateDirectory, Logger deletesLogger) {
735732
super(delegateDirectory);
736733
this.deletesLogger = deletesLogger;
737734
}
738735

736+
/** Estimate the cumulative size of all files in this directory in bytes. */
737+
long estimateSize() throws IOException {
738+
return ((ByteSizeCachingDirectory) getDelegate()).estimateSizeInBytes();
739+
}
740+
739741
@Override
740742
public void close() {
741743
assert false : "Nobody should close this directory except of the Store itself";
@@ -1428,38 +1430,6 @@ public void accept(ShardLock Lock) {
14281430
};
14291431
}
14301432

1431-
private static class StoreStatsCache extends SingleObjectCache<StoreStats> {
1432-
private final Directory directory;
1433-
1434-
StoreStatsCache(TimeValue refreshInterval, Directory directory) throws IOException {
1435-
super(refreshInterval, new StoreStats(estimateSize(directory)));
1436-
this.directory = directory;
1437-
}
1438-
1439-
@Override
1440-
protected StoreStats refresh() {
1441-
try {
1442-
return new StoreStats(estimateSize(directory));
1443-
} catch (IOException ex) {
1444-
throw new ElasticsearchException("failed to refresh store stats", ex);
1445-
}
1446-
}
1447-
1448-
private static long estimateSize(Directory directory) throws IOException {
1449-
long estimatedSize = 0;
1450-
String[] files = directory.listAll();
1451-
for (String file : files) {
1452-
try {
1453-
estimatedSize += directory.fileLength(file);
1454-
} catch (NoSuchFileException | FileNotFoundException | AccessDeniedException e) {
1455-
// ignore, the file is not there no more; on Windows, if one thread concurrently deletes a file while
1456-
// calling Files.size, you can also sometimes hit AccessDeniedException
1457-
}
1458-
}
1459-
return estimatedSize;
1460-
}
1461-
}
1462-
14631433
/**
14641434
* creates an empty lucene index and a corresponding empty translog. Any existing data will be deleted.
14651435
*/

0 commit comments

Comments
 (0)