tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r1326941 - in /tomcat/tc7.0.x/trunk: java/org/apache/catalina/ha/session/ java/org/apache/catalina/tribes/tipis/ webapps/docs/
Date Wed, 18 Apr 2012 19:05:28 GMT
2012/4/17  <kfujino@apache.org>:
> Author: kfujino
> Date: Tue Apr 17 06:23:21 2012
> New Revision: 1326941
>
> URL: http://svn.apache.org/viewvc?rev=1326941&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53087.
> In order to avoid that a backup node expire a session, replicate session access time
in BackupManager.
>
> Modified:
>    tomcat/tc7.0.x/trunk/java/org/apache/catalina/ha/session/DeltaSession.java
>    tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/tipis/AbstractReplicatedMap.java
>    tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/tipis/ReplicatedMapEntry.java
>    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
>

(...)

> Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/tipis/AbstractReplicatedMap.java
> URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/tipis/AbstractReplicatedMap.java?rev=1326941&r1=1326940&r2=1326941&view=diff
> ==============================================================================
> --- tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/tipis/AbstractReplicatedMap.java
(original)
> +++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/tribes/tipis/AbstractReplicatedMap.java
Tue Apr 17 06:23:21 2012
> @@ -402,8 +402,10 @@ public abstract class AbstractReplicated
>         if ( !entry.isSerializable() ) return;
>         if (entry.isPrimary() && entry.getBackupNodes()!= null &&
entry.getBackupNodes().length > 0) {
>             Object value = entry.getValue();
> -            //check to see if we need to replicate this object isDirty()||complete
> -            boolean repl = complete || ( (value instanceof ReplicatedMapEntry)
&& ( (ReplicatedMapEntry) value).isDirty());
> +            //check to see if we need to replicate this object isDirty()||complete
|| isAccessReplicate()
> +            boolean isDirty = ((value instanceof ReplicatedMapEntry) &&
((ReplicatedMapEntry) value).isDirty());
> +            boolean isAccess = ((value instanceof ReplicatedMapEntry) &&
((ReplicatedMapEntry) value).isAccessReplicate());
> +            boolean repl = complete || isDirty || isAccess;
>
>             if (!repl) {
>                 if ( log.isTraceEnabled() )
> @@ -412,9 +414,9 @@ public abstract class AbstractReplicated
>                 return;
>             }
>             //check to see if the message is diffable
> -            boolean diff = ( (value instanceof ReplicatedMapEntry) &&
( (ReplicatedMapEntry) value).isDiffable());
> +            boolean diff = ((value instanceof ReplicatedMapEntry) && ((ReplicatedMapEntry)
value).isDiffable());
>             MapMessage msg = null;
> -            if (diff) {
> +            if (diff && isDirty) {


What happens if "repl" variable is true because of "complete" being true?
Shouldn't it be
    if (diff && (isDirty || complete))
?

I do not quite understand the code, but it seems that omitting
"complete" here will change its behaviour.

E.g. DeltaSession.isDiffable() always returns true, and thus I think
the old "if(diff)" block was executed regardless of the value of
"complete".


>                 ReplicatedMapEntry rentry = (ReplicatedMapEntry)entry.getValue();

The above local variable could be moved outside this block, to avoid
multiple (instanceof) checks above.

>                 try {
>                     rentry.lock();
> @@ -432,6 +434,12 @@ public abstract class AbstractReplicated
>                 }
>
>             }
> +            if (msg == null && isAccess) {
> +                //construct a access message
> +                msg = new MapMessage(mapContextName, MapMessage.MSG_ACCESS,
> +                        false, (Serializable) entry.getKey(), null, null,
entry.getPrimary(),
> +                        entry.getBackupNodes());
> +            }
>             if (msg == null) {
>                 //construct a complete
>                 msg = new MapMessage(mapContextName, MapMessage.MSG_BACKUP,
> @@ -442,6 +450,9 @@ public abstract class AbstractReplicated
>             }
>             try {
>                 if ( channel!=null && entry.getBackupNodes()!= null &&
entry.getBackupNodes().length > 0 ) {
> +                    if ((entry.getValue() instanceof ReplicatedMapEntry))
{
> +                        ((ReplicatedMapEntry)entry.getValue()).setLastTimeReplicated(System.currentTimeMillis());
> +                    }
>                     channel.send(entry.getBackupNodes(), msg, channelSendOptions);
>                 }
>             } catch (ChannelException x) {
> @@ -670,6 +681,17 @@ public abstract class AbstractReplicated
>             } //end if
>             super.put(entry.getKey(), entry);
>         } //end if
> +
> +        if (mapmsg.getMsgType() == MapMessage.MSG_ACCESS) {
> +            MapEntry entry = (MapEntry)super.get(mapmsg.getKey());
> +            if (entry != null) {
> +                entry.setBackupNodes(mapmsg.getBackupNodes());
> +                entry.setPrimary(mapmsg.getPrimary());
> +                if (entry.getValue() instanceof ReplicatedMapEntry) {
> +                    ((ReplicatedMapEntry) entry.getValue()).accessEntry();
> +                }
> +            }
> +        }
>     }
>
>     @Override
> @@ -1277,6 +1299,7 @@ public abstract class AbstractReplicated
>         public static final int MSG_INIT = 8;
>         public static final int MSG_COPY = 9;
>         public static final int MSG_STATE_COPY = 10;
> +        public static final int MSG_ACCESS = 11;
>
>         private byte[] mapId;
>         private int msgtype;
> @@ -1314,6 +1337,7 @@ public abstract class AbstractReplicated
>                 case MSG_INIT: return "MSG_INIT";
>                 case MSG_STATE_COPY: return "MSG_STATE_COPY";
>                 case MSG_COPY: return "MSG_COPY";
> +                case MSG_ACCESS: return "MSG_ACCESS";
>                 default : return "UNKNOWN";
>             }
>         }
>

Best regards,
Konstantin Kolinko

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


Mime
View raw message