Skip to content

Commit 229b953

Browse files
authored
Add cache directory low-level instrumentation (#51637)
This commit is a first step to add instrumentation to the CacheDirectory added in #50693. It adds a new mutable IndexInputStats object that allows to track various information about how CacheBufferedIndexInput interacts with the underlying cache file to satisfy the read operations. It keep tracks of small/large forward/backward seekings as well as the total number of bytes read from the IndexInput and the total number of bytes read/written from/to the CacheFile. Note that the stats do not reflect the exact usage that Lucene does of the IndexInputs opened through a CacheDirectory: IndexInputStats is not aware of the read operations that are directly served at a higher level by the internal BufferedIndexInput's buffer. Instead it tracks what really hit the disk.
1 parent ef18352 commit 229b953

File tree

6 files changed

+707
-38
lines changed

6 files changed

+707
-38
lines changed

x-pack/plugin/searchable-snapshots/src/main/java/org/elasticsearch/xpack/searchablesnapshots/cache/CacheDirectory.java

Lines changed: 53 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
*/
66
package org.elasticsearch.xpack.searchablesnapshots.cache;
77

8+
import org.apache.logging.log4j.LogManager;
9+
import org.apache.logging.log4j.Logger;
10+
import org.apache.logging.log4j.message.ParameterizedMessage;
811
import org.apache.lucene.store.AlreadyClosedException;
912
import org.apache.lucene.store.BufferedIndexInput;
1013
import org.apache.lucene.store.Directory;
@@ -14,6 +17,7 @@
1417
import org.elasticsearch.common.Nullable;
1518
import org.elasticsearch.common.SuppressForbidden;
1619
import org.elasticsearch.common.io.Channels;
20+
import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
1721
import org.elasticsearch.common.util.concurrent.ReleasableLock;
1822
import org.elasticsearch.index.shard.ShardId;
1923
import org.elasticsearch.repositories.IndexId;
@@ -25,6 +29,7 @@
2529
import java.nio.channels.FileChannel;
2630
import java.nio.file.Files;
2731
import java.nio.file.Path;
32+
import java.util.Map;
2833
import java.util.Objects;
2934
import java.util.concurrent.atomic.AtomicBoolean;
3035
import java.util.concurrent.atomic.AtomicReference;
@@ -34,8 +39,10 @@
3439
*/
3540
public class CacheDirectory extends FilterDirectory {
3641

42+
private static final Logger logger = LogManager.getLogger(CacheDirectory.class);
3743
private static final int COPY_BUFFER_SIZE = 8192;
3844

45+
private final Map<String, IndexInputStats> stats;
3946
private final CacheService cacheService;
4047
private final SnapshotId snapshotId;
4148
private final IndexId indexId;
@@ -45,6 +52,7 @@ public class CacheDirectory extends FilterDirectory {
4552
public CacheDirectory(Directory in, CacheService cacheService, Path cacheDir, SnapshotId snapshotId, IndexId indexId, ShardId shardId)
4653
throws IOException {
4754
super(in);
55+
this.stats = ConcurrentCollections.newConcurrentMapWithAggressiveConcurrency();
4856
this.cacheService = Objects.requireNonNull(cacheService);
4957
this.cacheDir = Files.createDirectories(cacheDir);
5058
this.snapshotId = Objects.requireNonNull(snapshotId);
@@ -56,6 +64,11 @@ private CacheKey createCacheKey(String fileName) {
5664
return new CacheKey(snapshotId, indexId, shardId, fileName);
5765
}
5866

67+
// pkg private for tests
68+
@Nullable IndexInputStats getStats(String name) {
69+
return stats.get(name);
70+
}
71+
5972
public void close() throws IOException {
6073
super.close();
6174
// Ideally we could let the cache evict/remove cached files by itself after the
@@ -66,7 +79,8 @@ public void close() throws IOException {
6679
@Override
6780
public IndexInput openInput(final String name, final IOContext context) throws IOException {
6881
ensureOpen();
69-
return new CacheBufferedIndexInput(name, fileLength(name), context);
82+
final long fileLength = fileLength(name);
83+
return new CacheBufferedIndexInput(name, fileLength, context, stats.computeIfAbsent(name, n -> new IndexInputStats(fileLength)));
7084
}
7185

7286
private class CacheFileReference implements CacheFile.EvictionListener {
@@ -141,22 +155,28 @@ public class CacheBufferedIndexInput extends BufferedIndexInput {
141155
private final long offset;
142156
private final long end;
143157
private final CacheFileReference cacheFileReference;
158+
private final IndexInputStats stats;
144159

145160
// the following are only mutable so they can be adjusted after cloning
146161
private AtomicBoolean closed;
147162
private boolean isClone;
148163

149-
CacheBufferedIndexInput(String fileName, long fileLength, IOContext ioContext) {
150-
this(new CacheFileReference(fileName, fileLength), ioContext,
164+
// last read position is kept around in order to detect (non)contiguous reads for stats
165+
private long lastReadPosition;
166+
167+
CacheBufferedIndexInput(String fileName, long fileLength, IOContext ioContext, IndexInputStats stats) {
168+
this(new CacheFileReference(fileName, fileLength), ioContext, stats,
151169
"CachedBufferedIndexInput(" + fileName + ")", 0L, fileLength, false);
170+
stats.incrementOpenCount();
152171
}
153172

154-
private CacheBufferedIndexInput(CacheFileReference cacheFileReference, IOContext ioContext, String desc, long offset, long length,
155-
boolean isClone) {
173+
private CacheBufferedIndexInput(CacheFileReference cacheFileReference, IOContext ioContext, IndexInputStats stats,
174+
String desc, long offset, long length, boolean isClone) {
156175
super(desc, ioContext);
157176
this.ioContext = ioContext;
158177
this.offset = offset;
159178
this.cacheFileReference = cacheFileReference;
179+
this.stats = stats;
160180
this.end = offset + length;
161181
this.closed = new AtomicBoolean(false);
162182
this.isClone = isClone;
@@ -171,6 +191,7 @@ public long length() {
171191
public void close() {
172192
if (closed.compareAndSet(false, true)) {
173193
if (isClone == false) {
194+
stats.incrementCloseCount();
174195
cacheFileReference.releaseOnClose();
175196
}
176197
}
@@ -180,20 +201,21 @@ public void close() {
180201
protected void readInternal(final byte[] buffer, final int offset, final int length) throws IOException {
181202
final long position = getFilePointer() + this.offset;
182203

183-
int bytesRead = 0;
184-
while (bytesRead < length) {
185-
final long pos = position + bytesRead;
186-
final int off = offset + bytesRead;
187-
final int len = length - bytesRead;
204+
int totalBytesRead = 0;
205+
while (totalBytesRead < length) {
206+
final long pos = position + totalBytesRead;
207+
final int off = offset + totalBytesRead;
208+
final int len = length - totalBytesRead;
188209

210+
int bytesRead = 0;
189211
try {
190212
final CacheFile cacheFile = cacheFileReference.get();
191213
if (cacheFile == null) {
192214
throw new AlreadyClosedException("Failed to acquire a non-evicted cache file");
193215
}
194216

195217
try (ReleasableLock ignored = cacheFile.fileLock()) {
196-
bytesRead += cacheFile.fetchRange(pos,
218+
bytesRead = cacheFile.fetchRange(pos,
197219
(start, end) -> readCacheFile(cacheFile.getChannel(), end, pos, buffer, off, len),
198220
(start, end) -> writeCacheFile(cacheFile.getChannel(), start, end))
199221
.get();
@@ -202,33 +224,42 @@ protected void readInternal(final byte[] buffer, final int offset, final int len
202224
if (e instanceof AlreadyClosedException || (e.getCause() != null && e.getCause() instanceof AlreadyClosedException)) {
203225
try {
204226
// cache file was evicted during the range fetching, read bytes directly from source
205-
bytesRead += readDirectly(pos, pos + len, buffer, off);
227+
bytesRead = readDirectly(pos, pos + len, buffer, off);
206228
continue;
207229
} catch (Exception inner) {
208230
e.addSuppressed(inner);
209231
}
210232
}
211233
throw new IOException("Fail to read data from cache", e);
212234

235+
} finally {
236+
totalBytesRead += bytesRead;
213237
}
214238
}
215-
assert bytesRead == length : "partial read operation, read [" + bytesRead + "] bytes of [" + length + "]";
239+
assert totalBytesRead == length : "partial read operation, read [" + totalBytesRead + "] bytes of [" + length + "]";
240+
stats.incrementBytesRead(lastReadPosition, position, totalBytesRead);
241+
lastReadPosition = position + totalBytesRead;
216242
}
217243

218244
int readCacheFile(FileChannel fc, long end, long position, byte[] buffer, int offset, long length) throws IOException {
219245
assert assertFileChannelOpen(fc);
220-
return Channels.readFromFileChannel(fc, position, buffer, offset, Math.toIntExact(Math.min(length, end - position)));
246+
int bytesRead = Channels.readFromFileChannel(fc, position, buffer, offset, Math.toIntExact(Math.min(length, end - position)));
247+
stats.addCachedBytesRead(bytesRead);
248+
return bytesRead;
221249
}
222250

223251
@SuppressForbidden(reason = "Use positional writes on purpose")
224252
void writeCacheFile(FileChannel fc, long start, long end) throws IOException {
225253
assert assertFileChannelOpen(fc);
226254
final byte[] copyBuffer = new byte[Math.toIntExact(Math.min(COPY_BUFFER_SIZE, end - start))];
255+
logger.trace(() -> new ParameterizedMessage("writing range [{}-{}] to cache file [{}]", start, end, cacheFileReference));
256+
257+
int bytesCopied = 0;
227258
try (IndexInput input = in.openInput(cacheFileReference.getFileName(), ioContext)) {
259+
stats.incrementInnerOpenCount();
228260
if (start > 0) {
229261
input.seek(start);
230262
}
231-
int bytesCopied = 0;
232263
long remaining = end - start;
233264
while (remaining > 0) {
234265
final int size = (remaining < copyBuffer.length) ? Math.toIntExact(remaining) : copyBuffer.length;
@@ -237,6 +268,7 @@ void writeCacheFile(FileChannel fc, long start, long end) throws IOException {
237268
bytesCopied += size;
238269
remaining -= size;
239270
}
271+
stats.addCachedBytesWritten(bytesCopied);
240272
}
241273
}
242274

@@ -247,6 +279,7 @@ protected void seekInternal(long pos) throws IOException {
247279
} else if (pos < 0L) {
248280
throw new IOException("Seeking to negative position [" + pos + "] for " + toString());
249281
}
282+
stats.incrementSeeks(getFilePointer(), pos);
250283
}
251284

252285
@Override
@@ -263,7 +296,7 @@ public IndexInput slice(String sliceDescription, long offset, long length) {
263296
throw new IllegalArgumentException("slice() " + sliceDescription + " out of bounds: offset=" + offset
264297
+ ",length=" + length + ",fileLength=" + this.length() + ": " + this);
265298
}
266-
return new CacheBufferedIndexInput(cacheFileReference, ioContext,
299+
return new CacheBufferedIndexInput(cacheFileReference, ioContext, stats,
267300
getFullSliceDescription(sliceDescription), this.offset + offset, length, true);
268301
}
269302

@@ -280,9 +313,12 @@ public String toString() {
280313

281314
private int readDirectly(long start, long end, byte[] buffer, int offset) throws IOException {
282315
final byte[] copyBuffer = new byte[Math.toIntExact(Math.min(COPY_BUFFER_SIZE, end - start))];
316+
logger.trace(() ->
317+
new ParameterizedMessage("direct reading of range [{}-{}] for cache file [{}]", start, end, cacheFileReference));
283318

284319
int bytesCopied = 0;
285320
try (IndexInput input = in.openInput(cacheFileReference.getFileName(), ioContext)) {
321+
stats.incrementInnerOpenCount();
286322
if (start > 0) {
287323
input.seek(start);
288324
}
@@ -294,6 +330,7 @@ private int readDirectly(long start, long end, byte[] buffer, int offset) throws
294330
bytesCopied += len;
295331
remaining -= len;
296332
}
333+
stats.addDirectBytesRead(bytesCopied);
297334
}
298335
return bytesCopied;
299336
}

0 commit comments

Comments
 (0)