You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2021/03/31 06:50:08 UTC
[ignite] branch master updated: IGNITE-14347: Fix Node Failure on
Receiving Data of Unknown Class via Distributed Metastorage (#8898)
This is an automated email from the ASF dual-hosted git repository.
ibessonov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/master by this push:
new 288c290 IGNITE-14347: Fix Node Failure on Receiving Data of Unknown Class via Distributed Metastorage (#8898)
288c290 is described below
commit 288c290f8636d76e92d4c453ba89ffff4ccbfe60
Author: Atri Sharma <at...@gmail.com>
AuthorDate: Wed Mar 31 12:17:59 2021 +0530
IGNITE-14347: Fix Node Failure on Receiving Data of Unknown Class via Distributed Metastorage (#8898)
Co-authored-by: Semyon Danilov <sa...@yandex.ru>
---
.../persistence/DistributedMetaStorageImpl.java | 57 ++++--
.../apache/ignite/internal/util/IgniteUtils.java | 2 +-
.../org/apache/ignite/lang/IgniteProducer.java | 34 ++++
.../src/test/config/class_list_test_excluded.txt | 17 ++
.../DistributedMetaStorageClassloadingTest.java | 205 +++++++++++++++++++++
.../ignite/testsuites/IgniteBasicTestSuite.java | 2 +
6 files changed, 300 insertions(+), 17 deletions(-)
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageImpl.java b/modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageImpl.java
index 880f645..90f320c 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageImpl.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/processors/metastorage/persistence/DistributedMetaStorageImpl.java
@@ -66,6 +66,7 @@ import org.apache.ignite.internal.util.typedef.internal.S;
import org.apache.ignite.internal.util.typedef.internal.U;
import org.apache.ignite.lang.IgniteBiTuple;
import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteProducer;
import org.apache.ignite.marshaller.jdk.JdkMarshaller;
import org.apache.ignite.spi.IgniteNodeValidationResult;
import org.apache.ignite.spi.IgniteSpiException;
@@ -1240,8 +1241,15 @@ public class DistributedMetaStorageImpl extends GridProcessorAdapter
ver = ver.nextVersion(histItem);
- for (int i = 0, len = histItem.keys().length; i < len; i++)
- notifyListeners(histItem.keys()[i], bridge.read(histItem.keys()[i]), unmarshal(marshaller, histItem.valuesBytesArray()[i]));
+ for (int i = 0, len = histItem.keys().length; i < len; i++) {
+ String key = histItem.keys()[i];
+ byte[] valBytes = histItem.valuesBytesArray()[i];
+
+ notifyListeners(
+ histItem.keys()[i],
+ () -> bridge.read(key),
+ () -> unmarshal(marshaller, valBytes));
+ }
for (int i = 0, len = histItem.keys().length; i < len; i++)
bridge.write(histItem.keys()[i], histItem.valuesBytesArray()[i]);
@@ -1399,21 +1407,20 @@ public class DistributedMetaStorageImpl extends GridProcessorAdapter
int c = oldKey.compareTo(newKey);
if (c < 0) {
- notifyListeners(oldKey, unmarshal(marshaller, oldValBytes), null);
+ notifyListeners(oldKey, () -> unmarshal(marshaller, oldValBytes), () -> null);
++oldIdx;
}
else if (c > 0) {
- notifyListeners(newKey, null, unmarshal(marshaller, newValBytes));
+ notifyListeners(newKey, () -> null, () -> unmarshal(marshaller, newValBytes));
++newIdx;
}
else {
- Serializable oldVal = unmarshal(marshaller, oldValBytes);
-
- Serializable newVal = Arrays.equals(oldValBytes, newValBytes) ? oldVal : unmarshal(marshaller, newValBytes);
-
- notifyListeners(oldKey, oldVal, newVal);
+ notifyListeners(
+ oldKey,
+ () -> unmarshal(marshaller, oldValBytes),
+ () -> unmarshal(marshaller, newValBytes));
++oldIdx;
@@ -1421,23 +1428,41 @@ public class DistributedMetaStorageImpl extends GridProcessorAdapter
}
}
- for (; oldIdx < oldData.length; ++oldIdx)
- notifyListeners(oldData[oldIdx].key, unmarshal(marshaller, oldData[oldIdx].valBytes), null);
+ for (; oldIdx < oldData.length; ++oldIdx) {
+ byte[] oldValBytes = oldData[oldIdx].valBytes;
+ notifyListeners(oldData[oldIdx].key, () -> unmarshal(marshaller, oldValBytes), () -> null);
+ }
- for (; newIdx < newData.length; ++newIdx)
- notifyListeners(newData[newIdx].key, null, unmarshal(marshaller, newData[newIdx].valBytes));
+ for (; newIdx < newData.length; ++newIdx) {
+ byte[] newValBytes = newData[newIdx].valBytes;
+ notifyListeners(newData[newIdx].key, () -> null, () -> unmarshal(marshaller, newValBytes));
+ }
}
/**
* Notify listeners.
*
* @param key The key.
- * @param oldVal Old value.
- * @param newVal New value.
+ * @param oldValProducer Lazy getter for an old value and is executed only if there are listeners for the given key.
+ * @param newValProducer Lazy getter for a new value and is executed only if there are listeners for the given key.
*/
- private void notifyListeners(String key, Serializable oldVal, Serializable newVal) {
+ private void notifyListeners(String key,
+ @NotNull IgniteProducer<Serializable> oldValProducer,
+ @NotNull IgniteProducer<Serializable> newValProducer) throws IgniteCheckedException {
+ boolean valuesProduced = false;
+
+ Serializable newVal = null;
+
+ Serializable oldVal = null;
+
for (IgniteBiTuple<Predicate<String>, DistributedMetaStorageListener<Serializable>> entry : lsnrs) {
if (entry.get1().test(key)) {
+ if (!valuesProduced) {
+ newVal = newValProducer.produce();
+ oldVal = oldValProducer.produce();
+ valuesProduced = true;
+ }
+
try {
// ClassCastException might be thrown here for crappy listeners.
entry.get2().onUpdate(key, oldVal, newVal);
diff --git a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
index 61dba92..401b3ec 100755
--- a/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/util/IgniteUtils.java
@@ -9031,7 +9031,7 @@ public abstract class IgniteUtils {
if (cls == null) {
if (clsFilter != null && !clsFilter.apply(clsName))
- throw new RuntimeException("Deserialization of class " + clsName + " is disallowed.");
+ throw new ClassNotFoundException("Deserialization of class " + clsName + " is disallowed.");
// Avoid class caching inside Class.forName
if (ldr instanceof CacheClassLoaderMarker)
diff --git a/modules/core/src/main/java/org/apache/ignite/lang/IgniteProducer.java b/modules/core/src/main/java/org/apache/ignite/lang/IgniteProducer.java
new file mode 100644
index 0000000..d74517f
--- /dev/null
+++ b/modules/core/src/main/java/org/apache/ignite/lang/IgniteProducer.java
@@ -0,0 +1,34 @@
+/*
+ * 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.ignite.lang;
+
+import org.apache.ignite.IgniteCheckedException;
+
+/**
+ * Defines a producer which can throw IgniteCheckedException.
+ *
+ * @param <T> Type of producible value.
+ */
+@FunctionalInterface
+public interface IgniteProducer<T> {
+ /**
+ * Produce value.
+ */
+ public T produce() throws IgniteCheckedException;
+
+}
diff --git a/modules/core/src/test/config/class_list_test_excluded.txt b/modules/core/src/test/config/class_list_test_excluded.txt
new file mode 100644
index 0000000..787ed89
--- /dev/null
+++ b/modules/core/src/test/config/class_list_test_excluded.txt
@@ -0,0 +1,17 @@
+#
+# 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.
+#
+org.apache.ignite.internal.processors.metastorage.DistributedMetaStorageClassloadingTest$BamboozleClass
\ No newline at end of file
diff --git a/modules/core/src/test/java/org/apache/ignite/internal/processors/metastorage/DistributedMetaStorageClassloadingTest.java b/modules/core/src/test/java/org/apache/ignite/internal/processors/metastorage/DistributedMetaStorageClassloadingTest.java
new file mode 100644
index 0000000..21a308f
--- /dev/null
+++ b/modules/core/src/test/java/org/apache/ignite/internal/processors/metastorage/DistributedMetaStorageClassloadingTest.java
@@ -0,0 +1,205 @@
+/*
+ * 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.ignite.internal.processors.metastorage;
+
+import java.io.Serializable;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.failure.FailureContext;
+import org.apache.ignite.failure.FailureHandler;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.processors.metastorage.persistence.DistributedMetaStorageImpl;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_MARSHALLER_BLACKLIST;
+
+/**
+ * Test for {@link DistributedMetaStorageImpl} issues with classloading.
+ */
+public class DistributedMetaStorageClassloadingTest extends GridCommonAbstractTest {
+ /** Failure handler that keeps count of failures (initialized before every test). */
+ private CountingFailureHandler failureHandler;
+
+ /** {@inheritDoc} */
+ @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+ IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+ cfg.setConsistentId(igniteInstanceName);
+
+ return cfg;
+ }
+
+ /** {@inheritDoc} */
+ @Override protected FailureHandler getFailureHandler(String igniteInstanceName) {
+ return failureHandler;
+ }
+
+ /** */
+ @Before
+ public void before() throws Exception {
+ stopAllGrids();
+ failureHandler = new CountingFailureHandler();
+ }
+
+ /**
+ * Test that receiving data of unknown class into distributed metastorage doesn't lead to failure.
+ *
+ * Description:
+ * Start server node with exclusion of certain BamboozleClass (this is done via system property
+ * which adds class filter to class loader).
+ * Start client node and write new instance of BamboozleClass to the distributed metastorage to test that
+ * new value is not marshalled.
+ * Write another instance of BamboozleClass (with different value of fields) to test that
+ * old value is not unmarshalled.
+ * There must be no failures and all 2 grids must be alive.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testWontFailReceivingDataOfUnknownClass() throws Exception {
+ String path = U.resolveIgnitePath("modules/core/src/test/config/class_list_test_excluded.txt").getPath();
+
+ System.setProperty(IGNITE_MARSHALLER_BLACKLIST, path);
+ startGrid(1);
+ System.clearProperty(IGNITE_MARSHALLER_BLACKLIST);
+
+ IgniteEx client = startClientGrid(0);
+
+ client.context().distributedMetastorage().write("hey", new BamboozleClass(0));
+ client.context().distributedMetastorage().write("hey", new BamboozleClass(1));
+
+ assertEquals(0, failureHandler.getCount());
+ }
+
+ /**
+ * Test that reading data of unknown class from distributed metastorage doesn't lead to failure.
+ *
+ * Description:
+ * Start server node with exclusion of certain BamboozleClass (this is done via system property
+ * which adds class filter to class loader).
+ * Start client node and write new instance of BamboozleClass to the distributed metastorage.
+ * Try reading data of BamboozleClass
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testWontFailReadingDataOfUnknownClass() throws Exception {
+ String path = U.resolveIgnitePath("modules/core/src/test/config/class_list_test_excluded.txt").getPath();
+
+ System.setProperty(IGNITE_MARSHALLER_BLACKLIST, path);
+ IgniteEx ignite = startGrid(1);
+ System.clearProperty(IGNITE_MARSHALLER_BLACKLIST);
+
+ IgniteEx client = startClientGrid(0);
+
+ client.context().distributedMetastorage().write("hey", new BamboozleClass(0));
+
+ try {
+ Serializable hey = ignite.context().distributedMetastorage().read("hey");
+ }
+ catch (Exception ignored) { }
+
+ assertEquals(0, failureHandler.getCount());
+ }
+
+
+ /**
+ * Test that listening for data of unknown class from distributed metastorage lead to failure.
+ *
+ * Description:
+ * Start server node with exclusion of certain BamboozleClass (this is done via system property
+ * which adds class filter to class loader).
+ * Add listener to a server's distributed metadata.
+ * Start client node and write new instance of BamboozleClass to the distributed metastorage.
+ * This should lead to a failure on a server node.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testFailListeningForDataOfUnknownClass() throws Exception {
+ String path = U.resolveIgnitePath("modules/core/src/test/config/class_list_test_excluded.txt").getPath();
+
+ System.setProperty(IGNITE_MARSHALLER_BLACKLIST, path);
+ IgniteEx ignite = startGrid(1);
+ System.clearProperty(IGNITE_MARSHALLER_BLACKLIST);
+
+ IgniteEx client = startClientGrid(0);
+
+ ignite.context().distributedMetastorage().listen("hey"::equals, (key, oldVal, newVal) -> {
+ System.out.println(newVal);
+ });
+
+ try {
+ client.context().distributedMetastorage().write("hey", new BamboozleClass(0));
+ }
+ catch (Exception ignored) {}
+
+ assertEquals(1, failureHandler.getCount());
+ }
+
+ /** */
+ @After
+ public void after() throws Exception {
+ stopAllGrids();
+ }
+
+ /**
+ * Class that would be excluded on the certain npde.
+ */
+ public static final class BamboozleClass implements Serializable {
+
+ private final int i;
+
+ public BamboozleClass(int i) {
+ this.i = i;
+ }
+
+ public int getI() {
+ return i;
+ }
+ }
+
+ /**
+ * Failure handler that only keeps count of failures.
+ */
+ private static class CountingFailureHandler implements FailureHandler {
+
+ /**
+ * Count of failures.
+ * */
+ private int count = 0;
+
+ /** {@inheritDoc} */
+ @Override public boolean onFailure(Ignite ignite, FailureContext failureCtx) {
+ count++;
+ return false;
+ }
+
+ /**
+ * Get count of failures.
+ */
+ public int getCount() {
+ return count;
+ }
+ }
+
+}
diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java
index 6d8e9e3..0bb64f6 100644
--- a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java
+++ b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteBasicTestSuite.java
@@ -103,6 +103,7 @@ import org.apache.ignite.internal.processors.database.IndexStorageSelfTest;
import org.apache.ignite.internal.processors.database.SwapPathConstructionSelfTest;
import org.apache.ignite.internal.processors.failure.FailureProcessorLoggingTest;
import org.apache.ignite.internal.processors.failure.FailureProcessorThreadDumpThrottlingTest;
+import org.apache.ignite.internal.processors.metastorage.DistributedMetaStorageClassloadingTest;
import org.apache.ignite.internal.processors.metastorage.DistributedMetaStorageTest;
import org.apache.ignite.internal.processors.metastorage.persistence.DistributedMetaStorageHistoryCacheTest;
import org.apache.ignite.internal.processors.metastorage.persistence.DmsDataWriterWorkerTest;
@@ -280,6 +281,7 @@ import org.junit.runners.Suite;
// In-memory Distributed MetaStorage.
DistributedMetaStorageTest.class,
DistributedMetaStorageHistoryCacheTest.class,
+ DistributedMetaStorageClassloadingTest.class,
DmsDataWriterWorkerTest.class,
InMemoryCachedDistributedMetaStorageBridgeTest.class,
DistributedConfigurationInMemoryTest.class,