Minor changes to deal with remaining TODOs in this change. I still be…

…lieve there are some ugly code paths that fire off too many exceptions when we tear down a connection. Hopefully, the connection re-write that is starting off will clean up these ugly code paths.
up1 · Oct 9, 2012 · 133b9c7 · 133b9c7
1 parent a28bcd9
commit 133b9c7
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 6 deletions.
diff --git a/src/java/voldemort/store/socket/clientrequest/ClientRequestExecutorPool.java b/src/java/voldemort/store/socket/clientrequest/ClientRequestExecutorPool.java
@@ -310,8 +310,6 @@ public void handleException(Exception e) {
                 e = new UnreachableStoreException("Failure in " + operationName + ": "
                                                   + e.getMessage(), e);
             try {
-                // TODO: when can callback end up being null? HAs something to
-                // do with destroying resources. --JJW
                 callback.requestComplete(e, 0);
             } catch(Exception ex) {
                 if(logger.isEnabledFor(Level.WARN))
@@ -384,10 +382,10 @@ public void complete() {
             } catch(Exception e) {
                 invokeCallback(e, (System.nanoTime() - startNs) / Time.NS_PER_MS);
             } finally {
-                // TODO: checkin can throw an exception. should "iscomplete" be
-                // set before the call to checkin?
-                checkin(destination, clientRequestExecutor);
                 isComplete = true;
+                // checkin may throw a (new) exception. Any prior exception
+                // has been passed off via invokeCallback.
+                checkin(destination, clientRequestExecutor);
             }
         }
 

diff --git a/src/java/voldemort/utils/pool/QueuedKeyedResourcePool.java b/src/java/voldemort/utils/pool/QueuedKeyedResourcePool.java
@@ -230,7 +230,7 @@ protected void destroyRequest(AsyncResourceRequest<V> resourceRequest) {
      * @param requestQueue The queue for which all resource requests are to be
      *        destroyed.
      */
-    private void destroyRequestQueue(Queue<AsyncResourceRequest<V>> requestQueue) {
+    private synchronized void destroyRequestQueue(Queue<AsyncResourceRequest<V>> requestQueue) {
         AsyncResourceRequest<V> resourceRequest = requestQueue.poll();
         while(resourceRequest != null) {
             destroyRequest(resourceRequest);