jakarta-jcs-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Rall <...@collab.net>
Subject Re: cvs commit: jakarta-turbine-jcs/src/java/org/apache/jcs/auxiliary/lateral LateralCacheNoWaitFacade.java LateralCacheNoWait.java LateralCache.java
Date Tue, 27 Apr 2004 20:17:28 GMT
Comments inline.

asmuts@apache.org wrote:
> asmuts      2004/04/27 14:08:25
> 
>   Modified:    src/java/org/apache/jcs/auxiliary/lateral/socket/tcp
>                         LateralTCPSender.java
>                src/java/org/apache/jcs/auxiliary/lateral
>                         LateralCacheNoWaitFacade.java
>                         LateralCacheNoWait.java LateralCache.java
>   Log:
>   I saw the potential for a reported bug--where a get returns the wrong item under heavy
load from a lateral request.  Although I do not recommend using laterals to get, it should
work.
>   
>   I put the operations that must occur in order in a synchronized block, insuring that
gets from one lateral to another will not get mixed.  Needs to be improved, but should prevent
any wrong elements from being returned.  . . .
>   
>   Get timeout failures now zombie the lateral and start recovery.  Otherwise, since they
do not occur in the background, the cache could grind to a halt.
>   
>   The worst case now is that if another lateral gets hung, a caller trying to get will
block until the timout.  Removes and puts are all synchronous calls.
...
>   --- LateralTCPSender.java	15 Apr 2004 19:22:56 -0000	1.8
>   +++ LateralTCPSender.java	27 Apr 2004 21:08:24 -0000	1.9
>   @@ -75,6 +75,8 @@
>        /** Only block for 5 seconds before timing out on startup. */
>        private final static int openTimeOut = 5000;
>    
>   +    /** Use to synchronize multiple threads that may be trying to get.*/
>   +    private Object getLock = new int[0];

Use of int[] (even an empty one) could imply some semantic meaning which I don't 
think you intend here.  I recommend using "new Object()" in place of the array, 
as its semantics are less likely to be confused.

...
>   @@ -246,6 +259,7 @@
>                    catch ( IOException ioe )
>                    {
>                      log.error( "Problem cleaning socket before send " + socket, ioe
);
>   +                  throw ioe;
>                    }

This exception handling block is probably better implemented using something 
like NestableException from the Jakarta Commons Lang package.  It's often nice 
to be able to tack on some more information to an exception before passing it 
on, without causing duplicate stack traces in the log files and scattering 
exception logging throughout the code base.

...
>                    catch ( IOException ioe )
>                    {
>                        log.error( "Could not open ObjectInputStream to " + socket, ioe
);
>   +                    throw ioe;
>                    }

Ditto.

...
>   @@ -288,8 +303,11 @@
>                    log.error( "Detected problem with connection: " + e );
>                    throw e;
>                }

Ditto.

...
>   --- LateralCache.java	15 Apr 2004 19:22:48 -0000	1.7
>   +++ LateralCache.java	27 Apr 2004 21:08:24 -0000	1.8
>   @@ -142,7 +142,7 @@
>                catch ( Exception e )
>                {
>                    log.error( e );
>   -                // do something with this
>   +                handleException( e, "Failed to get " + key + " from " + cattr.getCacheName()
);

It's useful to quote values which are put into debug strings.  That way, if for 
some reason they contain whitespace or are null, it's quite clear what the 
"inserted value" is.


---------------------------------------------------------------------
To unsubscribe, e-mail: turbine-jcs-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: turbine-jcs-dev-help@jakarta.apache.org


Mime
View raw message