You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by je...@apache.org on 2019/03/26 13:00:37 UTC
[geode] branch develop updated: GEODE-6559: PdxInstance.getObject()
is using class from older jar in case of Reconnect (#3353)
This is an automated email from the ASF dual-hosted git repository.
jensdeppe pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 17a949b GEODE-6559: PdxInstance.getObject() is using class from older jar in case of Reconnect (#3353)
17a949b is described below
commit 17a949b9379a680af066b0a249ad475014f86331
Author: Jens Deppe <jd...@pivotal.io>
AuthorDate: Tue Mar 26 06:00:24 2019 -0700
GEODE-6559: PdxInstance.getObject() is using class from older jar in case of Reconnect (#3353)
- TypeRegistry needs to be cleared during reconnect
- Fix ServerStarterRule to not blindly disable some PDX attributes on server start
- Currently configuring PDX, via gfsh, does not work correctly in
conjunction with this rule.
Authored-by: Jens Deppe <jd...@pivotal.io>
---
...rConfigServerRestartWithJarDeployDUnitTest.java | 134 +++++++++++++++++++++
...erConfigServerRestartWithJarDeployFunction.java | 63 ++++++++++
.../org/apache/geode/internal/JarDeployer.java | 2 +-
.../messages/ConfigurationResponse.java | 11 +-
.../geode/pdx/internal/PeerTypeRegistration.java | 7 ++
.../configuration/ClusterConfigTestBase.java | 2 +-
.../geode/test/junit/rules/MemberStarterRule.java | 4 +
.../geode/test/junit/rules/ServerStarterRule.java | 12 +-
.../apache/geode/test/compiler/JarBuilderTest.java | 12 +-
.../org/apache/geode/test/compiler/JarBuilder.java | 9 ++
10 files changed, 242 insertions(+), 14 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigServerRestartWithJarDeployDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigServerRestartWithJarDeployDUnitTest.java
new file mode 100644
index 0000000..b83e484
--- /dev/null
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/internal/configuration/ClusterConfigServerRestartWithJarDeployDUnitTest.java
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.geode.management.internal.configuration;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+import java.util.Properties;
+import java.util.Set;
+
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.apache.geode.cache.execute.FunctionException;
+import org.apache.geode.cache.execute.FunctionInvocationTargetException;
+import org.apache.geode.cache.execute.FunctionService;
+import org.apache.geode.cache.execute.ResultCollector;
+import org.apache.geode.distributed.internal.membership.InternalDistributedMember;
+import org.apache.geode.internal.ClassPathLoader;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.test.compiler.JarBuilder;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+import org.apache.geode.test.junit.rules.serializable.SerializableTemporaryFolder;
+import org.apache.geode.util.test.TestUtil;
+
+public class ClusterConfigServerRestartWithJarDeployDUnitTest {
+
+ @Rule
+ public ClusterStartupRule rule = new ClusterStartupRule(5);
+
+ @Rule
+ public GfshCommandRule gfsh = new GfshCommandRule();
+
+ @Rule
+ public SerializableTemporaryFolder temporaryFolder = new SerializableTemporaryFolder();
+
+ @Test
+ public void functionExecutionAfterServerReconnect() throws Exception {
+ IgnoredException.addIgnoredException("org.apache.geode.ForcedDisconnectException: for testing");
+ IgnoredException.addIgnoredException("cluster configuration service not available");
+ IgnoredException.addIgnoredException("This thread has been stalled");
+ IgnoredException
+ .addIgnoredException("member unexpectedly shut down shared, unordered connection");
+ IgnoredException.addIgnoredException("Connection refused");
+
+ MemberVM locator0 = rule.startLocatorVM(0);
+ gfsh.connectAndVerify(locator0);
+
+ gfsh.executeAndAssertThat(
+ "configure pdx --read-serialized=true --auto-serializable-classes=ClusterConfigServerRestartWithJarDeployFunction.*");
+
+ Properties props = new Properties();
+ MemberVM server1 = rule.startServerVM(1, props, locator0.getPort());
+ MemberVM server2 = rule.startServerVM(2, props, locator0.getPort());
+
+ File functionJar = getFunctionJar();
+ gfsh.executeAndAssertThat("deploy --jar=" + functionJar.getAbsolutePath()).statusIsSuccess();
+
+ callFunction(server1);
+
+ server2.forceDisconnect();
+
+ server2.waitTilServerFullyReconnected();
+
+ callFunction(server1);
+ }
+
+ private File getFunctionJar() throws IOException {
+ JarBuilder jarBuilder = new JarBuilder();
+ String filePath =
+ TestUtil.getResourcePath(this.getClass(),
+ "/ClusterConfigServerRestartWithJarDeployFunction.java");
+ assertThat(filePath).as("java file resource not found").isNotBlank();
+
+ File functionJar = new File(temporaryFolder.newFolder(), "output.jar");
+ jarBuilder.buildJar(functionJar, new File(filePath));
+
+ return functionJar;
+ }
+
+ private void callFunction(MemberVM member) {
+ member.invoke(() -> {
+ while (true) {
+ try {
+ Set<InternalDistributedMember> others =
+ ClusterStartupRule.getCache().getDistributionManager()
+ .getOtherNormalDistributionManagerIds();
+ InternalDistributedMember otherMember = others.stream().findFirst().get();
+
+ Class<?> studentClass = ClassPathLoader.getLatest()
+ .forName("ClusterConfigServerRestartWithJarDeployFunction$Student");
+
+ Object student = studentClass.getConstructor().newInstance();
+
+ ResultCollector collector = FunctionService.onMember(otherMember)
+ .setArguments(student)
+ .execute("student-function");
+
+ List<Object> results = (List<Object>) collector.getResult();
+ break;
+ } catch (FunctionException fex) {
+ if (fex.getCause() instanceof FunctionInvocationTargetException) {
+ LogService.getLogger().info("Sleeping for 500ms after recoverable exception {}",
+ fex.getMessage());
+ Thread.sleep(500);
+ } else {
+ fail("Exception received from function execution: %s", fex.getMessage());
+ throw fex;
+ }
+ }
+ }
+ });
+ }
+
+}
diff --git a/geode-core/src/distributedTest/resources/ClusterConfigServerRestartWithJarDeployFunction.java b/geode-core/src/distributedTest/resources/ClusterConfigServerRestartWithJarDeployFunction.java
new file mode 100644
index 0000000..9813b9e
--- /dev/null
+++ b/geode-core/src/distributedTest/resources/ClusterConfigServerRestartWithJarDeployFunction.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+import org.apache.logging.log4j.Logger;
+
+import org.apache.geode.cache.execute.Function;
+import org.apache.geode.cache.execute.FunctionContext;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.pdx.PdxInstance;
+
+public class ClusterConfigServerRestartWithJarDeployFunction implements Function {
+
+ private static final Logger logger = LogService.getLogger();
+
+ public static class Student {
+ public Student() {}
+ }
+
+ @Override
+ public void execute(FunctionContext context) {
+ Object o = context.getArguments();
+ Object studentObject = ((PdxInstance) o).getObject();
+
+ logger.info("--->>> StudentObject = {}", studentObject);
+ logger.info("--->>> Student Loaded from {} using ClassLoader {}",
+ jarForClass(studentObject.getClass()),
+ studentObject.getClass().getClassLoader());
+ logger.info("--->>> Class<Student> Loaded from {} using ClassLoader {}",
+ jarForClass(Student.class),
+ Student.class.getClassLoader());
+
+ // Previously this would cause a ClassCastException if this was called after a reconnect
+ Student student = (Student) ((PdxInstance) o).getObject();
+
+ context.getResultSender().lastResult(true);
+ }
+
+ private String jarForClass(Class<?> clazz) {
+ ClassLoader loader = clazz.getClassLoader();
+ String className = clazz.getName();
+ String from = loader.getResource(className.replace('.', '/') +
+ ".class").toString();
+
+ return from;
+ }
+
+ @Override
+ public String getId() {
+ return "student-function";
+ }
+}
diff --git a/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java b/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
index 2778d68..24e1633 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java
@@ -64,7 +64,7 @@ public class JarDeployer implements Serializable {
private final File deployDirectory;
public JarDeployer() {
- this.deployDirectory = new File(System.getProperty("user.dir"));
+ this(new File(System.getProperty("user.dir")));
}
public JarDeployer(final File deployDirectory) {
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java
index 5ba37e8..4fc10d8 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/configuration/messages/ConfigurationResponse.java
@@ -101,14 +101,13 @@ public class ConfigurationResponse implements DataSerializableFixedID {
if (config != null) {
sb.append("\n***************************************************************");
sb.append("\nConfiguration for '" + configType + "'");
- sb.append("\n\nJar files to deployed");
+ sb.append("\n\nJar files to be deployed:");
Set<String> jarNames = config.getJarNames();
- Iterator<String> jarIter = jarNames.iterator();
- int jarCounter = 0;
-
- while (jarIter.hasNext()) {
- sb.append("\n" + ++jarCounter + "." + jarIter.next());
+ if (jarNames.size() == 0) {
+ sb.append("\n None");
+ } else {
+ jarNames.forEach(c -> sb.append("\n " + c));
}
try {
diff --git a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
index 7d6d557..0aae7bf 100644
--- a/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
+++ b/geode-core/src/main/java/org/apache/geode/pdx/internal/PeerTypeRegistration.java
@@ -127,6 +127,13 @@ public class PeerTypeRegistration implements TypeRegistration {
@Override
public void initialize() {
+ // Relevant during reconnect
+ TypeRegistry typeRegistry = cache.getPdxRegistry();
+ if (typeRegistry != null) {
+ typeRegistry.flushCache();
+ logger.debug("Flushing TypeRegistry");
+ }
+
AttributesFactory<Object, Object> factory = new AttributesFactory<Object, Object>();
factory.setScope(Scope.DISTRIBUTED_ACK);
if (cache.getPdxPersistent()) {
diff --git a/geode-dunit/src/main/java/org/apache/geode/management/internal/configuration/ClusterConfigTestBase.java b/geode-dunit/src/main/java/org/apache/geode/management/internal/configuration/ClusterConfigTestBase.java
index 5831977..d6474c8 100644
--- a/geode-dunit/src/main/java/org/apache/geode/management/internal/configuration/ClusterConfigTestBase.java
+++ b/geode-dunit/src/main/java/org/apache/geode/management/internal/configuration/ClusterConfigTestBase.java
@@ -118,7 +118,7 @@ public abstract class ClusterConfigTestBase {
return file;
}
- protected String createJarFileWithClass(String className, String jarName, File dir)
+ protected static String createJarFileWithClass(String className, String jarName, File dir)
throws IOException {
File jarFile = new File(dir, jarName);
new ClassBuilder().writeJarFromName(className, jarFile);
diff --git a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
index d04c4f2..4ca92b6 100644
--- a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
+++ b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/MemberStarterRule.java
@@ -72,6 +72,7 @@ import org.apache.geode.management.api.ClusterManagementService;
import org.apache.geode.management.internal.MBeanJMXAdapter;
import org.apache.geode.management.internal.SystemManagementService;
import org.apache.geode.management.internal.cli.CliUtil;
+import org.apache.geode.pdx.internal.TypeRegistry;
import org.apache.geode.security.SecurityManager;
import org.apache.geode.security.templates.UserPasswordAuthInit;
import org.apache.geode.test.awaitility.GeodeAwaitility;
@@ -157,6 +158,9 @@ public abstract class MemberStarterRule<T> extends SerializableExternalResource
// future tests
SocketCreatorFactory.close();
+ // This is required if PDX is in use and tests are run repeatedly.
+ TypeRegistry.init();
+
// delete the first-level children files that are created in the tests
if (cleanWorkingDir)
Arrays.stream(getWorkingDir().listFiles())
diff --git a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/ServerStarterRule.java b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/ServerStarterRule.java
index 0c8b89d..6f46b0b 100644
--- a/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/ServerStarterRule.java
+++ b/geode-dunit/src/main/java/org/apache/geode/test/junit/rules/ServerStarterRule.java
@@ -57,8 +57,10 @@ public class ServerStarterRule extends MemberStarterRule<ServerStarterRule> impl
private transient CacheServer server;
private int embeddedLocatorPort = -1;
private boolean pdxPersistent = false;
+ private boolean pdxPersistentUserSet = false;
private PdxSerializer pdxSerializer = null;
private boolean pdxReadSerialized = false;
+ private boolean pdxReadSerializedUserSet = false;
private boolean noCacheServer = false;
private Map<String, RegionShortcut> regions = new HashMap<>();
@@ -103,11 +105,13 @@ public class ServerStarterRule extends MemberStarterRule<ServerStarterRule> impl
public ServerStarterRule withPDXPersistent() {
pdxPersistent = true;
+ pdxPersistentUserSet = true;
return this;
}
public ServerStarterRule withPDXReadSerialized() {
pdxReadSerialized = true;
+ pdxReadSerializedUserSet = true;
return this;
}
@@ -166,8 +170,12 @@ public class ServerStarterRule extends MemberStarterRule<ServerStarterRule> impl
public void startServer() {
CacheFactory cf = new CacheFactory(this.properties);
- cf.setPdxPersistent(pdxPersistent);
- cf.setPdxReadSerialized(pdxReadSerialized);
+ if (pdxPersistentUserSet) {
+ cf.setPdxPersistent(pdxPersistent);
+ }
+ if (pdxReadSerializedUserSet) {
+ cf.setPdxReadSerialized(pdxReadSerialized);
+ }
if (pdxSerializer != null) {
cf.setPdxSerializer(pdxSerializer);
}
diff --git a/geode-junit/src/integrationTest/java/org/apache/geode/test/compiler/JarBuilderTest.java b/geode-junit/src/integrationTest/java/org/apache/geode/test/compiler/JarBuilderTest.java
index 21d93d1..9a2f0d8 100644
--- a/geode-junit/src/integrationTest/java/org/apache/geode/test/compiler/JarBuilderTest.java
+++ b/geode-junit/src/integrationTest/java/org/apache/geode/test/compiler/JarBuilderTest.java
@@ -53,8 +53,9 @@ public class JarBuilderTest {
jarBuilder.buildJar(outputJar, classContents);
Set<String> jarEntryNames = jarEntryNamesFromFile(outputJar);
- assertThat(jarEntryNames)
- .containsExactlyInAnyOrder("org/apache/geode/test/compiler/AbstractClass.class");
+ assertThat(jarEntryNames).containsExactlyInAnyOrder(
+ "timestamp",
+ "org/apache/geode/test/compiler/AbstractClass.class");
}
@Test
@@ -67,6 +68,7 @@ public class JarBuilderTest {
Set<String> jarEntryNames = jarEntryNamesFromFile(outputJar);
assertThat(jarEntryNames).containsExactlyInAnyOrder(
+ "timestamp",
"org/apache/geode/test/compiler/AbstractClass.class",
"org/apache/geode/test/compiler/ConcreteClass.class");
}
@@ -78,7 +80,9 @@ public class JarBuilderTest {
jarBuilder.buildJar(outputJar, classInFooBarPackage, classInDefaultPackage);
Set<String> jarEntryNames = jarEntryNamesFromFile(outputJar);
- assertThat(jarEntryNames).containsExactlyInAnyOrder("ClassInDefaultPackage.class",
+ assertThat(jarEntryNames).containsExactlyInAnyOrder(
+ "timestamp",
+ "ClassInDefaultPackage.class",
"foo/bar/ClassInFooBarPackage.class");
}
@@ -91,7 +95,7 @@ public class JarBuilderTest {
Set<String> jarEntryNames = jarEntryNamesFromFile(outputJar);
assertThat(jarEntryNames).containsExactlyInAnyOrder("DefaultClass.class",
- "foo/bar/OtherClass.class");
+ "foo/bar/OtherClass.class", "timestamp");
}
@Test
diff --git a/geode-junit/src/main/java/org/apache/geode/test/compiler/JarBuilder.java b/geode-junit/src/main/java/org/apache/geode/test/compiler/JarBuilder.java
index 0c22801..96ef3a5 100644
--- a/geode-junit/src/main/java/org/apache/geode/test/compiler/JarBuilder.java
+++ b/geode-junit/src/main/java/org/apache/geode/test/compiler/JarBuilder.java
@@ -112,6 +112,15 @@ public class JarBuilder {
jarOutputStream.write(compiledSource.compiledBytecode);
jarOutputStream.closeEntry();
}
+
+ // Add timestamp so that the jar file itself has a new MD5 signature
+ JarEntry timestampEntry = new JarEntry("timestamp");
+ long now = System.currentTimeMillis();
+ timestampEntry.setTime(now);
+ jarOutputStream.putNextEntry(timestampEntry);
+ jarOutputStream.write(Long.toString(now).getBytes());
+ jarOutputStream.closeEntry();
+
jarOutputStream.close();
}
}