Skip to content

Commit

Permalink
Minor fixes to various comments to clarify some implementation/usage …
Browse files Browse the repository at this point in the history
…details.
  • Loading branch information
jayjwylie committed Oct 10, 2012
1 parent 31bc42e commit 0df53c0
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 12 deletions.
11 changes: 6 additions & 5 deletions src/java/voldemort/store/stats/ClientSocketStats.java
Expand Up @@ -84,7 +84,8 @@ public class ClientSocketStats {
*/
public ClientSocketStats(ClientSocketStats parent,
SocketDestination destination,
QueuedKeyedResourcePool<SocketDestination, ClientRequestExecutor> pool, int jmxId) {
QueuedKeyedResourcePool<SocketDestination, ClientRequestExecutor> pool,
int jmxId) {
this.parent = parent;
this.statsMap = null;
this.destination = destination;
Expand Down Expand Up @@ -243,13 +244,13 @@ public Histogram getCheckoutWaitUsHistogram() {
}

/**
* @return -1 if there have been no checkout invocations
* @return 0 if there have been no checkout invocations
*/
public long getAvgCheckoutWaitUs() {
long count = checkoutCount.get();
if(count > 0)
return totalCheckoutTimeUs.get() / count;
return -1;
return 0;
}

public Histogram getCheckoutQueueLengthHistogram() {
Expand All @@ -267,13 +268,13 @@ public Histogram getResourceRequestWaitUsHistogram() {
}

/**
* @return -1 if there have been no resourceRequest invocations
* @return 0 if there have been no resourceRequest invocations
*/
public long getAvgResourceRequestWaitUs() {
long count = resourceRequestCount.get();
if(count > 0)
return totalResourceRequestTimeUs.get() / count;
return -1;
return 0;
}

public Histogram getResourceRequestQueueLengthHistogram() {
Expand Down
15 changes: 10 additions & 5 deletions src/java/voldemort/utils/pool/AsyncResourceRequest.java
Expand Up @@ -17,25 +17,30 @@

/**
* Interface for asynchronous requests for resources. Exactly one of
* useResource, handleTimeout, or handleException expected to be invoked within
* deadline specified by getDeadlineNs.
* useResource, handleTimeout, or handleException expected to be invoked before,
* or soon after, deadline specified by getDeadlineNs. Ideally, useResource is
* only invoked before the deadline. Ideally, handleTimeout is invoked soon
* after the deadline. If owners of an object with this interface need to take
* action after some specified timeout, then the owner needs to set their own
* timer.
*/
public interface AsyncResourceRequest<V> {

/**
* To be invoked with resource to use.
* To be invoked with resource to use before deadline.
*
* @param resource. resource should not be null.
*/
void useResource(V resource);

/**
* Invoked sometime after deadline.
* Invoked sometime (soon) after deadline.
*/
void handleTimeout();

/**
* Invoked upon exception trying to process resource request.
* Invoked upon exception trying to process resource request. Invoked before
* or (soon) after the deadline.
*
* @param e
*/
Expand Down
7 changes: 6 additions & 1 deletion src/java/voldemort/utils/pool/QueuedKeyedResourcePool.java
Expand Up @@ -103,7 +103,8 @@ public void registerResourceRequest(K key, AsyncResourceRequest<V> resourceReque

Queue<AsyncResourceRequest<V>> requestQueue = getRequestQueueForKey(key);
if(requestQueue.isEmpty()) {
// Attempt non-blocking checkout iff requestQueue is empty.
// Optimistically attempt non-blocking checkout iff requestQueue is
// empty.
Pool<V> resourcePool = getResourcePoolForKey(key);
try {
attemptGrow(key, resourcePool);
Expand Down Expand Up @@ -231,6 +232,10 @@ public void checkin(K key, V resource) throws Exception {
protected void destroyRequest(AsyncResourceRequest<V> resourceRequest) {
if(resourceRequest != null) {
try {
// To hand control back to the owner of the
// AsyncResourceRequest, treat "destroy" as an exception since
// there is no resource to pass into useResource, and the
// timeout has not expired.
Exception e = new UnreachableStoreException("Resource request destroyed before resource checked out.");
resourceRequest.handleException(e);
} catch(Exception ex) {
Expand Down
Expand Up @@ -380,7 +380,9 @@ public void testRORebalanceWithReplication() throws Exception {
// SocketServer.java fires with an error message like the following:
// "[18:46:32,994 voldemort.server.socket.SocketServer[admin-server]]
// ERROR Too many open connections, 20 of 20 threads in use, denying
// connection from /127.0.0.1:43756 [Thread-552]"
// connection from /127.0.0.1:43756 [Thread-552]". Note, this issues
// seems to only affect ThreadPoolBasedNonblockingStoreImpl tests rather
// than Nio-based tests.
Map<String, String> configProps = new HashMap<String, String>();
configProps.put("admin.max.threads", "50");
currentCluster = startServers(currentCluster,
Expand Down

0 comments on commit 0df53c0

Please sign in to comment.