Skip to content

Commit

Permalink
Gracefully recover from a failure to trim cache to size.
Browse files Browse the repository at this point in the history
  • Loading branch information
@dave-r12
dave-r12 committed Feb 25, 2016
1 parent 3598913 commit e0369c8
Show file tree
Hide file tree
Showing 3 changed files with 221 additions and 10 deletions.
201 changes: 193 additions & 8 deletions okhttp-tests/src/test/java/okhttp3/internal/DiskLruCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1073,11 +1073,11 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
set("b", "b", "b");

// We can't begin the edit if writing 'DIRTY' fails.
fileSystem.setFaulty(journalFile, true);
fileSystem.setFaultyWrite(journalFile, true);
assertNull(cache.edit("c"));

// Once the journal has a failure, subsequent writes aren't permitted.
fileSystem.setFaulty(journalFile, false);
fileSystem.setFaultyWrite(journalFile, false);
assertNull(cache.edit("d"));

// Confirm that the fault didn't corrupt entries stored before the fault was introduced.
Expand All @@ -1101,11 +1101,11 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
DiskLruCache.Editor editor = cache.edit("c");
setString(editor, 0, "c");
setString(editor, 1, "c");
fileSystem.setFaulty(journalFile, true);
fileSystem.setFaultyWrite(journalFile, true);
editor.commit();

// Once the journal has a failure, subsequent writes aren't permitted.
fileSystem.setFaulty(journalFile, false);
fileSystem.setFaultyWrite(journalFile, false);
assertNull(cache.edit("d"));

// Confirm that the fault didn't corrupt entries stored before the fault was introduced.
Expand All @@ -1125,11 +1125,11 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
DiskLruCache.Editor editor = cache.edit("c");
setString(editor, 0, "c");
setString(editor, 1, "c");
fileSystem.setFaulty(journalFile, true);
fileSystem.setFaultyWrite(journalFile, true);
editor.abort();

// Once the journal has a failure, subsequent writes aren't permitted.
fileSystem.setFaulty(journalFile, false);
fileSystem.setFaultyWrite(journalFile, false);
assertNull(cache.edit("d"));

// Confirm that the fault didn't corrupt entries stored before the fault was introduced.
Expand All @@ -1146,17 +1146,202 @@ private void createNewCacheWithSize(int maxSize) throws IOException {
set("b", "b", "b");

// Remove, but the journal write will fail.
fileSystem.setFaulty(journalFile, true);
fileSystem.setFaultyWrite(journalFile, true);
assertTrue(cache.remove("a"));

// Confirm that the entry was still removed.
fileSystem.setFaulty(journalFile, false);
fileSystem.setFaultyWrite(journalFile, false);
cache.close();
cache = new DiskLruCache(fileSystem, cacheDir, appVersion, 2, Integer.MAX_VALUE, executor);
assertAbsent("a");
assertValue("b", "b", "b");
}

@Test public void cleanupTrimFailurePreventsNewEditors() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// Confirm that edits are prevented after a cache trim failure.
assertNull(cache.edit("a"));
assertNull(cache.edit("b"));
assertNull(cache.edit("c"));

// Allow the test to clean up.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
}

@Test public void cleanupTrimFailureRetriedOnEditors() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// An edit should now add a job to clean up if the most recent trim failed.
assertNull(cache.edit("b"));
executor.jobs.pop().run();

// Confirm a successful cache trim now allows edits.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
assertNull(cache.edit("c"));
executor.jobs.pop().run();
set("c", "cc", "cc");
assertValue("c", "cc", "cc");
}

@Test public void cleanupTrimFailureWithInFlightEditor() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aaa");
set("b", "bb", "bb");
DiskLruCache.Editor inFlightEditor = cache.edit("c");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// The in-flight editor can still write after a trim failure.
setString(inFlightEditor, 0, "cc");
setString(inFlightEditor, 1, "cc");
inFlightEditor.commit();

// Confirm the committed values are present after a successful cache trim.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
executor.jobs.pop().run();
assertValue("c", "cc", "cc");
}

@Test public void cleanupTrimFailureAllowsSnapshotReads() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// Confirm we still allow snapshot reads after a trim failure.
assertValue("a", "aa", "aa");
assertValue("b", "bb", "bbb");

// Allow the test to clean up.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
}

@Test public void cleanupTrimFailurePreventsSnapshotWrites() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// Confirm snapshot writes are prevented after a trim failure.
DiskLruCache.Snapshot snapshot1 = cache.get("a");
assertNull(snapshot1.edit());
snapshot1.close();
DiskLruCache.Snapshot snapshot2 = cache.get("b");
assertNull(snapshot2.edit());
snapshot2.close();

// Allow the test to clean up.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
}

@Test public void evictAllAfterCleanupTrimFailure() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// Confirm we prevent edits after a trim failure.
assertNull(cache.edit("c"));

// A successful eviction should allow new writes.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
cache.evictAll();
set("c", "cc", "cc");
assertValue("c", "cc", "cc");
}

@Test public void manualRemovalAfterCleanupTrimFailure() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// Confirm we prevent edits after a trim failure.
assertNull(cache.edit("c"));

// A successful removal which trims the cache should allow new writes.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
cache.remove("a");
set("c", "cc", "cc");
assertValue("c", "cc", "cc");
}

@Test public void flushingAfterCleanupTrimFailure() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim job to fail.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), true);
executor.jobs.pop().run();

// Confirm we prevent edits after a trim failure.
assertNull(cache.edit("c"));

// A successful flush trims the cache and should allow new writes.
fileSystem.setFaultyDelete(new File(cacheDir, "a.0"), false);
cache.flush();
set("c", "cc", "cc");
assertValue("c", "cc", "cc");
}

@Test public void cleanupTrimFailureWithPartialSnapshot() throws Exception {
cache.setMaxSize(8);
executor.jobs.pop();
set("a", "aa", "aa");
set("b", "bb", "bbb");

// Cause the cache trim to fail on the second value leaving a partial snapshot.
fileSystem.setFaultyDelete(new File(cacheDir, "a.1"), true);
executor.jobs.pop().run();

// Confirm the partial snapshot is not returned.
assertNull(cache.get("a"));

// Confirm we prevent edits after a trim failure.
assertNull(cache.edit("a"));

// Confirm the partial snapshot is not returned after a successful trim.
fileSystem.setFaultyDelete(new File(cacheDir, "a.1"), false);
executor.jobs.pop().run();
assertNull(cache.get("a"));
}

private void assertJournalEquals(String... expectedBodyLines) throws Exception {
List<String> expectedLines = new ArrayList<>();
expectedLines.add(MAGIC);
Expand Down
12 changes: 11 additions & 1 deletion okhttp-tests/src/test/java/okhttp3/internal/FaultyFileSystem.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,28 @@
public final class FaultyFileSystem implements FileSystem {
private final FileSystem delegate;
private final Set<File> writeFaults = new LinkedHashSet<>();
private final Set<File> deleteFaults = new LinkedHashSet<>();

public FaultyFileSystem(FileSystem delegate) {
this.delegate = delegate;
}

public void setFaulty(File file, boolean faulty) {
public void setFaultyWrite(File file, boolean faulty) {
if (faulty) {
writeFaults.add(file);
} else {
writeFaults.remove(file);
}
}

public void setFaultyDelete(File file, boolean faulty) {
if (faulty) {
deleteFaults.add(file);
} else {
deleteFaults.remove(file);
}
}

@Override public Source source(File file) throws FileNotFoundException {
return delegate.source(file);
}
Expand All @@ -55,6 +64,7 @@ public void setFaulty(File file, boolean faulty) {
}

@Override public void delete(File file) throws IOException {
if (deleteFaults.contains(file)) throw new IOException("boom!");
delegate.delete(file);
}

Expand Down
18 changes: 17 additions & 1 deletion okhttp/src/main/java/okhttp3/internal/DiskLruCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ public final class DiskLruCache implements Closeable, Flushable {
// Must be read and written when synchronized on 'this'.
private boolean initialized;
private boolean closed;
private boolean mostRecentTrimFailed;

/**
* To differentiate between old and current snapshots, each entry is given a sequence number each
Expand All @@ -167,8 +168,14 @@ public void run() {
if (!initialized | closed) {
return; // Nothing to do
}

try {
trimToSize();
} catch (IOException ignored) {
mostRecentTrimFailed = true;
}

try {
if (journalRebuildRequired()) {
rebuildJournal();
redundantOpCount = 0;
Expand Down Expand Up @@ -453,6 +460,11 @@ private synchronized Editor edit(String key, long expectedSequenceNumber) throws
if (entry != null && entry.currentEditor != null) {
return null; // Another edit is in progress.
}
if (mostRecentTrimFailed) {
// Prevent new writes so the cache doesn't grow any further and retry the clean up operation.
executor.execute(cleanupRunnable);
return null;
}

// Flush the journal before creating files to prevent file leaks.
journalWriter.writeUtf8(DIRTY).writeByte(' ').writeUtf8(key).writeByte('\n');
Expand Down Expand Up @@ -586,7 +598,9 @@ public synchronized boolean remove(String key) throws IOException {
validateKey(key);
Entry entry = lruEntries.get(key);
if (entry == null) return false;
return removeEntry(entry);
boolean removed = removeEntry(entry);
if (removed && size <= maxSize) mostRecentTrimFailed = false;
return removed;
}

private boolean removeEntry(Entry entry) throws IOException {
Expand Down Expand Up @@ -654,6 +668,7 @@ private void trimToSize() throws IOException {
Entry toEvict = lruEntries.values().iterator().next();
removeEntry(toEvict);
}
mostRecentTrimFailed = false;
}

/**
Expand All @@ -675,6 +690,7 @@ public synchronized void evictAll() throws IOException {
for (Entry entry : lruEntries.values().toArray(new Entry[lruEntries.size()])) {
removeEntry(entry);
}
mostRecentTrimFailed = false;
}

private void validateKey(String key) {
Expand Down

0 comments on commit e0369c8

Please sign in to comment.