glassfish
  1. glassfish
  2. GLASSFISH-15714

synchronous replication does not wait for wait for acknowledgement

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 3.1_b39
    • Fix Version/s: 3.1_b40
    • Component/s: failover
    • Labels:
      None

      Description

      Currently, if I deploy an application with --asyncreplication=false, the response is sent back to the client before ACK is received from the replica instance. To make synchronous replication work properly, the response must be sent only after ACK is received

        Activity

        Hide
        Mahesh Kannan added a comment -

        1. How bad is its impact? (Severity)
        Quite bad. Synchronous replication is broken

        2. How often does it happen? (Frequency)
        All the time when apps are deployed with --asyncreplication=false

        3. How much effort is required to fix it? (Cost)
        Small and The fix is available and reviewed.

        4. What is the risk of fixing it? (Risk)
        Small. It will not break our DFT tests since they use asyncreplication=true

        5. Does a work around for the issue exist? Can the workaround be reasonably employed by the end user?
        No.

        6. If the issue is not fixed should the issue and its workaround (if applicable) be described in the Release Notes?
        No.

        Diffs reviewed by Joe Fialli

        svn diff
        Index: src/main/java/org/shoal/ha/cache/impl/store/ReplicatedDataStore.java
        ===================================================================
        — src/main/java/org/shoal/ha/cache/impl/store/ReplicatedDataStore.java (revision 1473)
        +++ src/main/java/org/shoal/ha/cache/impl/store/ReplicatedDataStore.java (working copy)
        @@ -156,9 +156,7 @@
        Class<V> vClazz = dsc.getValueClazz();
        DataStoreEntryUpdater<K, V> dseUpdater = null;

        • if (dsc.isDoSynchronousReplication()) { - dseUpdater = new SimpleDataStoreEntryUpdater(); - }

          else if (SimpleMetadata.class.isAssignableFrom(vClazz)) {
          + if (SimpleMetadata.class.isAssignableFrom(vClazz))

          { dseUpdater = new SimpleStoreableDataStoreEntryUpdater(); }

          else if (Storeable.class.isAssignableFrom(vClazz)) {
          dseUpdater = new StoreableDataStoreEntryUpdater();
          Index: src/main/java/org/shoal/ha/cache/impl/command/CommandManager.java
          ===================================================================

            • src/main/java/org/shoal/ha/cache/impl/command/CommandManager.java (revision 1473)
              +++ src/main/java/org/shoal/ha/cache/impl/command/CommandManager.java (working copy)
              @@ -120,9 +120,9 @@
              if (forward)
              Unknown macro: { try { head.onTransmit(cmd, initiator); - //cmd.onSuccess(); + cmd.onSuccess(); } catch (DataStoreException dseEx) { - //cmd.onError(dseEx); + cmd.onFailure(); } }

              else {
              tail.onReceive(cmd, initiator);
              Index: src/main/java/org/shoal/ha/cache/impl/interceptor/ReplicationFramePayloadCommand.java
              ===================================================================

            • src/main/java/org/shoal/ha/cache/impl/interceptor/ReplicationFramePayloadCommand.java (revision 1473)
              +++ src/main/java/org/shoal/ha/cache/impl/interceptor/ReplicationFramePayloadCommand.java (working copy)
              @@ -42,14 +42,10 @@

        import org.glassfish.ha.store.util.KeyTransformer;
        import org.shoal.ha.cache.api.DataStoreException;
        -import org.shoal.ha.cache.api.ObjectInputStreamWithLoader;
        import org.shoal.ha.cache.api.ShoalCacheLoggerConstants;
        import org.shoal.ha.cache.impl.command.Command;
        import org.shoal.ha.cache.impl.command.ReplicationCommandOpcode;
        -import org.shoal.ha.cache.impl.util.ReplicationInputStream;
        -import org.shoal.ha.cache.impl.util.ReplicationOutputStream;

        -import java.io.ByteArrayInputStream;
        import java.io.IOException;
        import java.io.ObjectInputStream;
        import java.io.ObjectOutputStream;
        @@ -62,8 +58,7 @@
        /**

        • @author Mahesh Kannan
          */
          -public class
        • ReplicationFramePayloadCommand<K, V>
          +public class ReplicationFramePayloadCommand<K, V>
          extends Command { private transient static final Logger _logger = @@ -179,19 +174,16 @@ getCommandManager().executeCommand(cmd, false, initiator); }

        + int executedRemoveCount = 0;
        if (removedKeys != null) {
        for (K k : removedKeys)

        { - dsc.getReplicaStore().remove(k); + dsc.getReplicaStore().remove(k); + executedRemoveCount++; }
        • }
        • }
        • @Override
        • public void onSuccess() {
        • int sz = commands.size();
        • for (int i = 0; i < sz; i++) {
        • Command cmd = commands.get;
        • cmd.onSuccess();
          + if (dsc.getDataStoreMBean() != null) { + dsc.getDataStoreMBean().updateExecutedRemoveCount(executedRemoveCount); + }

          }
          }

        Index: src/main/java/org/shoal/ha/cache/api/ReplicatedDataStoreStatsHolder.java
        ===================================================================
        — src/main/java/org/shoal/ha/cache/api/ReplicatedDataStoreStatsHolder.java (revision 1473)
        +++ src/main/java/org/shoal/ha/cache/api/ReplicatedDataStoreStatsHolder.java (working copy)
        @@ -268,6 +268,10 @@
        }

        + public int updateExecutedRemoveCount(int delta)

        { + return executedRemoveCount.addAndGet(delta); + }

        +
        //@Override
        public String toString() {
        return "ReplicatedDataStoreStatsHolder{" +

        Show
        Mahesh Kannan added a comment - 1. How bad is its impact? (Severity) Quite bad. Synchronous replication is broken 2. How often does it happen? (Frequency) All the time when apps are deployed with --asyncreplication=false 3. How much effort is required to fix it? (Cost) Small and The fix is available and reviewed. 4. What is the risk of fixing it? (Risk) Small. It will not break our DFT tests since they use asyncreplication=true 5. Does a work around for the issue exist? Can the workaround be reasonably employed by the end user? No. 6. If the issue is not fixed should the issue and its workaround (if applicable) be described in the Release Notes? No. Diffs reviewed by Joe Fialli svn diff Index: src/main/java/org/shoal/ha/cache/impl/store/ReplicatedDataStore.java =================================================================== — src/main/java/org/shoal/ha/cache/impl/store/ReplicatedDataStore.java (revision 1473) +++ src/main/java/org/shoal/ha/cache/impl/store/ReplicatedDataStore.java (working copy) @@ -156,9 +156,7 @@ Class<V> vClazz = dsc.getValueClazz(); DataStoreEntryUpdater<K, V> dseUpdater = null; if (dsc.isDoSynchronousReplication()) { - dseUpdater = new SimpleDataStoreEntryUpdater(); - } else if (SimpleMetadata.class.isAssignableFrom(vClazz)) { + if (SimpleMetadata.class.isAssignableFrom(vClazz)) { dseUpdater = new SimpleStoreableDataStoreEntryUpdater(); } else if (Storeable.class.isAssignableFrom(vClazz)) { dseUpdater = new StoreableDataStoreEntryUpdater(); Index: src/main/java/org/shoal/ha/cache/impl/command/CommandManager.java =================================================================== src/main/java/org/shoal/ha/cache/impl/command/CommandManager.java (revision 1473) +++ src/main/java/org/shoal/ha/cache/impl/command/CommandManager.java (working copy) @@ -120,9 +120,9 @@ if (forward) Unknown macro: { try { head.onTransmit(cmd, initiator); - //cmd.onSuccess(); + cmd.onSuccess(); } catch (DataStoreException dseEx) { - //cmd.onError(dseEx); + cmd.onFailure(); } } else { tail.onReceive(cmd, initiator); Index: src/main/java/org/shoal/ha/cache/impl/interceptor/ReplicationFramePayloadCommand.java =================================================================== src/main/java/org/shoal/ha/cache/impl/interceptor/ReplicationFramePayloadCommand.java (revision 1473) +++ src/main/java/org/shoal/ha/cache/impl/interceptor/ReplicationFramePayloadCommand.java (working copy) @@ -42,14 +42,10 @@ import org.glassfish.ha.store.util.KeyTransformer; import org.shoal.ha.cache.api.DataStoreException; -import org.shoal.ha.cache.api.ObjectInputStreamWithLoader; import org.shoal.ha.cache.api.ShoalCacheLoggerConstants; import org.shoal.ha.cache.impl.command.Command; import org.shoal.ha.cache.impl.command.ReplicationCommandOpcode; -import org.shoal.ha.cache.impl.util.ReplicationInputStream; -import org.shoal.ha.cache.impl.util.ReplicationOutputStream; -import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; @@ -62,8 +58,7 @@ /** @author Mahesh Kannan */ -public class ReplicationFramePayloadCommand<K, V> +public class ReplicationFramePayloadCommand<K, V> extends Command { private transient static final Logger _logger = @@ -179,19 +174,16 @@ getCommandManager().executeCommand(cmd, false, initiator); } + int executedRemoveCount = 0; if (removedKeys != null) { for (K k : removedKeys) { - dsc.getReplicaStore().remove(k); + dsc.getReplicaStore().remove(k); + executedRemoveCount++; } } } @Override public void onSuccess() { int sz = commands.size(); for (int i = 0; i < sz; i++) { Command cmd = commands.get ; cmd.onSuccess(); + if (dsc.getDataStoreMBean() != null) { + dsc.getDataStoreMBean().updateExecutedRemoveCount(executedRemoveCount); + } } } Index: src/main/java/org/shoal/ha/cache/api/ReplicatedDataStoreStatsHolder.java =================================================================== — src/main/java/org/shoal/ha/cache/api/ReplicatedDataStoreStatsHolder.java (revision 1473) +++ src/main/java/org/shoal/ha/cache/api/ReplicatedDataStoreStatsHolder.java (working copy) @@ -268,6 +268,10 @@ } + public int updateExecutedRemoveCount(int delta) { + return executedRemoveCount.addAndGet(delta); + } + //@Override public String toString() { return "ReplicatedDataStoreStatsHolder{" +
        Hide
        Chris Kasso added a comment -

        Approved for 3.1.

        Show
        Chris Kasso added a comment - Approved for 3.1.
        Hide
        Mahesh Kannan added a comment -

        svn commit -m "Integrate 1.5.29 version. Fixes issue 15714. QL passed"
        Sending packager/resources/pkg_conf.py
        Sending pom.xml
        Transmitting file data ..
        Committed revision 44750.

        Show
        Mahesh Kannan added a comment - svn commit -m "Integrate 1.5.29 version. Fixes issue 15714. QL passed" Sending packager/resources/pkg_conf.py Sending pom.xml Transmitting file data .. Committed revision 44750.

          People

          • Assignee:
            Mahesh Kannan
            Reporter:
            Mahesh Kannan
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: