You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by mc...@apache.org on 2019/08/06 21:18:22 UTC
[geode] branch release/1.10.0 updated: Revert "GEODE-6973: Use cachelistener to synchronize typeToId with IdToType r… (#3853)"
This is an automated email from the ASF dual-hosted git repository.
mcmellawatt pushed a commit to branch release/1.10.0
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/release/1.10.0 by this push:
new 1067733 Revert "GEODE-6973: Use cachelistener to synchronize typeToId with IdToType r… (#3853)"
1067733 is described below
commit 10677330666d6bbc45ac535268db026d1351872e
Author: Naburun Nag <na...@cs.wisc.edu>
AuthorDate: Tue Aug 6 14:17:48 2019 -0700
Revert "GEODE-6973: Use cachelistener to synchronize typeToId with IdToType r… (#3853)"
This reverts commit 181e3a4a465aa9f5e06f39cd1285c94f9bc78600.
---
.../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, 30 insertions(+), 762 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
deleted file mode 100644
index d0fc78a..0000000
--- a/geode-core/src/distributedTest/java/org/apache/geode/pdx/PdxTypeGenerationDUnitTest.java
+++ /dev/null
@@ -1,320 +0,0 @@
-/*
- * 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
deleted file mode 100644
index 735fa70..0000000
--- a/geode-core/src/distributedTest/resources/org/apache/geode/pdx/jsonStrings/testJSON.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-{
- "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 f8096f9..0995fb8 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,20 +15,11 @@
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;
@@ -36,32 +27,26 @@ 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")
- .set("log-level", "WARN").setPdxReadSerialized(true).create();
+ this.cache = new CacheFactory().set(MCAST_PORT, "0").setPdxReadSerialized(true).create();
- region = cache.createRegionFactory().setDataPolicy(DataPolicy.PARTITION)
- .create(REGION_NAME);
+ region = cache.createRegionFactory().setDataPolicy(DataPolicy.PARTITION).create(REGION_NAME);
}
@@ -104,8 +89,7 @@ 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);
@@ -127,16 +111,14 @@ 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;
@@ -158,8 +140,7 @@ 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);
}
@@ -172,8 +153,7 @@ 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}";
@@ -217,8 +197,7 @@ 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 }";
@@ -231,214 +210,4 @@ 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
deleted file mode 100644
index 680e723..0000000
--- a/geode-core/src/integrationTest/java/org/apache/geode/pdx/internal/PeerTypeRegistrationIntegrationTest.java
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * 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
deleted file mode 100644
index 735fa70..0000000
--- a/geode-core/src/integrationTest/resources/org/apache/geode/internal/pdx/jsonStrings/testJSON.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-{
- "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 f0c7d5d..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
@@ -26,7 +26,6 @@ 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;
@@ -159,7 +158,7 @@ public class PeerTypeRegistration implements TypeRegistration {
// update a local map with the pdxtypes registered
Object value = event.getNewValue();
if (value instanceof PdxType) {
- updateLocalMaps((PdxType) value);
+ updateClassToTypeMap((PdxType) value);
}
}
});
@@ -351,25 +350,23 @@ public class PeerTypeRegistration implements TypeRegistration {
public int defineType(PdxType newType) {
verifyConfiguration();
Integer existingId = typeToId.get(newType);
-
if (existingId != null) {
return existingId;
}
lock();
try {
- 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;
+ int id = getExistingIdForType(newType);
+ if (id != -1) {
+ return id;
}
- int id = allocateTypeId(newType);
+ id = allocateTypeId(newType);
newType.setTypeId(id);
+
updateIdToTypeRegion(newType);
+
+ typeToId.put(newType, id);
+
return newType.getTypeId();
} finally {
unlock();
@@ -541,10 +538,12 @@ public class PeerTypeRegistration implements TypeRegistration {
}
}
- private void buildTypeToIdFromIdToType() {
+ /** Should be called holding the dlock */
+ private int getExistingIdForType(PdxType newType) {
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();
@@ -558,16 +557,20 @@ 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);
}
@@ -734,9 +737,8 @@ public class PeerTypeRegistration implements TypeRegistration {
/**
* adds a PdxType for a field to a {@code className => Set<PdxType>} map
*/
- private void updateLocalMaps(PdxType type) {
+ private void updateClassToTypeMap(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
@@ -805,9 +807,4 @@ public class PeerTypeRegistration implements TypeRegistration {
public int getLocalSize() {
return idToType.size();
}
-
- @VisibleForTesting
- public int getTypeToIdSize() {
- return typeToId.size();
- }
}