You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by zh...@apache.org on 2019/08/01 17:23:00 UTC
[geode] branch develop updated: GEODE-6973: Use cachelistener to synchronize typeToId with IdToType r… (#3853)
This is an automated email from the ASF dual-hosted git repository.
zhouxj 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 181e3a4 GEODE-6973: Use cachelistener to synchronize typeToId with IdToType r… (#3853)
181e3a4 is described below
commit 181e3a4a465aa9f5e06f39cd1285c94f9bc78600
Author: Xiaojian Zhou <ge...@users.noreply.github.com>
AuthorDate: Thu Aug 1 10:22:41 2019 -0700
GEODE-6973: Use cachelistener to synchronize typeToId with IdToType r… (#3853)
* GEODE-6973: Use cachelistener to synchronize typeToId with IdToType region in
PeerTypeRegistrationm, then when creating a new json pdxType, no need
to look up in IdToType region. This will speed up creating new pdxType.
Co-authored-by: Xiaojian Zhou <gz...@pivotal.io>
Co-authored-by: Donal Evans <do...@pivotal.io>
---
.../geode/pdx/PdxTypeGenerationDUnitTest.java | 320 +++++++++++++++++++++
.../org/apache/geode/pdx/jsonStrings/testJSON.txt | 43 +++
.../apache/geode/pdx/JSONFormatterJUnitTest.java | 247 +++++++++++++++-
.../PeerTypeRegistrationIntegrationTest.java | 92 ++++++
.../geode/internal/pdx/jsonStrings/testJSON.txt | 43 +++
.../geode/pdx/internal/PeerTypeRegistration.java | 47 +--
6 files changed, 762 insertions(+), 30 deletions(-)
diff --git a/geode-core/src/distributedTest/java/org/apache/geode/pdx/PdxTypeGenerationDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/pdx/PdxTypeGenerationDUnitTest.java
new file mode 100644
index 0000000..d0fc78a
--- /dev/null
+++ b/geode-core/src/distributedTest/java/org/apache/geode/pdx/PdxTypeGenerationDUnitTest.java
@@ -0,0 +1,320 @@
+/*
+ * 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.pdx;
+
+import static org.apache.geode.test.util.ResourceUtils.createTempFileFromResource;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import util.TestException;
+
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientRegionShortcut;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.pdx.internal.PeerTypeRegistration;
+import org.apache.geode.test.dunit.AsyncInvocation;
+import org.apache.geode.test.dunit.rules.ClientVM;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+
+public class PdxTypeGenerationDUnitTest {
+
+ private static final float TARGET_CONSISTENCY_FACTOR = 2f;
+
+ private static final String FIELD_NAME_TO_REPLACE = "maxUrlLength";
+
+ private MemberVM locator, server1, server2;
+
+ @Rule
+ public ClusterStartupRule cluster = new ClusterStartupRule();
+
+ @Before
+ public void before() {
+ Properties props = new Properties();
+ props.setProperty("log-level", "WARN");
+
+ locator = cluster.startLocatorVM(0, props);
+
+ int locatorPort1 = locator.getPort();
+ server1 = cluster.startServerVM(1,
+ x -> x.withProperties(props).withConnectionToLocator(locatorPort1));
+
+ int locatorPort2 = locator.getPort();
+ server2 = cluster.startServerVM(2,
+ x -> x.withProperties(props).withConnectionToLocator(locatorPort2));
+ }
+
+ @Test
+ public void testSortJSONFieldNamesDoesNotImpactPerformance() {
+ final String fileName = "/org/apache/geode/pdx/jsonStrings/testJSON.txt";
+ String jsonString = loadJSONFileAsString(fileName);
+
+ server1.invoke(() -> {
+ int repeats = 1000;
+ long totalUnsortedTime = 0;
+ long totalSortedTime = 0;
+
+ // The first JSON document used to create a PdxInstance takes significantly longer than
+ // successive ones, so a dummy document is used to trigger this first, longer creation process
+ JSONFormatter.fromJSON("{\"testFieldName\": \"test\"}");
+ for (int i = 0; i < repeats; ++i) {
+ System.setProperty(JSONFormatter.SORT_JSON_FIELD_NAMES_PROPERTY, "false");
+
+ String fieldOne = "counter" + 2 * i;
+
+ long startTime = System.currentTimeMillis();
+ JSONFormatter.fromJSON(jsonString.replace(FIELD_NAME_TO_REPLACE, fieldOne));
+ totalUnsortedTime += (System.currentTimeMillis() - startTime);
+
+ System.setProperty(JSONFormatter.SORT_JSON_FIELD_NAMES_PROPERTY, "true");
+
+ String fieldTwo = "counter" + 2 * i + 1;
+
+ startTime = System.currentTimeMillis();
+ JSONFormatter.fromJSON(jsonString.replace(FIELD_NAME_TO_REPLACE, fieldTwo));
+ totalSortedTime += (System.currentTimeMillis() - startTime);
+ }
+ float consistencyFactor = (float) totalUnsortedTime / (float) totalSortedTime;
+ assertThat(consistencyFactor).withFailMessage(
+ "Expected unsorted JSON to take no longer than %f times as long as sorted JSON. Was actually %f times.",
+ TARGET_CONSISTENCY_FACTOR, consistencyFactor)
+ .isLessThan(TARGET_CONSISTENCY_FACTOR);
+
+ assertThat(1 / consistencyFactor).withFailMessage(
+ "Expected sorted JSON to take no longer than %f times as long as unsorted JSON. Was actually %f times.",
+ TARGET_CONSISTENCY_FACTOR, 1 / consistencyFactor)
+ .isLessThan(TARGET_CONSISTENCY_FACTOR);
+ });
+ }
+
+ @Test
+ public void testLocalPdxTypeMapRecoveredAfterServerRestart() {
+ final String fileName = "/org/apache/geode/pdx/jsonStrings/testJSON.txt";
+ String jsonString = loadJSONFileAsString(fileName);
+ final int addedTypes = 10;
+
+ server1.invoke(() -> {
+ InternalCache cache = ClusterStartupRule.getCache();
+ assertThat(cache).isNotNull();
+
+ // Creating a PdxType from the unmodified JSON document so that the same document can be used
+ // later to confirm that the PdxType associated with it is still registered
+ JSONFormatter.fromJSON(jsonString);
+
+ // Creating more PdxTypes to allow us to confirm that they are all recovered when we attempt
+ // to create a PdxType after restarting the server
+ for (int i = 0; i < addedTypes; ++i) {
+ String replacementField = "counter" + i;
+ String modifiedJSON = jsonString.replace(FIELD_NAME_TO_REPLACE, replacementField);
+ JSONFormatter.fromJSON(modifiedJSON);
+ }
+ PeerTypeRegistration registration =
+ (PeerTypeRegistration) (cache.getPdxRegistry().getTypeRegistration());
+
+ assertThat(registration.getLocalSize()).isEqualTo(addedTypes + 1);
+ assertThat(registration.getTypeToIdSize()).isEqualTo(addedTypes + 1);
+ });
+
+ server1.stop(false);
+ Properties props = new Properties();
+ props.setProperty("log-level", "WARN");
+ int locatorPort1 = locator.getPort();
+ server1 = cluster.startServerVM(1,
+ x -> x.withProperties(props).withConnectionToLocator(locatorPort1));
+
+ server1.invoke(() -> {
+ InternalCache cache = ClusterStartupRule.getCache();
+ assertThat(cache).isNotNull();
+
+ PeerTypeRegistration registration =
+ (PeerTypeRegistration) (cache.getPdxRegistry().getTypeRegistration());
+
+ assertThat(registration.getLocalSize()).isEqualTo(addedTypes + 1);
+ assertThat(registration.getTypeToIdSize()).isEqualTo(0);
+
+ // Attempting to create a PdxType that already exists in the /PdxTypes region in order to
+ // trigger rebuilding of the TypeToId map in PeerTypeRegistration
+ JSONFormatter.fromJSON(jsonString);
+
+ assertThat(registration.getLocalSize()).isEqualTo(addedTypes + 1);
+ assertThat(registration.getTypeToIdSize()).isEqualTo(addedTypes + 1);
+ });
+ }
+
+ @Test
+ public void testNoConflictsWhenGeneratingPdxTypesFromJSONOnMultipleServers() {
+ int repeats = 1000;
+ AsyncInvocation invocation1 = null;
+ AsyncInvocation invocation2 = null;
+
+ invocation1 = server1.invokeAsync(() -> {
+ for (int i = 0; i < repeats; ++i) {
+ JSONFormatter.fromJSON("{\"fieldName" + i + "\": \"value\"}");
+ }
+ });
+ invocation2 = server2.invokeAsync(() -> {
+ for (int i = 0; i < repeats; ++i) {
+ JSONFormatter.fromJSON("{\"fieldName" + i + "\": \"value\"}");
+ }
+ });
+
+ try {
+ invocation1.await();
+ invocation2.await();
+ } catch (Exception ex) {
+ throw new TestException("Exception while awaiting async invocation: " + ex);
+ }
+
+ server1.invoke(() -> {
+ InternalCache cache = ClusterStartupRule.getCache();
+ assertThat(cache).isNotNull();
+ int numberOfTypesInRegion = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH).size();
+ int numberOfTypesInLocalMap =
+ ((PeerTypeRegistration) cache.getPdxRegistry().getTypeRegistration()).getTypeToIdSize();
+
+ assertThat(numberOfTypesInRegion)
+ .withFailMessage("Expected number of PdxTypes in region to be %s but was %s",
+ repeats, numberOfTypesInRegion)
+ .isEqualTo(repeats);
+
+ assertThat(numberOfTypesInLocalMap)
+ .withFailMessage("Expected number of PdxTypes in local map to be %s but was %s",
+ repeats, numberOfTypesInLocalMap)
+ .isEqualTo(repeats);
+ });
+
+ server2.invoke(() -> {
+ InternalCache cache = ClusterStartupRule.getCache();
+ assertThat(cache).isNotNull();
+ int numberOfTypesInRegion = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH).size();
+ int numberOfTypesInLocalMap =
+ ((PeerTypeRegistration) cache.getPdxRegistry().getTypeRegistration()).getTypeToIdSize();
+
+ assertThat(numberOfTypesInRegion)
+ .withFailMessage("Expected number of PdxTypes in region to be %s but was %s",
+ repeats, numberOfTypesInRegion)
+ .isEqualTo(repeats);
+
+ assertThat(numberOfTypesInLocalMap)
+ .withFailMessage("Expected number of PdxTypes in local map to be %s but was %s",
+ repeats, numberOfTypesInLocalMap)
+ .isEqualTo(repeats);
+ });
+ }
+
+ @Test
+ public void testPdxTypesGeneratedFromJSONOnClientAreEnteredIntoTypeRegistry() throws Exception {
+ server1.invoke(() -> {
+ ClusterStartupRule.getCache().createRegionFactory().setDataPolicy(
+ DataPolicy.REPLICATE).create("Test");
+ });
+ server2.invoke(() -> {
+ ClusterStartupRule.getCache().createRegionFactory().setDataPolicy(
+ DataPolicy.REPLICATE).create("Test");
+ });
+ int port = locator.getPort();
+ int serverPort = server1.getPort();
+
+ Properties props = new Properties();
+ props.setProperty("log-level", "WARN");
+ ClientVM client = cluster.startClientVM(3,
+ cf -> cf.withLocatorConnection(port).withPoolSubscription(true).withProperties(props));
+
+ int entries = 10;
+ client.invoke(() -> {
+ ClientCache cache = ClusterStartupRule.getClientCache();
+ Region region =
+ cache.createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY).create("Test");
+ String field;
+ String jsonString;
+
+ for (int i = 0; i < entries; ++i) {
+ field = "\"counter" + i + "\": " + i;
+
+ jsonString = "{" + field + "}";
+
+ JSONFormatter.fromJSON(jsonString);
+ }
+ });
+
+ server1.invoke(() -> {
+ InternalCache cache = ClusterStartupRule.getCache();
+ assertThat(cache).isNotNull();
+ int numberOfTypesInRegion = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH).size();
+ int numberOfTypesInLocalMap =
+ ((PeerTypeRegistration) cache.getPdxRegistry().getTypeRegistration()).getTypeToIdSize();
+
+ assertThat(numberOfTypesInRegion)
+ .withFailMessage("Expected number of PdxTypes in region to be %s but was %s",
+ entries, numberOfTypesInRegion)
+ .isEqualTo(entries);
+
+ assertThat(numberOfTypesInLocalMap)
+ .withFailMessage("Expected number of PdxTypes in local map to be %s but was %s",
+ entries, numberOfTypesInLocalMap)
+ .isEqualTo(entries);
+ });
+ }
+
+ private String loadJSONFileAsString(String fileName) {
+ Path filePath = loadTestResourcePath(fileName);
+ String jsonString;
+ try {
+ jsonString = new String(Files.readAllBytes(filePath));
+ } catch (IOException ex) {
+ throw new TestException(ex.getMessage());
+ }
+ return jsonString;
+ }
+
+ private List<String> loadJSONFileAsList(String fileName) {
+ Path filePath = loadTestResourcePath(fileName);
+ List<String> jsonLines;
+ try {
+ jsonLines = new ArrayList<>(Files.readAllLines(filePath));
+ } catch (IOException ex) {
+ throw new TestException(ex.getMessage());
+ }
+ return jsonLines;
+ }
+
+ private static String buildJSONString(List<String> jsonLines) {
+ StringBuilder jsonString = new StringBuilder();
+ for (int i = 0; i < jsonLines.size(); ++i) {
+ String line = jsonLines.get(i);
+ jsonString.append(line + "\n");
+ }
+ return jsonString.toString();
+ }
+
+ private Path loadTestResourcePath(String fileName) {
+ Path filePath = createTempFileFromResource(getClass(), fileName).toPath();
+ assertThat(filePath).isNotNull();
+
+ return filePath;
+ }
+
+}
diff --git a/geode-core/src/distributedTest/resources/org/apache/geode/pdx/jsonStrings/testJSON.txt b/geode-core/src/distributedTest/resources/org/apache/geode/pdx/jsonStrings/testJSON.txt
new file mode 100644
index 0000000..735fa70
--- /dev/null
+++ b/geode-core/src/distributedTest/resources/org/apache/geode/pdx/jsonStrings/testJSON.txt
@@ -0,0 +1,43 @@
+{
+ "configGlossary:installationAt": "Philadelphia, PA",
+ "configGlossary:adminEmail": "ksm@pobox.com",
+ "configGlossary:poweredBy": "Cofax",
+ "configGlossary:poweredByIcon": "/images/cofax.gif",
+ "configGlossary:staticPath": "/content/static",
+ "templateProcessorClass": "org.cofax.WysiwygTemplate",
+ "templateLoaderClass": "org.cofax.FilesTemplateLoader",
+ "templatePath": "templates",
+ "templateOverridePath": "",
+ "defaultListTemplate": "listTemplate.htm",
+ "defaultFileTemplate": "articleTemplate.htm",
+ "useJSP": false,
+ "jspListTemplate": "listTemplate.jsp",
+ "jspFileTemplate": "articleTemplate.jsp",
+ "cachePackageTagsTrack": 200,
+ "cachePackageTagsStore": 200,
+ "cachePackageTagsRefresh": 60,
+ "cacheTemplatesTrack": 100,
+ "cacheTemplatesStore": 50,
+ "cacheTemplatesRefresh": 15,
+ "cachePagesTrack": 200,
+ "cachePagesStore": 100,
+ "cachePagesRefresh": 10,
+ "cachePagesDirtyRead": 10,
+ "searchEngineListTemplate": "forSearchEnginesList.htm",
+ "searchEngineFileTemplate": "forSearchEngines.htm",
+ "searchEngineRobotsDb": "WEB-INF/robots.db",
+ "useDataStore": true,
+ "dataStoreClass": "org.cofax.SqlDataStore",
+ "redirectionClass": "org.cofax.SqlRedirection",
+ "dataStoreName": "cofax",
+ "dataStoreDriver": "com.microsoft.jdbc.sqlserver.SQLServerDriver",
+ "dataStoreUrl": "jdbc:microsoft:sqlserver://LOCALHOST:1433;DatabaseName=goon",
+ "dataStoreUser": "sa",
+ "dataStorePassword": "dataStoreTestQuery",
+ "dataStoreTestQuery": "SET NOCOUNT ON;select test='test';",
+ "dataStoreLogFile": "/usr/local/tomcat/logs/datastore.log",
+ "dataStoreInitConns": 10,
+ "dataStoreMaxConns": 100,
+ "dataStoreConnUsageLimit": 100,
+ "dataStoreLogLevel": "debug",
+ "maxUrlLength": 500}
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/pdx/JSONFormatterJUnitTest.java b/geode-core/src/integrationTest/java/org/apache/geode/pdx/JSONFormatterJUnitTest.java
index 0995fb8..f8096f9 100755
--- a/geode-core/src/integrationTest/java/org/apache/geode/pdx/JSONFormatterJUnitTest.java
+++ b/geode-core/src/integrationTest/java/org/apache/geode/pdx/JSONFormatterJUnitTest.java
@@ -15,11 +15,20 @@
package org.apache.geode.pdx;
import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.apache.geode.test.util.ResourceUtils.createTempFileFromResource;
+import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import java.io.File;
+import java.io.FileNotFoundException;
import java.text.SimpleDateFormat;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
import java.util.List;
+import java.util.Scanner;
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -27,26 +36,32 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.experimental.categories.Category;
+import util.TestException;
import org.apache.geode.cache.Cache;
import org.apache.geode.cache.CacheFactory;
import org.apache.geode.cache.DataPolicy;
import org.apache.geode.cache.Region;
import org.apache.geode.pdx.internal.PdxInstanceImpl;
+import org.apache.geode.pdx.internal.PdxType;
import org.apache.geode.pdx.internal.PeerTypeRegistration;
import org.apache.geode.test.junit.categories.SerializationTest;
@Category({SerializationTest.class})
public class JSONFormatterJUnitTest {
public static final String REGION_NAME = "primitiveKVStore";
+
private Cache cache;
+
private Region<Object, Object> region;
@Before
public void setUp() throws Exception {
- this.cache = new CacheFactory().set(MCAST_PORT, "0").setPdxReadSerialized(true).create();
+ this.cache = new CacheFactory().set(MCAST_PORT, "0")
+ .set("log-level", "WARN").setPdxReadSerialized(true).create();
- region = cache.createRegionFactory().setDataPolicy(DataPolicy.PARTITION).create(REGION_NAME);
+ region = cache.createRegionFactory().setDataPolicy(DataPolicy.PARTITION)
+ .create(REGION_NAME);
}
@@ -89,7 +104,8 @@ public class JSONFormatterJUnitTest {
// 2. Get the JSON string from actualTestObject using jackson ObjectMapper.
ObjectMapper objectMapper = new ObjectMapper();
objectMapper.setDateFormat(new SimpleDateFormat("MM/dd/yyyy"));
- objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
+ objectMapper
+ .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
String json = objectMapper.writeValueAsString(expectedTestObject);
@@ -111,14 +127,16 @@ public class JSONFormatterJUnitTest {
}
@Test
- public void verifyJsonToPdxInstanceConversionWithJSONFormatter() throws Exception {
+ public void verifyJsonToPdxInstanceConversionWithJSONFormatter()
+ throws Exception {
TestObjectForJSONFormatter expectedTestObject = new TestObjectForJSONFormatter();
expectedTestObject.defaultInitialization();
// 1.gets pdxInstance using R.put() and R.get()
region.put("501", expectedTestObject);
Object receivedObject = region.get("501");
- assertEquals("receivedObject is expected to be of type PdxInstance", PdxInstanceImpl.class,
+ assertEquals("receivedObject is expected to be of type PdxInstance",
+ PdxInstanceImpl.class,
receivedObject.getClass());
PdxInstance expectedPI = (PdxInstance) receivedObject;
@@ -140,7 +158,8 @@ public class JSONFormatterJUnitTest {
assertEquals("receivedObject is expected to be of type PdxInstance",
TestObjectForJSONFormatter.class, actualTestObject.getClass());
- assertEquals("actualTestObject and expectedTestObject should be equal", expectedTestObject,
+ assertEquals("actualTestObject and expectedTestObject should be equal",
+ expectedTestObject,
actualTestObject);
}
@@ -153,7 +172,8 @@ public class JSONFormatterJUnitTest {
int pdxTypes = 0;
if (cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH) != null) {
- pdxTypes = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH).keySet().size();
+ pdxTypes = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH).keySet()
+ .size();
}
String js = "{name:\"ValueExist\", age:14}";
@@ -197,7 +217,8 @@ public class JSONFormatterJUnitTest {
int pdxTypes = 0;
if (cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH) != null) {
- pdxTypes = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH).keySet().size();
+ pdxTypes = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH)
+ .keySet().size();
}
String js2 = "{c:\"c' go\", bb:23, b:\"b\", age:14 }";
@@ -210,4 +231,214 @@ public class JSONFormatterJUnitTest {
System.setProperty(JSONFormatter.SORT_JSON_FIELD_NAMES_PROPERTY, "false");
}
}
+
+ @Test
+ public void testReorderJSONFieldsDoesNotCauseManyTypeIdConflicts() {
+ final int entries = 10000;
+ final float toleratedCollisonFraction = 0.01f;
+
+ List<String> jsonLines = loadJSONDocument();
+
+ for (int i = 0; i < entries; ++i) {
+ randomizeFieldsAndCreatePdx(jsonLines);
+ }
+
+ int collisons = calculateNumberOfCollisions();
+
+ float collisonRate = (float) collisons / (float) entries;
+
+ assertThat(collisonRate)
+ .withFailMessage("PdxTypeId collision rate too high. Expected %f but was %f.",
+ toleratedCollisonFraction, collisonRate)
+ .isLessThan(toleratedCollisonFraction);
+ }
+
+ @Test
+ public void testCounterInJSONFieldNameDoesNotCauseManyTypeIdConflicts() {
+ final int entries = 10000;
+ final float toleratedCollisonFraction = 0.01f;
+
+ List<String> jsonLines = loadJSONDocument();
+
+ String jsonString = "";
+ String counterField = "";
+ String fieldToReplace = jsonLines.get(1);
+ for (int i = 0; i < entries; ++i) {
+ jsonString = buildJSONString(jsonLines);
+ counterField = "\"counter" + i + "\": " + i + ",";
+ JSONFormatter.fromJSON(jsonString.replace(fieldToReplace, counterField));
+ }
+
+ int collisons = calculateNumberOfCollisions();
+
+ float collisonRate = (float) collisons / (float) entries;
+
+ assertThat(collisonRate)
+ .withFailMessage("PdxTypeId collision rate too high. Expected %f but was %f.",
+ toleratedCollisonFraction, collisonRate)
+ .isLessThan(toleratedCollisonFraction);
+ }
+
+ @Test
+ public void testSortingJSONFieldsPreventsTypeIDCollision() {
+ String initialState =
+ String.valueOf(
+ Boolean.getBoolean(JSONFormatter.SORT_JSON_FIELD_NAMES_PROPERTY));
+ try {
+ final int unsortedEntries = 100;
+ final int sortedEntries = 10000;
+ final int jsonFields = 40;
+
+ List<String> jsonLines = generateJSONWithCollidingFieldNames(jsonFields);
+
+ System.setProperty(JSONFormatter.SORT_JSON_FIELD_NAMES_PROPERTY, "false");
+
+ // Show that not sorting leads to 100% collision rate. There are 1 fewer collisions than there
+ // are entries because the first entry cannot have a TypeId collision
+ for (int i = 0; i < unsortedEntries; ++i) {
+ randomizeFieldsAndCreatePdx(jsonLines);
+ }
+ int unsortedCollisions = calculateNumberOfCollisions();
+ assertThat(unsortedCollisions).withFailMessage(
+ "Unexpected number of PdxTypeId collisions. Expected %d but was actually %d.",
+ unsortedEntries - 1, unsortedCollisions).isEqualTo(unsortedEntries - 1);
+
+ System.setProperty(JSONFormatter.SORT_JSON_FIELD_NAMES_PROPERTY, "true");
+
+ // Show that sorting leads to no additional collisions beyond the single probable collision
+ // caused by the first sorted entry
+ for (int i = 0; i < sortedEntries; ++i) {
+ randomizeFieldsAndCreatePdx(jsonLines);
+ }
+ int sortedCollisions = calculateNumberOfCollisions();
+ assertThat(sortedCollisions)
+ .withFailMessage("Too many PdxTypeId collisions. Expected %d but was %d.",
+ unsortedCollisions + 1, sortedCollisions)
+ .isLessThanOrEqualTo(unsortedCollisions + 1);
+ } finally {
+ System.setProperty(JSONFormatter.SORT_JSON_FIELD_NAMES_PROPERTY,
+ initialState);
+ }
+ }
+
+ public List<String> loadJSONDocument() {
+ File source = loadTestResource(
+ "/org/apache/geode/internal/pdx/jsonStrings/testJSON.txt");
+ assertThat(source.exists());
+ return getJSONLines(source);
+ }
+
+ private File loadTestResource(String fileName) {
+ String filePath = createTempFileFromResource(getClass(), fileName)
+ .getAbsolutePath();
+ assertThat(filePath).isNotNull();
+
+ return new File(filePath);
+ }
+
+ private List<String> getJSONLines(File source) {
+ try {
+ Scanner scanner = new Scanner(source);
+ List<String> jsonLines = new ArrayList<>();
+ while (scanner.hasNext()) {
+ jsonLines.add(scanner.nextLine());
+ }
+ return jsonLines;
+ } catch (FileNotFoundException e) {
+ throw new TestException(e.getMessage());
+ }
+ }
+
+ private String buildJSONString(List<String> jsonLines) {
+ StringBuilder jsonString = new StringBuilder();
+ for (int i = 0; i < jsonLines.size(); ++i) {
+ String line = jsonLines.get(i);
+ jsonString.append(line + "\n");
+ }
+ return jsonString.toString();
+ }
+
+ private void randomizeFieldsAndCreatePdx(List<String> jsonLines) {
+ String jsonString = "";
+ // First and last lines of the JSON document cannot be reordered due to containing "{" and "}"
+ Collections.shuffle(jsonLines.subList(1, jsonLines.size() - 1));
+ jsonString = buildJSONString(jsonLines);
+ JSONFormatter.fromJSON(jsonString);
+ }
+
+ private int calculateNumberOfCollisions() {
+ Collection pdxTypes = null;
+
+ if (cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH) != null) {
+ pdxTypes = cache.getRegion(PeerTypeRegistration.REGION_FULL_PATH)
+ .values();
+ }
+ assertThat(pdxTypes).isNotNull();
+
+ int collisions = 0;
+ for (Object object : pdxTypes) {
+ PdxType pdxType = (PdxType) object;
+
+ // Ideally this would be a method call to PeerTypeRegistration to avoid code duplication
+ int calculatedId =
+ pdxType.hashCode() & PeerTypeRegistration.PLACE_HOLDER_FOR_TYPE_ID;
+
+ if (pdxType.getTypeId() != calculatedId) {
+ collisions++;
+ }
+ }
+ return collisions;
+ }
+
+ private List<String> generateJSONWithCollidingFieldNames(int jsonFields) {
+ List<String> jsonLines = generateCollidingStrings(jsonFields);
+ assertThat(jsonLines).isNotNull();
+
+ // Format generated strings into JSON document
+ for (int i = 0; i < jsonLines.size(); ++i) {
+ String field = "\"" + jsonLines.get(i) + "\": " + i;
+ // Do not add a comma to the last field, but do add a closing }
+ if (i != jsonLines.size() - 1) {
+ field += ",";
+ } else {
+ field += "}";
+ }
+ jsonLines.set(i, field);
+ }
+ jsonLines.add(0, "{");
+ return jsonLines;
+ }
+
+ /**
+ * This method produces Strings with identical hashcode() values
+ */
+ private List<String> generateCollidingStrings(int numberOfStrings) {
+ final String[] baseStrings = {"Aa", "BB", "C#"};
+
+ int firstHash = baseStrings[0].hashCode();
+ for (String element : baseStrings) {
+ assertThat(element.hashCode() == firstHash);
+ }
+
+ // By definition, it requires at least two strings for there to be a collision
+ if (numberOfStrings < 2) {
+ return new ArrayList<>();
+ }
+
+ int elementsPerString =
+ (int) Math.ceil(Math.log(numberOfStrings) / Math.log(baseStrings.length));
+
+ List<String> result = Arrays.asList("", "", "");
+
+ for (int i = 0; i < elementsPerString; ++i) {
+ List<String> partialResult = new ArrayList();
+ for (String string : result) {
+ for (String element : baseStrings) {
+ partialResult.add(string + element);
+ }
+ }
+ result = partialResult;
+ }
+ return result.subList(0, numberOfStrings);
+ }
}
diff --git a/geode-core/src/integrationTest/java/org/apache/geode/pdx/internal/PeerTypeRegistrationIntegrationTest.java b/geode-core/src/integrationTest/java/org/apache/geode/pdx/internal/PeerTypeRegistrationIntegrationTest.java
new file mode 100644
index 0000000..680e723
--- /dev/null
+++ b/geode-core/src/integrationTest/java/org/apache/geode/pdx/internal/PeerTypeRegistrationIntegrationTest.java
@@ -0,0 +1,92 @@
+/*
+ * 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.pdx.internal;
+
+import static org.apache.geode.distributed.ConfigurationProperties.MCAST_PORT;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.test.junit.categories.SerializationTest;
+
+@Category({SerializationTest.class})
+public class PeerTypeRegistrationIntegrationTest {
+ private Cache cache;
+ private PeerTypeRegistration registration;
+
+ @Before
+ public void setUp() {
+ cache = new CacheFactory().set(MCAST_PORT, "0")
+ .set("log-level", "WARN").setPdxReadSerialized(true).create();
+ registration = new PeerTypeRegistration((InternalCache) cache);
+ registration.initialize();
+ }
+
+ @After
+ public void tearDown() {
+ try {
+ if (!cache.isClosed()) {
+ cache.close();
+ }
+ } catch (Exception e) {
+ throw new AssertionError(e);
+ }
+ }
+
+ @Test
+ public void testDefineType() {
+
+ PdxType firstType = new PdxType();
+ firstType.setClassName("Mock.Test.Class.One");
+
+ PdxType secondType = new PdxType();
+ secondType.setClassName("Mock.Test.Class.Two");
+
+ assertThat(registration.getLocalSize()).isEqualTo(0);
+ assertThat(registration.getTypeToIdSize()).isEqualTo(0);
+
+ int firstTypeId1 = registration.defineType(firstType);
+
+ // Confirm the PdxType was added to the region and the local map
+ assertThat(registration.getLocalSize()).isEqualTo(1);
+ assertThat(registration.getTypeToIdSize()).isEqualTo(1);
+
+ firstType.setTypeId(firstTypeId1 - 1);
+ int firstTypeId2 = registration.defineType(firstType);
+
+ // Defining an existing type with a different TypeId returns the existing TypeId
+ assertThat(firstTypeId1).isEqualTo(firstTypeId2);
+ assertThat(registration.getType(firstTypeId2)).isEqualTo(firstType);
+
+ // Defining an existing type does not add a new type to the region or local map
+ assertThat(registration.getLocalSize()).isEqualTo(1);
+ assertThat(registration.getTypeToIdSize()).isEqualTo(1);
+
+ secondType.setTypeId(firstTypeId1);
+
+ int secondTypeId = registration.defineType(secondType);
+
+ // Defining a new type with an existing TypeId does not overwrite the existing type
+ assertThat(secondTypeId).isNotEqualTo(firstTypeId1);
+ assertThat(registration.getLocalSize()).isEqualTo(2);
+ assertThat(registration.getTypeToIdSize()).isEqualTo(2);
+ }
+}
diff --git a/geode-core/src/integrationTest/resources/org/apache/geode/internal/pdx/jsonStrings/testJSON.txt b/geode-core/src/integrationTest/resources/org/apache/geode/internal/pdx/jsonStrings/testJSON.txt
new file mode 100644
index 0000000..735fa70
--- /dev/null
+++ b/geode-core/src/integrationTest/resources/org/apache/geode/internal/pdx/jsonStrings/testJSON.txt
@@ -0,0 +1,43 @@
+{
+ "configGlossary:installationAt": "Philadelphia, PA",
+ "configGlossary:adminEmail": "ksm@pobox.com",
+ "configGlossary:poweredBy": "Cofax",
+ "configGlossary:poweredByIcon": "/images/cofax.gif",
+ "configGlossary:staticPath": "/content/static",
+ "templateProcessorClass": "org.cofax.WysiwygTemplate",
+ "templateLoaderClass": "org.cofax.FilesTemplateLoader",
+ "templatePath": "templates",
+ "templateOverridePath": "",
+ "defaultListTemplate": "listTemplate.htm",
+ "defaultFileTemplate": "articleTemplate.htm",
+ "useJSP": false,
+ "jspListTemplate": "listTemplate.jsp",
+ "jspFileTemplate": "articleTemplate.jsp",
+ "cachePackageTagsTrack": 200,
+ "cachePackageTagsStore": 200,
+ "cachePackageTagsRefresh": 60,
+ "cacheTemplatesTrack": 100,
+ "cacheTemplatesStore": 50,
+ "cacheTemplatesRefresh": 15,
+ "cachePagesTrack": 200,
+ "cachePagesStore": 100,
+ "cachePagesRefresh": 10,
+ "cachePagesDirtyRead": 10,
+ "searchEngineListTemplate": "forSearchEnginesList.htm",
+ "searchEngineFileTemplate": "forSearchEngines.htm",
+ "searchEngineRobotsDb": "WEB-INF/robots.db",
+ "useDataStore": true,
+ "dataStoreClass": "org.cofax.SqlDataStore",
+ "redirectionClass": "org.cofax.SqlRedirection",
+ "dataStoreName": "cofax",
+ "dataStoreDriver": "com.microsoft.jdbc.sqlserver.SQLServerDriver",
+ "dataStoreUrl": "jdbc:microsoft:sqlserver://LOCALHOST:1433;DatabaseName=goon",
+ "dataStoreUser": "sa",
+ "dataStorePassword": "dataStoreTestQuery",
+ "dataStoreTestQuery": "SET NOCOUNT ON;select test='test';",
+ "dataStoreLogFile": "/usr/local/tomcat/logs/datastore.log",
+ "dataStoreInitConns": 10,
+ "dataStoreMaxConns": 100,
+ "dataStoreConnUsageLimit": 100,
+ "dataStoreLogLevel": "debug",
+ "maxUrlLength": 500}
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 0aae7bf..f0c7d5d 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
@@ -26,6 +26,7 @@ import org.apache.logging.log4j.Logger;
import org.apache.geode.InternalGemFireError;
import org.apache.geode.InternalGemFireException;
+import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.cache.AttributesFactory;
import org.apache.geode.cache.CacheWriterException;
import org.apache.geode.cache.DataPolicy;
@@ -158,7 +159,7 @@ public class PeerTypeRegistration implements TypeRegistration {
// update a local map with the pdxtypes registered
Object value = event.getNewValue();
if (value instanceof PdxType) {
- updateClassToTypeMap((PdxType) value);
+ updateLocalMaps((PdxType) value);
}
}
});
@@ -350,23 +351,25 @@ public class PeerTypeRegistration implements TypeRegistration {
public int defineType(PdxType newType) {
verifyConfiguration();
Integer existingId = typeToId.get(newType);
+
if (existingId != null) {
return existingId;
}
lock();
try {
- int id = getExistingIdForType(newType);
- if (id != -1) {
- return id;
+ if (typeToId.isEmpty()) {
+ buildTypeToIdFromIdToType();
+ }
+ // double check if my type is in region in case the typeToId map has been updated while
+ // waiting to obtain a lock
+ existingId = typeToId.get(newType);
+ if (existingId != null) {
+ return existingId;
}
- id = allocateTypeId(newType);
+ int id = allocateTypeId(newType);
newType.setTypeId(id);
-
updateIdToTypeRegion(newType);
-
- typeToId.put(newType, id);
-
return newType.getTypeId();
} finally {
unlock();
@@ -538,12 +541,10 @@ public class PeerTypeRegistration implements TypeRegistration {
}
}
- /** Should be called holding the dlock */
- private int getExistingIdForType(PdxType newType) {
+ private void buildTypeToIdFromIdToType() {
int totalPdxTypeIdInDS = 0;
TXStateProxy currentState = suspendTX();
try {
- int result = -1;
for (Map.Entry<Object, Object> entry : getIdToType().entrySet()) {
Object v = entry.getValue();
Object k = entry.getKey();
@@ -557,20 +558,16 @@ public class PeerTypeRegistration implements TypeRegistration {
int tmpDsId = PLACE_HOLDER_FOR_DS_ID & id;
if (tmpDsId == this.dsId) {
totalPdxTypeIdInDS++;
+ if (totalPdxTypeIdInDS >= this.maxTypeId) {
+ throw new InternalGemFireError(
+ "Used up all of the PDX type ids for this distributed system. The maximum number of PDX types is "
+ + this.maxTypeId);
+ }
}
typeToId.put(foundType, id);
- if (foundType.equals(newType)) {
- result = foundType.getTypeId();
- }
}
}
- if (totalPdxTypeIdInDS == this.maxTypeId) {
- throw new InternalGemFireError(
- "Used up all of the PDX type ids for this distributed system. The maximum number of PDX types is "
- + this.maxTypeId);
- }
- return result;
} finally {
resumeTX(currentState);
}
@@ -737,8 +734,9 @@ public class PeerTypeRegistration implements TypeRegistration {
/**
* adds a PdxType for a field to a {@code className => Set<PdxType>} map
*/
- private void updateClassToTypeMap(PdxType type) {
+ private void updateLocalMaps(PdxType type) {
if (type != null) {
+ typeToId.put(type, type.getTypeId());
synchronized (this.classToType) {
if (type.getClassName().equals(JSONFormatter.JSON_CLASSNAME)) {
return; // no need to include here
@@ -807,4 +805,9 @@ public class PeerTypeRegistration implements TypeRegistration {
public int getLocalSize() {
return idToType.size();
}
+
+ @VisibleForTesting
+ public int getTypeToIdSize() {
+ return typeToId.size();
+ }
}