Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix RoutedStore.getAll to pass Throwable subclasses that are not Erro…
…r subclasses to InsufficientOperationalNodesException.

To make the above possible, change InsufficientOperationalNodesException to hold a collection of Throwable subclasses instead of Exception subclasses.
  • Loading branch information
ijuma committed Jul 14, 2009
1 parent 776ec8c commit f232537
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 17 deletions.
Expand Up @@ -29,9 +29,9 @@ public class InsufficientOperationalNodesException extends StoreOperationFailure

private static final long serialVersionUID = 1L;

private Collection<? extends Exception> causes;
private Collection<? extends Throwable> causes;

public InsufficientOperationalNodesException(String s, Exception e) {
public InsufficientOperationalNodesException(String s, Throwable e) {
super(s, e);
causes = Collections.singleton(e);
}
Expand All @@ -41,22 +41,22 @@ public InsufficientOperationalNodesException(String s) {
causes = Collections.emptyList();
}

public InsufficientOperationalNodesException(Exception e) {
public InsufficientOperationalNodesException(Throwable e) {
super(e);
causes = Collections.singleton(e);
}

public InsufficientOperationalNodesException(Collection<? extends Exception> failures) {
public InsufficientOperationalNodesException(Collection<? extends Throwable> failures) {
this("Insufficient operational nodes to immediately satisfy request.", failures);
}

public InsufficientOperationalNodesException(String message,
Collection<? extends Exception> failures) {
Collection<? extends Throwable> failures) {
super(message, failures.size() > 0 ? failures.iterator().next() : null);
this.causes = failures;
}

public Collection<? extends Exception> getCauses() {
public Collection<? extends Throwable> getCauses() {
return this.causes;
}

Expand Down
23 changes: 12 additions & 11 deletions src/java/voldemort/store/routed/RoutedStore.java
Expand Up @@ -304,7 +304,7 @@ public Map<ByteArray, List<Versioned<byte[]>>> getAll(Iterable<ByteArray> keys)
}

// A list of thrown exceptions, indicating the number of failures
List<Exception> failures = Lists.newArrayList();
List<Throwable> failures = Lists.newArrayList();
List<NodeValue<ByteArray, byte[]>> nodeValues = Lists.newArrayList();

Map<ByteArray, MutableInt> keyToSuccessCount = Maps.newHashMap();
Expand Down Expand Up @@ -357,11 +357,9 @@ public Map<ByteArray, List<Versioned<byte[]>>> getAll(Iterable<ByteArray> keys)
} catch(InterruptedException e) {
throw new InsufficientOperationalNodesException("getAll operation interrupted.", e);
} catch(ExecutionException e) {
// TODO We catch all Exception and subclasses inside
// the Callable as get() does. That means that Error
// subclasses or classes
// that extends Throwable directly escape. What to do about
// those?
// We catch all Throwables apart from Error in the callable, so
// the else part
// should never happen
if(e.getCause() instanceof Error)
throw (Error) e.getCause();
else
Expand Down Expand Up @@ -704,7 +702,7 @@ private boolean isAvailable(Node node) {
return !node.getStatus().isUnavailable(this.nodeBannageMs);
}

private void markUnavailable(Node node, Exception e) {
private void markUnavailable(Node node, UnreachableStoreException e) {
logger.warn("Could not connect to node " + node.getId() + " at " + node.getHost()
+ " marking as unavailable for " + this.nodeBannageMs + " ms.", e);
logger.debug(e);
Expand Down Expand Up @@ -775,7 +773,7 @@ private GetAllCallable(Node node, Collection<ByteArray> nodeKeys) {

public GetAllResult call() {
Map<ByteArray, List<Versioned<byte[]>>> retrieved = Collections.emptyMap();
Exception exception = null;
Throwable exception = null;
List<NodeValue<ByteArray, byte[]>> nodeValues = Lists.newArrayList();
try {
retrieved = innerStores.get(node.getId()).getAll(nodeKeys);
Expand All @@ -790,7 +788,9 @@ public GetAllResult call() {
} catch(UnreachableStoreException e) {
exception = e;
markUnavailable(node, e);
} catch(Exception e) {
} catch(Throwable e) {
if(e instanceof Error)
throw (Error) e;
exception = e;
logger.warn("Error in GET on node " + node.getId() + "(" + node.getHost() + ")", e);
}
Expand All @@ -802,13 +802,14 @@ private static class GetAllResult {

final GetAllCallable callable;
final Map<ByteArray, List<Versioned<byte[]>>> retrieved;
final Exception exception;
/* Note that this can never be an Error subclass */
final Throwable exception;
final List<NodeValue<ByteArray, byte[]>> nodeValues;

private GetAllResult(GetAllCallable callable,
Map<ByteArray, List<Versioned<byte[]>>> retrieved,
List<NodeValue<ByteArray, byte[]>> nodeValues,
Exception exception) {
Throwable exception) {
this.callable = callable;
this.exception = exception;
this.retrieved = retrieved;
Expand Down

0 comments on commit f232537

Please sign in to comment.