diff --git a/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java b/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java index c9ad35a208197..92e5c84d9c299 100644 --- a/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java +++ b/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java @@ -1396,8 +1396,20 @@ public void updateMetaIfNeeded( registerUserClassName(typeId, typeName, writer.failIfUnregistered(), false, JAVA_ID); - updateMetadata(typeId, new BinaryMetadata(typeId, typeName, fieldsMeta, affFieldName, - Collections.singleton(curSchema), false, null), writer.failIfUnregistered()); + BinaryMetadata newMeta = new BinaryMetadata( + typeId, + typeName, + fieldsMeta, + affFieldName, + Collections.singleton(curSchema), + false, + null + ); + + if (meta != null) + newMeta = BinaryUtils.mergeMetadata(((BinaryTypeImpl)meta).metadata(), newMeta); + + updateMetadata(typeId, newMeta, writer.failIfUnregistered()); schemaReg.addSchema(curSchema.schemaId(), curSchema); } diff --git a/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java b/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java index 557a4f314c330..d1321dd8fb0b1 100644 --- a/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java +++ b/modules/binary/api/src/main/java/org/apache/ignite/internal/binary/BinaryMetadataHandler.java @@ -37,7 +37,7 @@ public interface BinaryMetadataHandler { public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered) throws BinaryObjectException; /** - * Adds meta data locally on current node without sending any messages. + * Adds metadata locally on current node without sending any messages. * * @param typeId Type ID. * @param meta Metadata. diff --git a/modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectBinaryContexTest.java b/modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectBinaryContextTest.java similarity index 97% rename from modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectBinaryContexTest.java rename to modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectBinaryContextTest.java index ec053828e1509..9b745d2012950 100644 --- a/modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectBinaryContexTest.java +++ b/modules/core/src/test/java/org/apache/ignite/internal/IgniteClientReconnectBinaryContextTest.java @@ -30,7 +30,7 @@ /** * */ -public class IgniteClientReconnectBinaryContexTest extends IgniteClientReconnectAbstractTest { +public class IgniteClientReconnectBinaryContextTest extends IgniteClientReconnectAbstractTest { /** {@inheritDoc} */ @Override protected int serverCount() { return 1; diff --git a/modules/core/src/test/java/org/apache/ignite/internal/client/thin/BinaryMetadataChannelFailureTest.java b/modules/core/src/test/java/org/apache/ignite/internal/client/thin/BinaryMetadataChannelFailureTest.java new file mode 100644 index 0000000000000..e2c5130d88f7f --- /dev/null +++ b/modules/core/src/test/java/org/apache/ignite/internal/client/thin/BinaryMetadataChannelFailureTest.java @@ -0,0 +1,168 @@ +/* + * 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.client.thin; + +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import org.apache.ignite.Ignition; +import org.apache.ignite.binary.BinaryBasicIdMapper; +import org.apache.ignite.binary.BinaryIdMapper; +import org.apache.ignite.binary.BinaryObject; +import org.apache.ignite.binary.BinaryObjectBuilder; +import org.apache.ignite.client.ClientCache; +import org.apache.ignite.client.IgniteClient; +import org.apache.ignite.configuration.BinaryConfiguration; +import org.apache.ignite.configuration.ClientConfiguration; +import org.apache.ignite.internal.IgniteEx; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +/** + * Checks the thin client binary metadata cache when a channel fails while a binary object is being built. + */ +@RunWith(Parameterized.class) +public class BinaryMetadataChannelFailureTest extends AbstractThinClientTest { + /** Type name. */ + private static final String TYPE_NAME = "TestType"; + + /** Field names. */ + private static final List FIELDS = Arrays.asList("f0", "f1", "f2"); + + /** Field names. */ + private static final List FIELDS_NEW = Arrays.asList("f0", "f1", "f2", "f3"); + + /** Field whose ID resolution triggers the channel failure. */ + private static final String HOOK_FIELD = "f1"; + + /** If {@code true} put a different value. */ + @Parameterized.Parameter(0) + public Boolean differentValue; + + /** Parameters. */ + @Parameterized.Parameters(name = "differentValue = {0}") + public static Collection parameters() { + return List.of(true, false); + } + + /** {@inheritDoc} */ + @Override protected void afterTest() throws Exception { + super.afterTest(); + + stopAllGrids(); + } + + /** + * Channel failure happens after the builder has read the cached type metadata, but before the new schema is + * registered. The client must not end up with a cached type that lacks the fields of the registered schema. + * + * @throws Exception If failed. + */ + @Test + public void testChannelFailureDuringBuild() throws Exception { + IgniteEx srv = startGrid(0); + + HookIdMapper idMapper = new HookIdMapper(); + + ClientConfiguration cfg = getClientConfiguration(srv) + .setBinaryConfiguration(new BinaryConfiguration().setIdMapper(idMapper)); + + try (IgniteClient client = Ignition.startClient(cfg)) { + ClientCache cache = client.getOrCreateCache(DEFAULT_CACHE_NAME).withKeepBinary(); + + BinaryObjectBuilder builder = builder(client, FIELDS); + + // Type with all fields is registered and cached by the client. + cache.put(0, builder.build()); + + assertTrue(cache.get(0).type().fieldNames().containsAll(FIELDS)); + + idMapper.hook = () -> { + dropAllThinClientConnections(srv); + + // The next request detects the broken channel and runs the channel failure listeners. + client.cacheNames(); + }; + + if (differentValue) + builder = builder(client, FIELDS_NEW); + + cache.put(1, builder.build()); + + assertNull("Channel failure hook was not triggered", idMapper.hook); + + BinaryObject obj = cache.get(1); + + List ethalon = differentValue ? FIELDS_NEW : FIELDS; + + for (String field : ethalon) + assertEquals(field, obj.field(field)); + + assertTrue(srv.binary().type(TYPE_NAME).fieldNames().containsAll(ethalon)); + + Collection clientFields = obj.type().fieldNames(); + + assertTrue("Client type metadata lacks fields of the object schema: " + clientFields, + clientFields.containsAll(ethalon)); + } + } + + /** + * @param client Client. + * @return Builder with all fields set. + */ + private static BinaryObjectBuilder builder(IgniteClient client, List fields) { + BinaryObjectBuilder builder = client.binary().builder(TYPE_NAME); + + for (String field : fields) + builder.setField(field, field, String.class); + + return builder; + } + + /** + * ID mapper that runs a one-time hook on resolving {@link #HOOK_FIELD} ID. + * Just a simple way to reproduce concurrent disconnection during Binary Object meta processing. + */ + private static class HookIdMapper implements BinaryIdMapper { + /** Delegate. */ + private final BinaryIdMapper delegate = new BinaryBasicIdMapper(true); + + /** Hook. */ + private volatile Runnable hook; + + /** {@inheritDoc} */ + @Override public int typeId(String typeName) { + return delegate.typeId(typeName); + } + + /** {@inheritDoc} */ + @Override public int fieldId(int typeId, String fieldName) { + Runnable hook0 = hook; + + if (hook0 != null && HOOK_FIELD.equals(fieldName)) { + hook = null; + + hook0.run(); + } + + return delegate.fieldId(typeId, fieldName); + } + } +} diff --git a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteClientReconnectTestSuite.java b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteClientReconnectTestSuite.java index fd01faeeb8553..96eaea409caef 100644 --- a/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteClientReconnectTestSuite.java +++ b/modules/core/src/test/java/org/apache/ignite/testsuites/IgniteClientReconnectTestSuite.java @@ -22,7 +22,7 @@ import org.apache.ignite.internal.IgniteClientReconnectApiExceptionTest; import org.apache.ignite.internal.IgniteClientReconnectAtomicsTest; import org.apache.ignite.internal.IgniteClientReconnectAtomicsWithLostPartitionsTest; -import org.apache.ignite.internal.IgniteClientReconnectBinaryContexTest; +import org.apache.ignite.internal.IgniteClientReconnectBinaryContextTest; import org.apache.ignite.internal.IgniteClientReconnectCacheTest; import org.apache.ignite.internal.IgniteClientReconnectCollectionsTest; import org.apache.ignite.internal.IgniteClientReconnectComputeTest; @@ -48,7 +48,7 @@ IgniteClientReconnectDiscoveryStateTest.class, IgniteClientReconnectCacheTest.class, IgniteClientReconnectDelayedSpiTest.class, - IgniteClientReconnectBinaryContexTest.class, + IgniteClientReconnectBinaryContextTest.class, IgniteClientReconnectContinuousProcessorTest.class, IgniteClientCheckClusterGroupLocalIdAfterReconnect.class, IgniteClientReconnectComputeTest.class, diff --git a/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java b/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java index 97021b7896e6e..fc035b35b2ce0 100644 --- a/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java +++ b/modules/indexing/src/test/java/org/apache/ignite/client/ClientTestSuite.java @@ -20,6 +20,7 @@ import org.apache.ignite.client.thin.ExtraColumnInH2RowsTest; import org.apache.ignite.internal.client.thin.AffinityMetricsTest; import org.apache.ignite.internal.client.thin.AtomicLongTest; +import org.apache.ignite.internal.client.thin.BinaryMetadataChannelFailureTest; import org.apache.ignite.internal.client.thin.BlockingTxOpsTest; import org.apache.ignite.internal.client.thin.CacheAsyncTest; import org.apache.ignite.internal.client.thin.CacheConfigurationTest; @@ -117,6 +118,7 @@ ReliableChannelDuplicationTest.class, CacheExceptionsTest.class, CacheConfigurationTest.class, + BinaryMetadataChannelFailureTest.class, }) public class ClientTestSuite { // No-op.