Skip to content

Commit

Permalink
pool: avoid throwing a RuntimeException for non-bugs
Browse files Browse the repository at this point in the history
Motivation:

The pool uses a lambda function to create a mover, wrapping any
CacheException thrown while creating the mover with a RuntimeException.

The code that handles RuntimeExceptions is faulty, as it checks whether
the root cause is a CacheException, rather than the immediate cause.
The CacheException may, itself, have a cause that isn't a
CacheException.

The result is that a CacheException is re-thrown as a RuntimeException,
which is then logged as a bug in dCache.

Modification:

Use a custom RuntimeException (UncheckedCacheException) to wrap the
CacheException within the lambda.  The catch block is now specific
enough to catch only the CacheException, leaving other RuntimeExceptions
to propagate.

Result:

Certain error cases, where a pool is unable to create a mover are no
longer logged as a bug in dCache.

Target: master
Request: 5.1
Request: 5.0
Request: 4.2
Request: 4.1
Request: 4.0
Request: 3.2
Requires-notes: yes
Requires-book: no
Patch: https://rb.dcache.org/r/11688/
Acked-by: Tigran Mkrtchyan
  • Loading branch information
paulmillar committed Apr 13, 2019
1 parent 382c98c commit 577a54d
Showing 1 changed file with 20 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package org.dcache.pool.classic;

import com.google.common.base.Throwables;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.InterruptedIOException;
import java.nio.channels.CompletionHandler;
import java.util.ArrayList;
Expand Down Expand Up @@ -54,6 +54,22 @@ public class MoverRequestScheduler
private static final long DEFAULT_LAST_ACCESSED = 0;
private static final long DEFAULT_TOTAL = 0;

/**
* A RuntimeException that wraps a CacheException.
*/
private static class UncheckedCacheException extends RuntimeException
{
private UncheckedCacheException(CacheException cause)
{
super(cause.getMessage(), cause);
}

private CacheException getCacheException()
{
return (CacheException) getCause();
}
}

/**
* The name of IoScheduler.
*/
Expand Down Expand Up @@ -199,7 +215,7 @@ public int getOrCreateMover(MoverSupplier moverSupplier, String doorUniqueId,
try {
return createRequest(moverSupplier, key, priority);
} catch (CacheException e) {
throw new RuntimeException(e);
throw new UncheckedCacheException(e);
}
});

Expand All @@ -217,10 +233,8 @@ public int getOrCreateMover(MoverSupplier moverSupplier, String doorUniqueId,
}

return request.getId();
} catch (RuntimeException e) {
Throwable t = Throwables.getRootCause(e);
Throwables.throwIfInstanceOf(t, CacheException.class);
throw e;
} catch (UncheckedCacheException e) {
throw e.getCacheException();
}
}

Expand Down

0 comments on commit 577a54d

Please sign in to comment.