You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ds...@apache.org on 2021/01/26 00:18:12 UTC

[geode] branch support/1.12 updated: GEODE-8685: change export to not deserialize region values (#5735) (#5951)

This is an automated email from the ASF dual-hosted git repository.

dschneider pushed a commit to branch support/1.12
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/support/1.12 by this push:
     new 90ec4d3  GEODE-8685: change export to not deserialize region values (#5735) (#5951)
90ec4d3 is described below

commit 90ec4d3d8caf59a8818bf2de6dcab9da614e0996
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Mon Jan 25 16:17:17 2021 -0800

    GEODE-8685: change export to not deserialize region values (#5735) (#5951)
    
    Modified test to fail if it attempts to deserialize values during export.
    Fixed product export code to no longer deserialize and test now passes.
    
    Co-authored-by: Darrel Schneider <ds...@vmware.com>
    (cherry picked from commit 078ceb0187b718d86d6f6e6ec058d2c743e2655a)
---
 .../apache/geode/internal/cache/EntrySnapshot.java | 15 ++++++++++-
 .../internal/cache/snapshot/SnapshotPacket.java    |  5 ++++
 .../cli/commands/ExportDataIntegrationTest.java    | 29 ++++++++++++++++++++--
 3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/EntrySnapshot.java b/geode-core/src/main/java/org/apache/geode/internal/cache/EntrySnapshot.java
index 717c9f0..84809f0 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/EntrySnapshot.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/EntrySnapshot.java
@@ -123,6 +123,20 @@ public class EntrySnapshot implements Region.Entry, DataSerializable {
     return v;
   }
 
+  /**
+   * If the value is available as a CachedDeserializable instance then return it.
+   * Otherwise behave as if getValue() was called.
+   */
+  public Object getValuePreferringCachedDeserializable() {
+    checkEntryDestroyed();
+    Object value = regionEntry.getValue(null);
+    if (value instanceof CachedDeserializable) {
+      return value;
+    } else {
+      return getRawValue();
+    }
+  }
+
   @Override
   public Object getValue() {
     checkEntryDestroyed();
@@ -281,5 +295,4 @@ public class EntrySnapshot implements Region.Entry, DataSerializable {
     }
     this.regionEntry.fromData(in);
   }
-
 }
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/snapshot/SnapshotPacket.java b/geode-core/src/main/java/org/apache/geode/internal/cache/snapshot/SnapshotPacket.java
index 10af540..0d2e3c9 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/snapshot/SnapshotPacket.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/snapshot/SnapshotPacket.java
@@ -25,6 +25,7 @@ import org.apache.geode.cache.EntryDestroyedException;
 import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.internal.InternalDataSerializer;
 import org.apache.geode.internal.cache.CachedDeserializable;
+import org.apache.geode.internal.cache.EntrySnapshot;
 import org.apache.geode.internal.cache.LocalRegion;
 import org.apache.geode.internal.cache.NonTXEntry;
 import org.apache.geode.internal.cache.Token;
@@ -75,6 +76,10 @@ public class SnapshotPacket implements DataSerializableFixedID {
         } finally {
           OffHeapHelper.release(v);
         }
+      } else if (entry instanceof EntrySnapshot) {
+        EntrySnapshot entrySnapshot = (EntrySnapshot) entry;
+        Object entryValue = entrySnapshot.getValuePreferringCachedDeserializable();
+        value = convertToBytes(entryValue);
       } else {
         value = convertToBytes(entry.getValue());
       }
diff --git a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java
index bc5eb9b..9fed376 100644
--- a/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java
+++ b/geode-gfsh/src/integrationTest/java/org/apache/geode/management/internal/cli/commands/ExportDataIntegrationTest.java
@@ -19,6 +19,9 @@ package org.apache.geode.management.internal.cli.commands;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertFalse;
 
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.stream.IntStream;
@@ -29,6 +32,8 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import org.apache.geode.DataSerializable;
+import org.apache.geode.DataSerializer;
 import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionShortcut;
 import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
@@ -52,10 +57,29 @@ public class ExportDataIntegrationTest {
   @Rule
   public TemporaryFolder tempDir = new TemporaryFolder();
 
-  private Region<String, String> region;
+  private Region<String, Object> region;
   private Path snapshotFile;
   private Path snapshotDir;
 
+  public static class StringWrapper implements DataSerializable {
+
+    private String value;
+
+    public StringWrapper(String string) {
+      value = string;
+    }
+
+    @Override
+    public void toData(DataOutput out) throws IOException {
+      DataSerializer.writeString(value, out);
+    }
+
+    @Override
+    public void fromData(DataInput in) throws IOException, ClassNotFoundException {
+      throw new ClassNotFoundException("Should never be deserialized");
+    }
+  }
+
   @Before
   public void setup() throws Exception {
     gfsh.connectAndVerify(server.getEmbeddedLocatorPort(), GfshCommandRule.PortType.locator);
@@ -68,6 +92,7 @@ public class ExportDataIntegrationTest {
 
   @Test
   public void testExport() throws Exception {
+    loadRegion(new StringWrapper("value"));
     String exportCommand = buildBaseExportCommand()
         .addOption(CliStrings.EXPORT_DATA__FILE, snapshotFile.toString()).getCommandString();
     gfsh.executeAndAssertThat(exportCommand).statusIsSuccess();
@@ -152,7 +177,7 @@ public class ExportDataIntegrationTest {
     assertFalse(Files.exists(snapshotDir));
   }
 
-  private void loadRegion(String value) {
+  private void loadRegion(Object value) {
     IntStream.range(0, DATA_POINTS).forEach(i -> region.put("key" + i, value));
   }