You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2020/07/06 09:11:13 UTC

[ignite] branch master updated: IGNITE-13192 Java thin client: Fix binary type schema registration for already cached typeId - Fixes #7990.

This is an automated email from the ASF dual-hosted git repository.

alexpl 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 236f30b  IGNITE-13192 Java thin client: Fix binary type schema registration for already cached typeId - Fixes #7990.
236f30b is described below

commit 236f30bbf2c82225f5d26ad6bf69347dec122479
Author: Aleksey Plekhanov <pl...@gmail.com>
AuthorDate: Mon Jul 6 14:08:58 2020 +0500

    IGNITE-13192 Java thin client: Fix binary type schema registration for already cached typeId - Fixes #7990.
    
    Signed-off-by: Aleksey Plekhanov <pl...@gmail.com>
---
 .../internal/client/thin/TcpIgniteClient.java      | 101 ++++++++++++++-------
 .../org/apache/ignite/client/IgniteBinaryTest.java |  38 ++++++++
 .../apache/ignite/client/FunctionalQueryTest.java  |  33 +++++++
 3 files changed, 140 insertions(+), 32 deletions(-)

diff --git a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
index b5ab5e4..7fd0a1d 100644
--- a/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
+++ b/modules/core/src/main/java/org/apache/ignite/internal/client/thin/TcpIgniteClient.java
@@ -301,11 +301,16 @@ public class TcpIgniteClient implements IgniteClient {
         /** {@inheritDoc} */
         @Override public void addMeta(int typeId, BinaryType meta, boolean failIfUnregistered)
             throws BinaryObjectException {
-            if (cache.metadata(typeId) == null) {
+            BinaryType oldType = cache.metadata(typeId);
+            BinaryMetadata oldMeta = oldType == null ? null : ((BinaryTypeImpl)oldType).metadata();
+            BinaryMetadata newMeta = ((BinaryTypeImpl)meta).metadata();
+
+            // If type wasn't registered before or metadata changed, send registration request.
+            if (oldType == null || BinaryUtils.mergeMetadata(oldMeta, newMeta) != oldMeta) {
                 try {
                     ch.request(
                         ClientOperation.PUT_BINARY_TYPE,
-                        req -> serDes.binaryMetadata(((BinaryTypeImpl)meta).metadata(), req.out())
+                        req -> serDes.binaryMetadata(newMeta, req.out())
                     );
                 }
                 catch (ClientException e) {
@@ -326,15 +331,8 @@ public class TcpIgniteClient implements IgniteClient {
         @Override public BinaryType metadata(int typeId) throws BinaryObjectException {
             BinaryType meta = cache.metadata(typeId);
 
-            if (meta == null) {
-                BinaryMetadata meta0 = metadata0(typeId);
-
-                if (meta0 != null) {
-                    meta = new BinaryTypeImpl(marsh.context(), meta0);
-
-                    cache.addMeta(typeId, meta, false);
-                }
-            }
+            if (meta == null)
+                meta = requestAndCacheBinaryType(typeId);
 
             return meta;
         }
@@ -343,34 +341,22 @@ public class TcpIgniteClient implements IgniteClient {
         @Override public BinaryMetadata metadata0(int typeId) throws BinaryObjectException {
             BinaryMetadata meta = cache.metadata0(typeId);
 
-            if (meta == null) {
-                try {
-                    meta = ch.service(
-                        ClientOperation.GET_BINARY_TYPE,
-                        req -> req.out().writeInt(typeId),
-                        res -> {
-                            try {
-                                return res.in().readBoolean() ? serDes.binaryMetadata(res.in()) : null;
-                            }
-                            catch (IOException e) {
-                                throw new BinaryObjectException(e);
-                            }
-                        }
-                    );
-                }
-                catch (ClientException e) {
-                    throw new BinaryObjectException(e);
-                }
-            }
+            if (meta == null)
+                meta = requestBinaryMetadata(typeId);
 
             return meta;
         }
 
         /** {@inheritDoc} */
         @Override public BinaryType metadata(int typeId, int schemaId) throws BinaryObjectException {
-            BinaryType meta = metadata(typeId);
+            BinaryType meta = cache.metadata(typeId);
+
+            if (hasSchema(meta, schemaId))
+                return meta;
 
-            return meta != null && ((BinaryTypeImpl)meta).metadata().hasSchema(schemaId) ? meta : null;
+            meta = requestAndCacheBinaryType(typeId);
+
+            return hasSchema(meta, schemaId) ? meta : null;
         }
 
         /** {@inheritDoc} */
@@ -379,6 +365,57 @@ public class TcpIgniteClient implements IgniteClient {
         }
 
         /**
+         * @param type Binary type.
+         * @param schemaId Schema id.
+         */
+        private boolean hasSchema(BinaryType type, int schemaId) {
+            return type != null && ((BinaryTypeImpl)type).metadata().hasSchema(schemaId);
+        }
+
+        /**
+         * Request binary metadata from server and add binary type to cache.
+         *
+         * @param typeId Type id.
+         */
+        private BinaryType requestAndCacheBinaryType(int typeId) throws BinaryObjectException {
+            BinaryMetadata meta0 = requestBinaryMetadata(typeId);
+
+            if (meta0 == null)
+                return null;
+
+            BinaryType meta = new BinaryTypeImpl(marsh.context(), meta0);
+
+            cache.addMeta(typeId, meta, false);
+
+            return meta;
+        }
+
+        /**
+         * Request binary metadata for type id from the server.
+         *
+         * @param typeId Type id.
+         */
+        private BinaryMetadata requestBinaryMetadata(int typeId) throws BinaryObjectException {
+            try {
+                return ch.service(
+                    ClientOperation.GET_BINARY_TYPE,
+                    req -> req.out().writeInt(typeId),
+                    res -> {
+                        try {
+                            return res.in().readBoolean() ? serDes.binaryMetadata(res.in()) : null;
+                        }
+                        catch (IOException e) {
+                            throw new BinaryObjectException(e);
+                        }
+                    }
+                );
+            }
+            catch (ClientException e) {
+                throw new BinaryObjectException(e);
+            }
+        }
+
+        /**
          * Clear local cache on reconnect.
          */
         void onReconnect() {
diff --git a/modules/core/src/test/java/org/apache/ignite/client/IgniteBinaryTest.java b/modules/core/src/test/java/org/apache/ignite/client/IgniteBinaryTest.java
index a201a6e..9f93e2f 100644
--- a/modules/core/src/test/java/org/apache/ignite/client/IgniteBinaryTest.java
+++ b/modules/core/src/test/java/org/apache/ignite/client/IgniteBinaryTest.java
@@ -26,6 +26,7 @@ import org.apache.ignite.IgniteBinary;
 import org.apache.ignite.IgniteCache;
 import org.apache.ignite.Ignition;
 import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
 import org.apache.ignite.binary.BinaryType;
 import org.apache.ignite.configuration.BinaryConfiguration;
 import org.apache.ignite.configuration.ClientConfiguration;
@@ -148,6 +149,43 @@ public class IgniteBinaryTest {
     }
 
     /**
+     * Check that binary type schema updates are propagated from client to server and from server to client.
+     */
+    @Test
+    public void testCompactFooterModifiedSchemaRegistration() throws Exception {
+        try (Ignite ignite = Ignition.start(Config.getServerConfiguration())) {
+            ignite.getOrCreateCache(Config.DEFAULT_CACHE_NAME);
+
+            ClientConfiguration cfg = new ClientConfiguration().setAddresses(Config.SERVER)
+                .setBinaryConfiguration(new BinaryConfiguration().setCompactFooter(true));
+
+            try (IgniteClient client1 = Ignition.startClient(cfg); IgniteClient client2 = Ignition.startClient(cfg)) {
+                ClientCache<Integer, Object> cache1 = client1.cache(Config.DEFAULT_CACHE_NAME).withKeepBinary();
+                ClientCache<Integer, Object> cache2 = client2.cache(Config.DEFAULT_CACHE_NAME).withKeepBinary();
+
+                String type = "Person";
+
+                // Register type and schema.
+                BinaryObjectBuilder builder = client1.binary().builder(type);
+
+                BinaryObject val1 = builder.setField("Name", "Person 1").build();
+
+                cache1.put(1, val1);
+
+                assertEquals("Person 1", ((BinaryObject)cache2.get(1)).field("Name"));
+
+                // Update schema.
+                BinaryObject val2 = builder.setField("Name", "Person 2").setField("Age", 2).build();
+
+                cache1.put(2, val2);
+
+                assertEquals("Person 2", ((BinaryObject)cache2.get(2)).field("Name"));
+                assertEquals((Integer)2, ((BinaryObject)cache2.get(2)).field("Age"));
+            }
+        }
+    }
+
+    /**
      * Binary Object API:
      * {@link IgniteBinary#typeId(String)}
      * {@link IgniteBinary#toBinary(Object)}
diff --git a/modules/indexing/src/test/java/org/apache/ignite/client/FunctionalQueryTest.java b/modules/indexing/src/test/java/org/apache/ignite/client/FunctionalQueryTest.java
index c733ecc..f1b4e2e 100644
--- a/modules/indexing/src/test/java/org/apache/ignite/client/FunctionalQueryTest.java
+++ b/modules/indexing/src/test/java/org/apache/ignite/client/FunctionalQueryTest.java
@@ -238,6 +238,39 @@ public class FunctionalQueryTest {
     }
 
     /** */
+    @Test
+    public void testMixedQueryAndCacheApiOperations() throws Exception {
+        try (Ignite ignored = Ignition.start(Config.getServerConfiguration());
+             IgniteClient client = Ignition.startClient(
+                 new ClientConfiguration().setBinaryConfiguration(new BinaryConfiguration().setCompactFooter(true))
+                     .setAddresses(Config.SERVER))
+        ) {
+            String cacheName = "PersonCache";
+
+            client.query(
+                new SqlFieldsQuery(String.format(
+                    "CREATE TABLE IF NOT EXISTS Person (key INT PRIMARY KEY, name VARCHAR) WITH \"VALUE_TYPE=%s,CACHE_NAME=%s\"",
+                    Person.class.getName(), cacheName
+                )).setSchema("PUBLIC")
+            ).getAll();
+
+            client.query(new SqlFieldsQuery("INSERT INTO Person(key, name) VALUES(?, ?)")
+                .setArgs(1, "Person 1")
+                .setSchema("PUBLIC")
+            ).getAll();
+
+            ClientCache<Integer, Person> cache = client.cache(cacheName);
+
+            cache.put(2, new Person(2, "Person 2"));
+
+            assertEquals("Person 1", cache.get(1).getName());
+
+            assertEquals("Person 2", client.query(
+                new SqlFieldsQuery("SELECT name FROM PUBLIC.Person WHERE key = 2")).getAll().get(0).get(0));
+        }
+    }
+
+    /** */
     private static ClientConfiguration getClientConfiguration() {
         return new ClientConfiguration().setAddresses(Config.SERVER)
             .setSendBufferSize(0)