You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/06/16 14:08:13 UTC

[GitHub] [ignite] tledkov-gridgain opened a new pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

tledkov-gridgain opened a new pull request #7936:
URL: https://github.com/apache/ignite/pull/7936


   Thank you for submitting the pull request to the Apache Ignite.
   
   In order to streamline the review of the contribution 
   we ask you to ensure the following steps have been taken:
   
   ### The Contribution Checklist
   - [ ] There is a single JIRA ticket related to the pull request. 
   - [ ] The web-link to the pull request is attached to the JIRA ticket.
   - [ ] The JIRA ticket has the _Patch Available_ state.
   - [ ] The pull request body describes changes that have been made. 
   The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
   - [ ] The pull request title is treated as the final commit message. 
   The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
   - [ ] A reviewer has been mentioned through the JIRA comments 
   (see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442762668



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveResult.java
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.cache.binary;
+
+import org.apache.ignite.binary.BinaryObjectException;
+
+/**
+ * Represents result of metadata remove.
+ */
+final class MetadataRemoveResult {
+    /** */
+    private final ResultType resType;
+
+    /** */
+    private final BinaryObjectException error;
+
+    /**
+     * @param resType Response type.
+     * @param error Error.
+     */
+    private MetadataRemoveResult(ResultType resType, BinaryObjectException error) {
+        this.resType = resType;
+        this.error = error;
+    }
+
+    /**
+     *
+     */
+    boolean rejected() {
+        return resType == ResultType.REJECT;
+    }
+
+    /**
+     *
+     */
+    BinaryObjectException error() {
+        return error;
+    }
+
+    /**
+     */
+    static MetadataRemoveResult createSuccessfulResult() {
+        return new MetadataRemoveResult(ResultType.SUCCESS, null);
+    }
+
+    /**
+     * @param err Error lead to request failure.
+     */
+    static MetadataRemoveResult createFailureResult(BinaryObjectException err) {

Review comment:
       Mark @NotNull




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442771975



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataRemoveCommand.java
##########
@@ -0,0 +1,153 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.logging.Logger;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.CommandLogger;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataMarshalled;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataRemoveTask;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataTypeArgs;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+/**
+ *
+ */
+public class MetadataRemoveCommand
+    extends MetadataAbstractSubCommand<MetadataTypeArgs, MetadataMarshalled> {
+    /** Output file name. */
+    public static String OPT_OUT_FILE_NAME = "--out";

Review comment:
       Can this be final ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442750227



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/CommandArgIterator.java
##########
@@ -97,19 +97,52 @@ public String peekNextArg() {
         return peekedArg;
     }
 
+    /**
+     * @return Numeric value.
+     */
+    public long nextPositiveLongArg(String argName) {
+        long val = nextLongArg(argName);
+
+        if (val < 0)

Review comment:
       Fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442755047



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataListResult.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import java.util.ArrayList;
+import java.util.Collection;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.dto.IgniteDataTransferObject;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Represents information about cluster metadata.
+ */
+@GridInternal
+public class MetadataListResult extends IgniteDataTransferObject {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** Cluster metadata. */
+    private Collection<BinaryMetadata> meta = new ArrayList<>();

Review comment:
       "meta" collection is never modified and can be initialized with Collection.emptyList()




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442761370



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java
##########
@@ -1536,6 +1574,41 @@ public void setBinaryMetadataFileStoreDir(@Nullable File binaryMetadataFileStore
         this.binaryMetadataFileStoreDir = binaryMetadataFileStoreDir;
     }
 
+    /** {@inheritDoc} */
+    @Override public void removeType(int typeId) {
+        if (metadataLocCache.computeIfAbsent(typeId,
+                (id) -> {
+                    throw new IgniteException("Failed to remove metadata, type not found: " + id);
+                })
+            .removing())
+            throw new IgniteException("Failed to remove metadata, type is being removed: " + typeId);

Review comment:
       This looks hard to understand.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442786278



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/MetadataCommand.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.commandline.meta;
+
+import java.util.logging.Logger;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.commandline.Command;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.subcommands.MetadataRemoveCommand;
+import org.apache.ignite.internal.commandline.meta.subcommands.MetadataUpdateCommand;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataTypeArgs;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_ENABLE_EXPERIMENTAL_COMMAND;
+import static org.apache.ignite.internal.commandline.Command.usage;
+import static org.apache.ignite.internal.commandline.CommandHandler.UTILITY_NAME;
+import static org.apache.ignite.internal.commandline.CommandList.METADATA;
+import static org.apache.ignite.internal.commandline.CommandLogger.optional;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.DETAILS;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.HELP;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.LIST;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.REMOVE;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.UPDATE;
+
+/**
+ *
+ */
+public class MetadataCommand implements Command<Object> {
+    /**
+     *
+     */
+    private Command<?> delegate;
+
+    /** {@inheritDoc} */
+    @Override public void printUsage(Logger log) {
+        if (!experimentalEnabled())
+            return;
+
+        usage(log, "Print metadata command help:",
+            METADATA,
+            HELP.toString()
+        );
+
+        usage(log, "Print list of binary metadata types:",
+            METADATA,
+            LIST.toString()
+        );
+
+        usage(log, "Print detailed info about specified binary type " +
+                "(the type must be specified by type name or by type identifier):",
+            METADATA,
+            DETAILS.toString(),
+            optional(MetadataTypeArgs.OPT_TYPE_ID, "<typeId>"),
+            optional(MetadataTypeArgs.OPT_TYPE_NAME, "<typeName>")
+        );
+
+        usage(log, "Remove the metadata of the specified type " +
+                "(the type must be specified by type name or by type identifier) from cluster and saves the removed " +
+                "metadata to the specified file. \n" +
+                "If the file name isn't specified the output file name is: '<typeId>.bin'",
+            METADATA,
+            REMOVE.toString(),
+            optional(MetadataTypeArgs.OPT_TYPE_ID, "<typeId>"),
+            optional(MetadataTypeArgs.OPT_TYPE_NAME, "<typeName>"),
+            optional(MetadataRemoveCommand.OPT_OUT_FILE_NAME, "<fileName>")
+        );
+
+        usage(log, "Update cluster metadata from specified file (file name is required)",
+            METADATA,
+            UPDATE.toString(),
+            MetadataUpdateCommand.OPT_IN_FILE_NAME, "<fileName>"

Review comment:
       I guess OPT_ prefix means 'option' not optional. May be changed to ARG_




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r445605615



##########
File path: modules/indexing/src/test/java/org/apache/ignite/util/GridCommandHandlerMetadataTest.java
##########
@@ -271,6 +276,65 @@ public void testDropThinConnectionsOnRemove() throws Exception {
         }
     }
 
+    /**
+     * Check the all thin connections are dropped on the command '--meta remove' and '--meta update'.
+     * Steps:
+     * - opens JDBC thin client connection.
+     * - executes: CREATE TABLE test(id INT PRIMARY KEY, objVal OTHER).
+     * - inserts the instance of the 'TestValue' class to the table.
+     * - removes the type 'TestValue' by cmd line.
+     * - executes any command on JDBC driver to detect disconnect.
+     * - checks metadata on client side. It must be empty.
+     */
+    @Test
+    public void testDropJdbcThinConnectionsOnRemove() throws Exception {
+        injectTestSystemOut();
+
+        Path typeFile = FS.getPath("type0.bin");
+
+        try (Connection conn = DriverManager.getConnection("jdbc:ignite:thin://127.0.0.1")) {
+            try (final Statement stmt = conn.createStatement()) {
+                stmt.execute("CREATE TABLE test(id INT PRIMARY KEY, objVal OTHER)");
+
+                try (PreparedStatement pstmt = conn.prepareStatement("INSERT INTO test(id, objVal) VALUES (?, ?)")) {
+                    pstmt.setInt(1, 0);
+                    pstmt.setObject(2, new TestValue());
+
+                    pstmt.execute();
+                }
+
+                stmt.execute("DELETE FROM test WHERE id >= 0");
+            }
+
+            HashMap<Integer, BinaryType> metasOld = GridTestUtils.getFieldValue(conn, "metaHnd", "cache", "metas");
+
+            assertFalse(metasOld.isEmpty());
+
+            assertEquals(EXIT_CODE_OK, execute("--meta", "remove",
+                "--typeName", TestValue.class.getName(),
+                "--out", typeFile.toString()));
+
+            // Executes anu command to check disconnect.

Review comment:
       Type fixed: anu -> any




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442761875



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveProposedMessage.java
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.cache.binary;
+
+import java.util.UUID;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * <b>MetadataRemoveProposedMessage</b> and {@link MetadataRemoveAcceptedMessage} messages make a basis for
+ * discovery-based protocol for manage {@link BinaryMetadata metadata} describing objects in binary format stored in Ignite caches.
+ *
+ */
+public final class MetadataRemoveProposedMessage implements DiscoveryCustomMessage {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** */
+    private final IgniteUuid id = IgniteUuid.randomUuid();
+
+    /** Node UUID which initiated metadata update. */
+    private final UUID origNodeId;
+
+    /** Metadata type id. */
+    private final int typeId;
+
+    /** Message acceptance status. */
+    private ProposalStatus status = ProposalStatus.SUCCESSFUL;
+
+    /** Message received on coordinator. */
+    private boolean onCoordinator = true;
+
+    /** */
+    private BinaryObjectException err;
+
+    /**
+     * @param typeId Binary type ID.
+     * @param origNodeId ID of node requested update.
+     */
+    public MetadataRemoveProposedMessage(int typeId, UUID origNodeId) {
+        assert origNodeId != null;
+
+        this.origNodeId = origNodeId;
+
+        this.typeId = typeId;
+    }
+
+    /**
+     * {@inheritDoc}
+     */

Review comment:
       /** {@inheritDoc} */

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveProposedMessage.java
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.cache.binary;
+
+import java.util.UUID;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * <b>MetadataRemoveProposedMessage</b> and {@link MetadataRemoveAcceptedMessage} messages make a basis for
+ * discovery-based protocol for manage {@link BinaryMetadata metadata} describing objects in binary format stored in Ignite caches.
+ *
+ */
+public final class MetadataRemoveProposedMessage implements DiscoveryCustomMessage {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** */
+    private final IgniteUuid id = IgniteUuid.randomUuid();
+
+    /** Node UUID which initiated metadata update. */
+    private final UUID origNodeId;
+
+    /** Metadata type id. */
+    private final int typeId;
+
+    /** Message acceptance status. */
+    private ProposalStatus status = ProposalStatus.SUCCESSFUL;
+
+    /** Message received on coordinator. */
+    private boolean onCoordinator = true;
+
+    /** */
+    private BinaryObjectException err;
+
+    /**
+     * @param typeId Binary type ID.
+     * @param origNodeId ID of node requested update.
+     */
+    public MetadataRemoveProposedMessage(int typeId, UUID origNodeId) {
+        assert origNodeId != null;
+
+        this.origNodeId = origNodeId;
+
+        this.typeId = typeId;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override public IgniteUuid id() {
+        return id;
+    }
+
+    /**
+     * {@inheritDoc}
+     */

Review comment:
       /** {@inheritDoc} */

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveProposedMessage.java
##########
@@ -0,0 +1,161 @@
+/*
+ * 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.cache.binary;
+
+import java.util.UUID;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * <b>MetadataRemoveProposedMessage</b> and {@link MetadataRemoveAcceptedMessage} messages make a basis for
+ * discovery-based protocol for manage {@link BinaryMetadata metadata} describing objects in binary format stored in Ignite caches.
+ *
+ */
+public final class MetadataRemoveProposedMessage implements DiscoveryCustomMessage {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** */
+    private final IgniteUuid id = IgniteUuid.randomUuid();
+
+    /** Node UUID which initiated metadata update. */
+    private final UUID origNodeId;
+
+    /** Metadata type id. */
+    private final int typeId;
+
+    /** Message acceptance status. */
+    private ProposalStatus status = ProposalStatus.SUCCESSFUL;
+
+    /** Message received on coordinator. */
+    private boolean onCoordinator = true;
+
+    /** */
+    private BinaryObjectException err;
+
+    /**
+     * @param typeId Binary type ID.
+     * @param origNodeId ID of node requested update.
+     */
+    public MetadataRemoveProposedMessage(int typeId, UUID origNodeId) {
+        assert origNodeId != null;
+
+        this.origNodeId = origNodeId;
+
+        this.typeId = typeId;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Override public IgniteUuid id() {
+        return id;
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    @Nullable @Override public DiscoveryCustomMessage ackMessage() {
+        return (status == ProposalStatus.SUCCESSFUL) ? new MetadataRemoveAcceptedMessage(typeId) : null;
+    }
+
+    /**
+     * {@inheritDoc}
+     */

Review comment:
       /** {@inheritDoc} */




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442743689



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/CommandArgIterator.java
##########
@@ -97,19 +97,52 @@ public String peekNextArg() {
         return peekedArg;
     }
 
+    /**
+     * @return Numeric value.
+     */
+    public long nextPositiveLongArg(String argName) {
+        long val = nextLongArg(argName);
+
+        if (val < 0)

Review comment:
       Here, val is "non-negative" rather than "positive"
   Let's rename methods.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] korlov42 commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442816668



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveProposedMessage.java
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.cache.binary;
+
+import java.util.UUID;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * <b>MetadataRemoveProposedMessage</b> and {@link MetadataRemoveAcceptedMessage} messages make a basis for
+ * discovery-based protocol for manage {@link BinaryMetadata metadata} describing objects in binary format stored in Ignite caches.
+ *
+ */
+public final class MetadataRemoveProposedMessage implements DiscoveryCustomMessage {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** */
+    private final IgniteUuid id = IgniteUuid.randomUuid();
+
+    /** Node UUID which initiated metadata update. */
+    private final UUID origNodeId;
+
+    /** Metadata type id. */
+    private final int typeId;
+
+    /** Message acceptance status. */
+    private ProposalStatus status = ProposalStatus.SUCCESSFUL;
+
+    /** Message received on coordinator. */
+    private boolean onCoordinator = true;
+
+    /** */
+    private BinaryObjectException err;
+
+    /**
+     * @param typeId Binary type ID.
+     * @param origNodeId ID of node requested update.
+     */
+    public MetadataRemoveProposedMessage(int typeId, UUID origNodeId) {
+        assert origNodeId != null;
+
+        this.origNodeId = origNodeId;
+
+        this.typeId = typeId;
+    }
+
+    /** {@inheritDoc} */
+    @Override public IgniteUuid id() {
+        return id;
+    }
+
+    /** {@inheritDoc} */
+    @Nullable @Override public DiscoveryCustomMessage ackMessage() {
+        return (status == ProposalStatus.SUCCESSFUL) ? new MetadataRemoveAcceptedMessage(typeId) : null;
+    }
+
+    /** {@inheritDoc} */
+    @Override public boolean isMutable() {
+        return true;
+    }
+
+
+    /** {@inheritDoc} */
+    @Nullable @Override public DiscoCache createDiscoCache(GridDiscoveryManager mgr,
+        AffinityTopologyVersion topVer, DiscoCache discoCache) {
+        throw new UnsupportedOperationException();
+    }
+
+    /**
+     * @param err Error caused this update to be rejected.
+     */
+    void markRejected(BinaryObjectException err) {
+        status = ProposalStatus.REJECTED;
+        this.err = err;
+    }
+
+    /**
+     *

Review comment:
       Let's use one-line stub here, or as a better option let's provide minimal javadoc

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveProposedMessage.java
##########
@@ -0,0 +1,155 @@
+/*
+ * 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.cache.binary;
+
+import java.util.UUID;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * <b>MetadataRemoveProposedMessage</b> and {@link MetadataRemoveAcceptedMessage} messages make a basis for
+ * discovery-based protocol for manage {@link BinaryMetadata metadata} describing objects in binary format stored in Ignite caches.

Review comment:
       It's better to break the line since it too long. Also empty line at the end of comment should be removed 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442764559



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/binary/BinaryMetadataRemoveTest.java
##########
@@ -0,0 +1,312 @@
+/*
+ * 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.cache.binary;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.IgniteInterruptedCheckedException;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.spi.discovery.DiscoverySpiCustomMessage;
+import org.apache.ignite.spi.discovery.DiscoverySpiListener;
+import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class BinaryMetadataRemoveTest extends GridCommonAbstractTest {
+    /** Max retry cont. */
+    private static final int MAX_RETRY_CONT = 10;
+
+    /**
+     *

Review comment:
       Javadoc.
   Let's make one-liner.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442758314



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataRemoveTask.java
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.compute.ComputeJobContext;
+import org.apache.ignite.compute.ComputeJobResult;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.commandline.cache.CheckIndexInlineSizes;
+import org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorMultiNodeTask;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteInClosure;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.plugin.security.SecurityPermission;
+import org.apache.ignite.resources.IgniteInstanceResource;
+import org.apache.ignite.resources.JobContextResource;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task for {@link MetadataRemoveTask} command.
+ */
+@GridInternal
+public class MetadataRemoveTask extends VisorMultiNodeTask<MetadataTypeArgs, MetadataMarshalled, MetadataMarshalled> {
+    /**
+     *

Review comment:
       Unfortunately, IDEA doesn't like empty one-line comments. =(




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442766141



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveResult.java
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.cache.binary;
+
+import org.apache.ignite.binary.BinaryObjectException;
+
+/**
+ * Represents result of metadata remove.
+ */
+final class MetadataRemoveResult {
+    /** */
+    private final ResultType resType;
+
+    /** */
+    private final BinaryObjectException error;
+
+    /**
+     * @param resType Response type.
+     * @param error Error.
+     */
+    private MetadataRemoveResult(ResultType resType, BinaryObjectException error) {
+        this.resType = resType;
+        this.error = error;
+    }
+
+    /**
+     *
+     */
+    boolean rejected() {
+        return resType == ResultType.REJECT;
+    }
+
+    /**
+     *
+     */
+    BinaryObjectException error() {
+        return error;
+    }
+
+    /**
+     */
+    static MetadataRemoveResult createSuccessfulResult() {
+        return new MetadataRemoveResult(ResultType.SUCCESS, null);
+    }
+
+    /**
+     * @param err Error lead to request failure.
+     */
+    static MetadataRemoveResult createFailureResult(BinaryObjectException err) {

Review comment:
       MetadataRemoveResult is unused. I'll drop the class




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] korlov42 commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r445601543



##########
File path: modules/indexing/src/test/java/org/apache/ignite/util/GridCommandHandlerMetadataTest.java
##########
@@ -271,6 +276,65 @@ public void testDropThinConnectionsOnRemove() throws Exception {
         }
     }
 
+    /**
+     * Check the all thin connections are dropped on the command '--meta remove' and '--meta update'.
+     * Steps:
+     * - opens JDBC thin client connection.
+     * - executes: CREATE TABLE test(id INT PRIMARY KEY, objVal OTHER).
+     * - inserts the instance of the 'TestValue' class to the table.
+     * - removes the type 'TestValue' by cmd line.
+     * - executes any command on JDBC driver to detect disconnect.
+     * - checks metadata on client side. It must be empty.
+     */
+    @Test
+    public void testDropJdbcThinConnectionsOnRemove() throws Exception {
+        injectTestSystemOut();
+
+        Path typeFile = FS.getPath("type0.bin");
+
+        try (Connection conn = DriverManager.getConnection("jdbc:ignite:thin://127.0.0.1")) {
+            try (final Statement stmt = conn.createStatement()) {
+                stmt.execute("CREATE TABLE test(id INT PRIMARY KEY, objVal OTHER)");
+
+                try (PreparedStatement pstmt = conn.prepareStatement("INSERT INTO test(id, objVal) VALUES (?, ?)")) {
+                    pstmt.setInt(1, 0);
+                    pstmt.setObject(2, new TestValue());
+
+                    pstmt.execute();
+                }
+
+                stmt.execute("DELETE FROM test WHERE id >= 0");
+            }
+
+            HashMap<Integer, BinaryType> metasOld = GridTestUtils.getFieldValue(conn, "metaHnd", "cache", "metas");
+
+            assertFalse(metasOld.isEmpty());
+
+            assertEquals(EXIT_CODE_OK, execute("--meta", "remove",
+                "--typeName", TestValue.class.getName(),
+                "--out", typeFile.toString()));
+
+            // Executes anu command to check disconnect.

Review comment:
       anu




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442743689



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/CommandArgIterator.java
##########
@@ -97,19 +97,52 @@ public String peekNextArg() {
         return peekedArg;
     }
 
+    /**
+     * @return Numeric value.
+     */
+    public long nextPositiveLongArg(String argName) {
+        long val = nextLongArg(argName);
+
+        if (val < 0)

Review comment:
       Actually, val is "non-negative" rather "positive"
   Let's rename methods.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain closed pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain closed pull request #7936:
URL: https://github.com/apache/ignite/pull/7936


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] korlov42 commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442798375



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataUpdateCommand.java
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.logging.Logger;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataMarshalled;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataUpdateTask;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+/** */
+public class MetadataUpdateCommand
+    extends MetadataAbstractSubCommand<MetadataMarshalled, MetadataMarshalled>
+{
+    /** Output file name. */
+    public static final String IN_FILE_NAME = "--in";
+
+    /** {@inheritDoc} */
+    @Override protected String taskName() {
+        return MetadataUpdateTask.class.getName();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt() {
+        return "Warning: the command will update the binary metadata at the cluster.";
+    }
+
+    /** {@inheritDoc} */
+    @Override public MetadataMarshalled parseArguments0(CommandArgIterator argIter) {
+        String opt = argIter.nextArg("--in");
+
+        if (!IN_FILE_NAME.equalsIgnoreCase(opt))
+            throw new IllegalArgumentException("");

Review comment:
       Let's provide meaningful message here 

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataDetailsCommand.java
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.logging.Logger;
+import java.util.stream.Collectors;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.binary.BinaryUtils;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataListResult;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataInfoTask;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataTypeArgs;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+import static org.apache.ignite.internal.commandline.CommandLogger.INDENT;
+
+/** */
+public class MetadataDetailsCommand
+    extends MetadataAbstractSubCommand<MetadataTypeArgs, MetadataListResult>
+{
+    /** {@inheritDoc} */
+    @Override protected String taskName() {
+        return MetadataInfoTask.class.getName();
+    }
+
+    /** {@inheritDoc} */
+    @Override public MetadataTypeArgs parseArguments0(CommandArgIterator argIter) {
+        return MetadataTypeArgs.parseArguments(argIter);
+    }
+
+    /** {@inheritDoc} */
+    @Override protected MetadataListResult execute0(
+        GridClientConfiguration clientCfg,
+        GridClient client
+    ) throws Exception {
+        GridClientCompute compute = client.compute();
+
+        Collection<GridClientNode> connectableNodes = compute.nodes(GridClientNode::connectable);
+
+        if (F.isEmpty(connectableNodes))
+            throw new GridClientDisconnectedException("Connectable nodes not found", null);
+
+        GridClientNode node = connectableNodes.stream()
+            .findAny().orElse(null);
+
+        if (node == null)
+            node = compute.balancer().balancedNode(connectableNodes);
+
+        return compute.projection(node).execute(
+            taskName(),
+            new VisorTaskArgument<>(node.nodeId(), arg(), false)
+        );
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void printResult(MetadataListResult res, Logger log) {
+        if (res.metadata() == null) {
+            log.info("Type not found");
+
+            return;
+        }
+
+        assert res.metadata().size() == 1 : "Unexpected  metadata results: " + res.metadata();
+
+        BinaryMetadata m = F.first(res.metadata());
+
+        log.info("typeId=" + printInt(m.typeId()));
+        log.info("typeName=" + m.typeName());
+        log.info("Fields:");
+
+        final Map<Integer, String> fldMap = new HashMap<>();
+        m.fieldsMap().forEach((name, fldMeta) -> {
+            log.info(INDENT +
+                "name=" + name +
+                ", type=" + BinaryUtils.fieldTypeName(fldMeta.typeId()) +
+                ", fieldId=" + printInt(fldMeta.fieldId()));
+
+            fldMap.put(fldMeta.fieldId(), name);
+        });
+
+        log.info("Schemas:");
+
+        m.schemas().forEach(s ->
+            log.info(INDENT +
+                "schemaId=" + printInt(s.schemaId()) +
+                ", fields=" + Arrays.stream(s.fieldIds())
+                    .mapToObj(fldMap::get)
+                    .collect(Collectors.toList())));
+    }
+
+    /**
+     * @param val Integer value.
+     * @return String.
+     */
+    private String printInt(int val) {

Review comment:
       let's move it to the MetadataAbstractSubCommand and reuse inside the MetadataListCommand

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataUpdateCommand.java
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.logging.Logger;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataMarshalled;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataUpdateTask;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+/** */
+public class MetadataUpdateCommand
+    extends MetadataAbstractSubCommand<MetadataMarshalled, MetadataMarshalled>
+{
+    /** Output file name. */
+    public static final String IN_FILE_NAME = "--in";
+
+    /** {@inheritDoc} */
+    @Override protected String taskName() {
+        return MetadataUpdateTask.class.getName();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt() {
+        return "Warning: the command will update the binary metadata at the cluster.";
+    }
+
+    /** {@inheritDoc} */
+    @Override public MetadataMarshalled parseArguments0(CommandArgIterator argIter) {
+        String opt = argIter.nextArg("--in");
+
+        if (!IN_FILE_NAME.equalsIgnoreCase(opt))
+            throw new IllegalArgumentException("");
+
+        Path inFile = FS.getPath(argIter.nextArg("input file name"));
+
+        try (InputStream is = Files.newInputStream(inFile)) {
+            ByteArrayOutputStream buf = new ByteArrayOutputStream();
+
+            U.copy(is, buf);
+
+            return new MetadataMarshalled(buf.toByteArray(), null);
+        }
+        catch (IOException e) {
+            throw new IllegalArgumentException("Cannot read metadata from " + inFile, e);
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override protected MetadataMarshalled execute0(

Review comment:
       Look like all commands have the same execute0() methods, except logic for finding nodes from GridClientCompute, so let's change this part

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataUpdateCommand.java
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.logging.Logger;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataMarshalled;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataUpdateTask;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+/** */
+public class MetadataUpdateCommand
+    extends MetadataAbstractSubCommand<MetadataMarshalled, MetadataMarshalled>
+{
+    /** Output file name. */
+    public static final String IN_FILE_NAME = "--in";
+
+    /** {@inheritDoc} */
+    @Override protected String taskName() {
+        return MetadataUpdateTask.class.getName();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt() {
+        return "Warning: the command will update the binary metadata at the cluster.";
+    }
+
+    /** {@inheritDoc} */
+    @Override public MetadataMarshalled parseArguments0(CommandArgIterator argIter) {
+        String opt = argIter.nextArg("--in");

Review comment:
       Look's like we waiting IN_FILE_NAME arg here, so let's replace constant with it




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] korlov42 commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442804913



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataRemoveTask.java
##########
@@ -0,0 +1,146 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.compute.ComputeJobContext;
+import org.apache.ignite.compute.ComputeJobResult;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.commandline.cache.CheckIndexInlineSizes;
+import org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorMultiNodeTask;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteInClosure;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.plugin.security.SecurityPermission;
+import org.apache.ignite.resources.IgniteInstanceResource;
+import org.apache.ignite.resources.JobContextResource;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task for {@link MetadataRemoveTask} command.
+ */
+@GridInternal
+public class MetadataRemoveTask extends VisorMultiNodeTask<MetadataTypeArgs, MetadataMarshalled, MetadataMarshalled> {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** {@inheritDoc} */
+    @Override protected VisorJob<MetadataTypeArgs, MetadataMarshalled> job(MetadataTypeArgs arg) {
+        return new MetadataRemoveJob(arg, debug);
+    }
+
+    /** {@inheritDoc} */
+    @Nullable @Override protected MetadataMarshalled reduce0(List<ComputeJobResult> results) {
+        if (results.get(0).getException() != null)

Review comment:
       Why result size is not checked here? 

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataTypeArgs.java
##########
@@ -0,0 +1,129 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.internal.GridKernalContext;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.dto.IgniteDataTransferObject;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/** */
+public class MetadataTypeArgs extends IgniteDataTransferObject {
+    /** Serial version uid. */
+    private static final long serialVersionUID = 0L;
+
+    /** Type name argument. */
+    public static final String TYPE_NAME = "--typeName";
+
+    /** Type ID argument. */
+    public static final String TYPE_ID = "--typeId";
+
+    /** Config. */
+    private String typeName;
+
+    /** Metrics. */
+    private Integer typeId;
+
+    /**
+     * Default constructor.
+     */
+    public MetadataTypeArgs() {
+        // No-op.
+    }
+
+    /** */
+    public MetadataTypeArgs(String typeName, Integer typeId) {
+        assert typeName != null ^ typeId != null;
+
+        this.typeName = typeName;
+        this.typeId = typeId;
+    }
+
+    /** */
+    public String typeName() {
+        return typeName;
+    }
+
+    /** */
+    public int typeId(GridKernalContext ctx) {
+        if (typeId != null)
+            return typeId;
+        else
+            return ctx.cacheObjects().typeId(typeName);
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void writeExternalData(ObjectOutput out) throws IOException {
+        out.writeBoolean(typeName != null);
+
+        if (typeName != null)
+            U.writeString(out, typeName);
+        else
+            out.writeInt(typeId);
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void readExternalData(byte protoVer, ObjectInput in) throws IOException, ClassNotFoundException {
+        boolean useName = in.readBoolean();
+
+        if (useName)
+            typeName = U.readString(in);
+        else
+            typeId = in.readInt();
+    }
+
+    /**
+     * @param argIter Command line arguments iterator.
+     * @return Metadata type argument.
+     */
+    public static MetadataTypeArgs parseArguments(CommandArgIterator argIter) {
+        String typeName = null;
+        Integer typeId = null;
+
+        while (argIter.hasNextSubArg() && typeName == null && typeId == null) {
+            String optName = argIter.nextArg("Expecting " + TYPE_NAME + " or " + TYPE_ID);
+
+            switch (optName) {
+                case TYPE_NAME:
+                    typeName = argIter.nextArg("type name");
+
+                    break;
+
+                case TYPE_ID:
+                    typeId = argIter.nextIntArg("typeId");
+
+                    break;
+            }
+        }
+
+        if (typeName == null && typeId == null) {
+            throw new IllegalArgumentException("Type to remove is not specified. " +
+                "Please add one of the options: --typeName <type_name> or --typeId <type_ID>");

Review comment:
       type_ID -> type_id

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/MetadataRemoveAcceptedMessage.java
##########
@@ -0,0 +1,96 @@
+/*
+ * 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.cache.binary;
+
+import org.apache.ignite.internal.managers.discovery.DiscoCache;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.managers.discovery.GridDiscoveryManager;
+import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
+import org.apache.ignite.internal.util.typedef.internal.S;
+import org.apache.ignite.lang.IgniteUuid;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Acknowledge message for {@link MetadataRemoveAcceptedMessage}: see its javadoc for detailed description of protocol.

Review comment:
       Link to itself?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442758314



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataRemoveTask.java
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.compute.ComputeJobContext;
+import org.apache.ignite.compute.ComputeJobResult;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.commandline.cache.CheckIndexInlineSizes;
+import org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorMultiNodeTask;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteInClosure;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.plugin.security.SecurityPermission;
+import org.apache.ignite.resources.IgniteInstanceResource;
+import org.apache.ignite.resources.JobContextResource;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task for {@link MetadataRemoveTask} command.
+ */
+@GridInternal
+public class MetadataRemoveTask extends VisorMultiNodeTask<MetadataTypeArgs, MetadataMarshalled, MetadataMarshalled> {
+    /**
+     *

Review comment:
       Unfortunately, IDEA doesn't like empty one-line comments. =(
   Let's make it a one-line comment.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442845384



##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/binary/BinaryMetadataRemoveTest.java
##########
@@ -0,0 +1,308 @@
+/*
+ * 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.cache.binary;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.IgniteInterruptedCheckedException;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.spi.discovery.DiscoverySpiCustomMessage;
+import org.apache.ignite.spi.discovery.DiscoverySpiListener;
+import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class BinaryMetadataRemoveTest extends GridCommonAbstractTest {
+    /** Max retry cont. */
+    private static final int MAX_RETRY_CONT = 10;
+
+    /** */
+    private static final String CACHE_NAME = "cache";
+
+    /** */
+    private GridTestUtils.DiscoveryHook discoveryHook;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        TcpDiscoverySpi discoSpi;
+
+        final GridTestUtils.DiscoveryHook discoveryHook0 = discoveryHook;
+
+        if (discoveryHook0 != null) {
+            discoSpi = new TcpDiscoverySpi() {
+                @Override public void setListener(@Nullable DiscoverySpiListener lsnr) {
+                    if (discoveryHook0 != null)
+                        super.setListener(GridTestUtils.DiscoverySpiListenerWrapper.wrap(lsnr, discoveryHook0));
+                }
+            };
+        }
+        else
+            discoSpi = new TcpDiscoverySpi();
+
+        cfg.setDiscoverySpi(discoSpi);
+
+        cfg.setCacheConfiguration(new CacheConfiguration().setName(CACHE_NAME));
+
+        return cfg;
+    }
+
+    /**
+     *
+     */
+    protected void startCluster() throws Exception {
+        startGrid("srv0");
+        startGrid("srv1");
+        startGrid("srv2");
+        startClientGrid("cli0");
+        startClientGrid("cli1");
+        startClientGrid("cli2");
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTest() throws Exception {
+        super.beforeTest();
+
+        startCluster();
+
+        discoveryHook = null;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids();
+
+        super.afterTest();
+    }
+
+    /**
+     * Tests remove not existent type and checks the exception.
+     */
+    @Test
+    public void testRemoveNotExistentType() {
+        for (Ignite testNode : G.allGrids()) {
+            GridTestUtils.assertThrows(log, () -> {
+                    ((IgniteEx)testNode).context().cacheObjects().removeType(
+                        ((IgniteEx)testNode).context().cacheObjects().typeId("NotExistentType"));
+
+                    return null;
+                },
+                IgniteException.class, "Failed to remove metadata, type not found");
+        }
+    }
+
+    /**
+     * Tests remove type metadata at all nodes (coordinator, server, client).
+     */
+    @Test
+    public void testRemoveTypeOnNodes() throws Exception {
+        List<IgniteEx[]> testNodeSets = new ArrayList<>();
+
+        // Add all servers permutations to tests sets.
+        for (Ignite ign0 : G.allGrids()) {
+            for (Ignite ign1 : G.allGrids()) {
+                for (Ignite ign2 : G.allGrids()) {
+                    IgniteEx ignx0 = (IgniteEx)ign0;
+                    IgniteEx ignx1 = (IgniteEx)ign1;
+                    IgniteEx ignx2 = (IgniteEx)ign2;
+
+                    if (!ignx0.context().clientNode()

Review comment:
       Sure, take a look below.
   We check all combinations of server nodes and add test set with  client nodes manually.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442767258



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java
##########
@@ -1536,6 +1574,41 @@ public void setBinaryMetadataFileStoreDir(@Nullable File binaryMetadataFileStore
         this.binaryMetadataFileStoreDir = binaryMetadataFileStoreDir;
     }
 
+    /** {@inheritDoc} */
+    @Override public void removeType(int typeId) {
+        if (metadataLocCache.computeIfAbsent(typeId,
+                (id) -> {
+                    throw new IgniteException("Failed to remove metadata, type not found: " + id);
+                })
+            .removing())
+            throw new IgniteException("Failed to remove metadata, type is being removed: " + typeId);

Review comment:
       Let's add a comment like this:
   Fail if "typeId" wasn't found otherwise check "removing" flag.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442771586



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataUpdateCommand.java
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.logging.Logger;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataMarshalled;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataUpdateTask;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+/** */
+public class MetadataUpdateCommand
+    extends MetadataAbstractSubCommand<MetadataMarshalled, MetadataMarshalled>
+{
+    /** Output file name. */
+    public static String OPT_IN_FILE_NAME = "--in";

Review comment:
       Can this be final ?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442773068



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/CacheObjectBinaryProcessorImpl.java
##########
@@ -1536,6 +1574,41 @@ public void setBinaryMetadataFileStoreDir(@Nullable File binaryMetadataFileStore
         this.binaryMetadataFileStoreDir = binaryMetadataFileStoreDir;
     }
 
+    /** {@inheritDoc} */
+    @Override public void removeType(int typeId) {
+        if (metadataLocCache.computeIfAbsent(typeId,
+                (id) -> {
+                    throw new IgniteException("Failed to remove metadata, type not found: " + id);
+                })
+            .removing())
+            throw new IgniteException("Failed to remove metadata, type is being removed: " + typeId);

Review comment:
       Simplified




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r445405158



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -1044,8 +1044,12 @@ public static BinaryMetadata mergeMetadata(@Nullable BinaryMetadata oldMeta, Bin
                             "Type '" + oldMeta.typeName() + "' with typeId " + oldMeta.typeId()
                                 + " has a different/incorrect type for field '" + newField.getKey()
                                 + "'. Expected '" + oldFieldTypeName + "' but '" + newFieldTypeName
-                                + "' was provided. Field type's modification is unsupported, clean {root_path}/marshaller " +
-                                "and {root_path}/db/binary_meta directories if the type change is required."
+                                + "' was provided. Field type's modification is unsupported, if the type change is " +

Review comment:
       Fixed




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] korlov42 commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442761288



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryContext.java
##########
@@ -1449,6 +1449,15 @@ public void onUndeploy(ClassLoader ldr) {
         U.clearClassCache(ldr);
     }
 
+    /**
+     * @param typeId Type ID.
+     */
+    public void removeType(int typeId) {
+        synchronized (this) {

Review comment:
       Why not just mark the whole method as synchronized?

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/jdbc/thin/JdbcThinConnection.java
##########
@@ -1333,6 +1333,9 @@ private void onDisconnect(JdbcThinTcpIo cliIo) {
 
             stmts.clear();
         }
+
+        // Clear local metadata cache on disconnect.
+        metaHnd = new JdbcBinaryMetadataHandler();

Review comment:
       It's not enough just to swap links here, because metaHnd mainly used inside a BinaryContext, thus we have to update that link too

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataMarshalled.java
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.io.IOException;
+import java.io.ObjectInput;
+import java.io.ObjectOutput;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.dto.IgniteDataTransferObject;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+
+/**
+ * Represents information about cluster metadata.
+ */
+@GridInternal
+public class MetadataMarshalled extends IgniteDataTransferObject {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** Marshaled metadata. */
+    private byte[] metaMarshalled;
+
+    /** Metadata */

Review comment:
       Missed dot




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] korlov42 commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442824582



##########
File path: modules/indexing/src/test/java/org/apache/ignite/util/GridCommandHandlerMetadataTest.java
##########
@@ -0,0 +1,312 @@
+/*
+ * 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.util;
+
+import java.nio.file.FileSystem;
+import java.nio.file.FileSystems;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Date;
+import java.util.UUID;
+import org.apache.ignite.IgniteBinary;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.Ignition;
+import org.apache.ignite.binary.BinaryObject;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.binary.BinaryType;
+import org.apache.ignite.client.ClientCacheConfiguration;
+import org.apache.ignite.client.IgniteClient;
+import org.apache.ignite.configuration.ClientConfiguration;
+import org.apache.ignite.internal.binary.BinarySchema;
+import org.apache.ignite.internal.binary.BinaryTypeImpl;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_INVALID_ARGUMENTS;
+import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_OK;
+import static org.apache.ignite.internal.commandline.CommandHandler.EXIT_CODE_UNEXPECTED_ERROR;
+import static org.apache.ignite.testframework.GridTestUtils.assertContains;
+
+/**
+ * Checks command line metadata commands.
+ */
+public class GridCommandHandlerMetadataTest extends GridCommandHandlerClusterByClassAbstractTest {
+    /** File system. */
+    public static final FileSystem FS = FileSystems.getDefault();
+
+    /** Types count. */
+    private static final int TYPES_CNT = 10;
+
+    /**
+     * Check the command '--meta list'.
+     * Steps:
+     * - Creates binary types for a test (by BinaryObjectBuilder);
+     * - execute the command '--meta list'.
+     * - Check command output (must contains all created types).
+     */
+    @Test
+    public void testMetadataList() {
+        injectTestSystemOut();
+
+        for (int typeNum = 0; typeNum < TYPES_CNT; ++typeNum) {
+            BinaryObjectBuilder bob = crd.binary().builder("Type_" + typeNum);
+
+            for (int fldNum = 0; fldNum <= typeNum; ++fldNum)
+                bob.setField("fld_" + fldNum, 0);
+
+            bob.build();
+        }
+
+        assertEquals(EXIT_CODE_OK, execute("--meta", "list"));
+
+        String out = testOut.toString();
+
+        for (int typeNum = 0; typeNum < TYPES_CNT; ++typeNum)
+            assertContains(log, out, "typeName=Type_" + typeNum);
+    }
+
+    /**
+     * Check the command '--meta details'.
+     * Steps:
+     * - Creates binary two types for a test (by BinaryObjectBuilder) with several fields and shemas;
+     * - execute the command '--meta details' for the type Type0 by name
+     * - check metadata print.
+     * - execute the command '--meta details' for the type Type0 by type ID on different formats.
+     * - check metadata print.
+     */
+    @Test
+    public void testMetadataDetails() {
+        injectTestSystemOut();
+
+        BinaryObjectBuilder bob0 = crd.binary().builder("TypeName0");
+        bob0.setField("fld0", 0);
+        bob0.build();
+
+        bob0 = crd.binary().builder("TypeName0");
+        bob0.setField("fld1", "0");
+        bob0.build();
+
+        bob0 = crd.binary().builder("TypeName0");
+        bob0.setField("fld0", 1);
+        bob0.setField("fld2", UUID.randomUUID());
+        BinaryObject bo0 = bob0.build();
+
+        BinaryObjectBuilder bob1 = crd.binary().builder("TypeName1");
+        bob1.setField("fld0", 0);
+        bob1.build();
+
+        bob1 = crd.binary().builder("TypeName1");
+        bob1.setField("fld0", 0);
+        bob1.setField("fld1", new Date());
+        bob1.setField("fld2", 0.1);
+        bob1.setField("fld3", new long[]{0, 1, 2, 3});
+
+        BinaryObject bo1 = bob1.build();
+
+        assertEquals(EXIT_CODE_OK, execute("--meta", "details", "--typeName", "TypeName0"));
+        checkTypeDetails(log, testOut.toString(), crd.context().cacheObjects().metadata(bo0.type().typeId()));
+
+        assertEquals(EXIT_CODE_OK, execute("--meta", "details", "--typeId",
+            "0x" + Integer.toHexString(crd.context().cacheObjects().typeId("TypeName1"))));
+        checkTypeDetails(log, testOut.toString(), crd.context().cacheObjects().metadata(bo1.type().typeId()));
+
+        assertEquals(EXIT_CODE_OK, execute("--meta", "details", "--typeId",
+            Integer.toString(crd.context().cacheObjects().typeId("TypeName1"))));
+        checkTypeDetails(log, testOut.toString(), crd.context().cacheObjects().metadata(bo1.type().typeId()));
+    }
+
+    /**
+     * Check the command '--meta remove' and '--meta update' with invalid arguments.
+     * Steps:
+     * - executes 'remove' command without specified type.
+     * - checks error code and command output.
+     * - executes 'remove' command with '--out' option specified as the exist directory
+     *      ('--out' parameter must be a file name).
+     * - checks error code and command output.
+     * - executes 'update' command with '--in' option specified as the exist directory
+     *      ('--in' parameter must be a file name)
+     * - checks error code and command output.
+     */
+    @Test
+    public void testInvalidArguments() {
+        injectTestSystemOut();
+
+        String out;
+
+        assertEquals(EXIT_CODE_INVALID_ARGUMENTS, execute("--meta", "remove"));
+        out = testOut.toString();
+        assertContains(log, out, "Check arguments.");
+        assertContains(log, out, "Type to remove is not specified");
+        assertContains(log, out, "Please add one of the options: --typeName <type_name> or --typeId <type_ID>");
+
+        assertEquals(EXIT_CODE_INVALID_ARGUMENTS, execute("--meta", "remove", "--typeId", "0", "--out", "target"));
+        out = testOut.toString();
+        assertContains(log, out, "Check arguments.");
+        assertContains(log, out, "Cannot write to output file target. " +
+            "Error: java.nio.file.FileSystemException: target: Is a directory");
+
+        assertEquals(EXIT_CODE_INVALID_ARGUMENTS, execute("--meta", "update", "--in", "target"));
+        out = testOut.toString();
+        assertContains(log, out, "Check arguments.");
+        assertContains(log, out, "Cannot read metadata from target");
+    }
+
+    /**
+     * Check the command '--meta remove' and '--meta update'.
+     * Steps:
+     * - creates the type 'Type0'.
+     * - removes the type by cmdline utility (store removed metadata to specified file).
+     * - checks that type removed (try to create Type0 with the same field and different type).
+     * - removes the new type 'Type0' by cmd line utility.
+     * - restores Typo0 from the file
+     * - checks restored type.
+     */
+    @Test
+    public void testRemoveUpdate() throws Exception {
+        injectTestSystemOut();
+
+        Path typeFile = FS.getPath("type0.bin");
+
+        try {
+            createType("Type0", 0);
+
+            // Type of the field cannot be changed.
+            GridTestUtils.assertThrowsAnyCause(log, () -> {
+                createType("Type0", "string");
+
+                return null;
+            }, BinaryObjectException.class, "Wrong value has been set");
+
+            assertEquals(EXIT_CODE_OK, execute("--meta", "remove",
+                "--typeName", "Type0",
+                "--out", typeFile.toString()));
+
+            // New type must be created successfully after remove the type.
+            createType("Type0", "string");
+
+            assertEquals(EXIT_CODE_OK, execute("--meta", "remove", "--typeName", "Type0"));
+
+            // Restore the metadata from file.
+            assertEquals(EXIT_CODE_OK, execute("--meta", "update", "--in", typeFile.toString()));
+
+            // Type of the field cannot be changed.
+            GridTestUtils.assertThrowsAnyCause(log, () -> {
+                createType("Type0", "string");
+
+                return null;
+            }, BinaryObjectException.class, "Wrong value has been set");
+
+            createType("Type0", 1);
+
+            crd.context().cacheObjects().removeType(crd.context().cacheObjects().typeId("Type0"));
+
+            createType("Type0", "string");
+
+            // Restore the metadata from file.
+            assertEquals(EXIT_CODE_UNEXPECTED_ERROR, execute("--meta", "update", "--in", typeFile.toString()));
+
+            String out = testOut.toString();
+
+            assertContains(log, out, "Failed to execute metadata command='update'");
+            assertContains(log, out, "Type 'Type0' with typeId 110843958 has a " +
+                "different/incorrect type for field 'fld'.");
+            assertContains(log, out, "Expected 'String' but 'int' was provided. " +
+                "Field type's modification is unsupported");
+        }
+        finally {
+            if (Files.exists(typeFile))
+                Files.delete(typeFile);
+        }
+    }
+
+    /**
+     * Check the all thin connections are dropped on the command '--meta remove' and '--meta update'.
+     * Steps:
+     * - opens thin client connection.
+     * - creates Type0 on thin client side.
+     * - removes type by cmd line util.
+     * - executes any command on thin client to detect disconnect.
+     * - creates Type0 on thin client side with other type of field (checks the type was removed).
+     */
+    @Test
+    public void testDropThinConnectionsOnRemove() throws Exception {

Review comment:
       Let's add test like this but for JDBC thin client, to get sure that all works fine

##########
File path: modules/core/src/test/java/org/apache/ignite/internal/processors/cache/binary/BinaryMetadataRemoveTest.java
##########
@@ -0,0 +1,308 @@
+/*
+ * 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.cache.binary;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.ignite.Ignite;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.binary.BinaryObjectBuilder;
+import org.apache.ignite.binary.BinaryObjectException;
+import org.apache.ignite.configuration.CacheConfiguration;
+import org.apache.ignite.configuration.IgniteConfiguration;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.IgniteInterruptedCheckedException;
+import org.apache.ignite.internal.managers.discovery.DiscoveryCustomMessage;
+import org.apache.ignite.internal.util.IgniteUtils;
+import org.apache.ignite.internal.util.typedef.G;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.spi.discovery.DiscoverySpiCustomMessage;
+import org.apache.ignite.spi.discovery.DiscoverySpiListener;
+import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi;
+import org.apache.ignite.testframework.GridTestUtils;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.jetbrains.annotations.Nullable;
+import org.junit.Test;
+
+/**
+ *
+ */
+public class BinaryMetadataRemoveTest extends GridCommonAbstractTest {
+    /** Max retry cont. */
+    private static final int MAX_RETRY_CONT = 10;
+
+    /** */
+    private static final String CACHE_NAME = "cache";
+
+    /** */
+    private GridTestUtils.DiscoveryHook discoveryHook;
+
+    /** {@inheritDoc} */
+    @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception {
+        IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName);
+
+        TcpDiscoverySpi discoSpi;
+
+        final GridTestUtils.DiscoveryHook discoveryHook0 = discoveryHook;
+
+        if (discoveryHook0 != null) {
+            discoSpi = new TcpDiscoverySpi() {
+                @Override public void setListener(@Nullable DiscoverySpiListener lsnr) {
+                    if (discoveryHook0 != null)
+                        super.setListener(GridTestUtils.DiscoverySpiListenerWrapper.wrap(lsnr, discoveryHook0));
+                }
+            };
+        }
+        else
+            discoSpi = new TcpDiscoverySpi();
+
+        cfg.setDiscoverySpi(discoSpi);
+
+        cfg.setCacheConfiguration(new CacheConfiguration().setName(CACHE_NAME));
+
+        return cfg;
+    }
+
+    /**
+     *
+     */
+    protected void startCluster() throws Exception {
+        startGrid("srv0");
+        startGrid("srv1");
+        startGrid("srv2");
+        startClientGrid("cli0");
+        startClientGrid("cli1");
+        startClientGrid("cli2");
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void beforeTest() throws Exception {
+        super.beforeTest();
+
+        startCluster();
+
+        discoveryHook = null;
+    }
+
+    /** {@inheritDoc} */
+    @Override protected void afterTest() throws Exception {
+        stopAllGrids();
+
+        super.afterTest();
+    }
+
+    /**
+     * Tests remove not existent type and checks the exception.
+     */
+    @Test
+    public void testRemoveNotExistentType() {
+        for (Ignite testNode : G.allGrids()) {
+            GridTestUtils.assertThrows(log, () -> {
+                    ((IgniteEx)testNode).context().cacheObjects().removeType(
+                        ((IgniteEx)testNode).context().cacheObjects().typeId("NotExistentType"));
+
+                    return null;
+                },
+                IgniteException.class, "Failed to remove metadata, type not found");
+        }
+    }
+
+    /**
+     * Tests remove type metadata at all nodes (coordinator, server, client).
+     */
+    @Test
+    public void testRemoveTypeOnNodes() throws Exception {
+        List<IgniteEx[]> testNodeSets = new ArrayList<>();
+
+        // Add all servers permutations to tests sets.
+        for (Ignite ign0 : G.allGrids()) {
+            for (Ignite ign1 : G.allGrids()) {
+                for (Ignite ign2 : G.allGrids()) {
+                    IgniteEx ignx0 = (IgniteEx)ign0;
+                    IgniteEx ignx1 = (IgniteEx)ign1;
+                    IgniteEx ignx2 = (IgniteEx)ign2;
+
+                    if (!ignx0.context().clientNode()

Review comment:
       Test description says that removal should be verified on client nodes too, but this condition seems exclude client nodes at all




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] korlov42 commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442783259



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/BinaryMetadataHolder.java
##########
@@ -36,17 +36,38 @@
     /** */
     private final int acceptedVer;
 
+    /** */
+    private final transient boolean removing;
+
     /**
      * @param metadata Metadata.
      * @param pendingVer Version of this metadata - how many updates were issued for this type.
      * @param acceptedVer Pending updates count.
      */
     BinaryMetadataHolder(BinaryMetadata metadata, int pendingVer, int acceptedVer) {
+        this(metadata, pendingVer, acceptedVer, false);
+    }
+
+    /**
+     * @param metadata Metadata.
+     * @param pendingVer Pending updates count.
+     * @param acceptedVer Version of this metadata - how many updates were issued for this type.
+     */
+    private BinaryMetadataHolder(BinaryMetadata metadata, int pendingVer, int acceptedVer, boolean removing) {
         assert metadata != null;
 
         this.metadata = metadata;
         this.pendingVer = pendingVer;
         this.acceptedVer = acceptedVer;
+        this.removing = removing;
+

Review comment:
       empty line

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/MetadataCommand.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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.commandline.meta;
+
+import java.util.logging.Logger;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.commandline.Command;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.subcommands.MetadataRemoveCommand;
+import org.apache.ignite.internal.commandline.meta.subcommands.MetadataUpdateCommand;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataTypeArgs;
+
+import static org.apache.ignite.IgniteSystemProperties.IGNITE_ENABLE_EXPERIMENTAL_COMMAND;
+import static org.apache.ignite.internal.commandline.Command.usage;
+import static org.apache.ignite.internal.commandline.CommandHandler.UTILITY_NAME;
+import static org.apache.ignite.internal.commandline.CommandList.METADATA;
+import static org.apache.ignite.internal.commandline.CommandLogger.optional;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.DETAILS;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.HELP;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.LIST;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.REMOVE;
+import static org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList.UPDATE;
+
+/**
+ *
+ */
+public class MetadataCommand implements Command<Object> {
+    /**
+     *
+     */
+    private Command<?> delegate;
+
+    /** {@inheritDoc} */
+    @Override public void printUsage(Logger log) {
+        if (!experimentalEnabled())
+            return;
+
+        usage(log, "Print metadata command help:",
+            METADATA,
+            HELP.toString()
+        );
+
+        usage(log, "Print list of binary metadata types:",
+            METADATA,
+            LIST.toString()
+        );
+
+        usage(log, "Print detailed info about specified binary type " +
+                "(the type must be specified by type name or by type identifier):",
+            METADATA,
+            DETAILS.toString(),
+            optional(MetadataTypeArgs.OPT_TYPE_ID, "<typeId>"),
+            optional(MetadataTypeArgs.OPT_TYPE_NAME, "<typeName>")
+        );
+
+        usage(log, "Remove the metadata of the specified type " +
+                "(the type must be specified by type name or by type identifier) from cluster and saves the removed " +
+                "metadata to the specified file. \n" +
+                "If the file name isn't specified the output file name is: '<typeId>.bin'",
+            METADATA,
+            REMOVE.toString(),
+            optional(MetadataTypeArgs.OPT_TYPE_ID, "<typeId>"),
+            optional(MetadataTypeArgs.OPT_TYPE_NAME, "<typeName>"),
+            optional(MetadataRemoveCommand.OPT_OUT_FILE_NAME, "<fileName>")
+        );
+
+        usage(log, "Update cluster metadata from specified file (file name is required)",
+            METADATA,
+            UPDATE.toString(),
+            MetadataUpdateCommand.OPT_IN_FILE_NAME, "<fileName>"

Review comment:
       since file name is not optional, let's remove `OPT_` prefix

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataUpdateTask.java
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.compute.ComputeJobResult;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.commandline.cache.CheckIndexInlineSizes;
+import org.apache.ignite.internal.commandline.meta.subcommands.MetadataUpdateCommand;
+import org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorMultiNodeTask;
+import org.apache.ignite.plugin.security.SecurityPermission;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task for {@link MetadataUpdateCommand} command.
+ */
+@GridInternal
+public class MetadataUpdateTask extends VisorMultiNodeTask<MetadataMarshalled, MetadataMarshalled, MetadataMarshalled> {
+    /** */
+    private static final long serialVersionUID = 0L;
+
+    /** {@inheritDoc} */
+    @Override protected VisorJob<MetadataMarshalled, MetadataMarshalled> job(MetadataMarshalled arg) {
+        return new MetadataUpdateJob(arg, debug);
+    }
+
+    /** {@inheritDoc} */
+    @Nullable @Override protected MetadataMarshalled reduce0(List<ComputeJobResult> results) {
+        if (results.isEmpty())

Review comment:
       Let's merge both validation with message `"Invalid job results. Expected exactly 1 result, but was " + results`

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/BinaryMetadataHolder.java
##########
@@ -36,17 +36,38 @@
     /** */
     private final int acceptedVer;
 
+    /** */
+    private final transient boolean removing;
+
     /**
      * @param metadata Metadata.
      * @param pendingVer Version of this metadata - how many updates were issued for this type.
      * @param acceptedVer Pending updates count.
      */
     BinaryMetadataHolder(BinaryMetadata metadata, int pendingVer, int acceptedVer) {
+        this(metadata, pendingVer, acceptedVer, false);
+    }
+
+    /**
+     * @param metadata Metadata.
+     * @param pendingVer Pending updates count.
+     * @param acceptedVer Version of this metadata - how many updates were issued for this type.

Review comment:
       missed param `removing`

##########
File path: modules/core/src/main/java/org/apache/ignite/internal/jdbc/thin/JdbcThinConnection.java
##########
@@ -1333,6 +1333,9 @@ private void onDisconnect(JdbcThinTcpIo cliIo) {
 
             stmts.clear();
         }
+
+        // Clear local metadata cache on disconnect.
+        metaHnd = new JdbcBinaryMetadataHandler();

Review comment:
       Well, can't imagine what could goes wrong, so let's do so 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442752907



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataUpdateCommand.java
##########
@@ -0,0 +1,130 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.logging.Logger;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataMarshalled;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataUpdateTask;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+/** */
+public class MetadataUpdateCommand
+    extends MetadataAbstractSubCommand<MetadataMarshalled, MetadataMarshalled>
+{
+    /** Output file name. */
+    public static String OPT_IN_FILE_NAME = "--in";
+
+    /** {@inheritDoc} */
+    @Override protected String taskName() {
+        return MetadataUpdateTask.class.getName();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt() {
+        return "Warning: the command will update the binary metadata at the cluster.";
+    }
+
+    /** {@inheritDoc} */
+    @Override public MetadataMarshalled parseArguments0(CommandArgIterator argIter) {
+        String opt = argIter.nextArg("--in");
+
+        if (!OPT_IN_FILE_NAME.equalsIgnoreCase(opt))
+            throw new IllegalArgumentException("");
+
+        Path inFile = FS.getPath(argIter.nextArg("input file name"));
+
+        try (InputStream is = Files.newInputStream(inFile)) {
+            ByteArrayOutputStream buffer = new ByteArrayOutputStream();
+            int nRead;
+
+            byte[] data = new byte[1024];
+
+            while ((nRead = is.read(data, 0, data.length)) != -1)

Review comment:
       FYI, we have a method for this: U.copy(in, out)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442756768



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataRemoveTask.java
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.compute.ComputeJobContext;
+import org.apache.ignite.compute.ComputeJobResult;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.commandline.cache.CheckIndexInlineSizes;
+import org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorMultiNodeTask;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteInClosure;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.plugin.security.SecurityPermission;
+import org.apache.ignite.resources.IgniteInstanceResource;
+import org.apache.ignite.resources.JobContextResource;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task for {@link MetadataRemoveTask} command.
+ */
+@GridInternal
+public class MetadataRemoveTask extends VisorMultiNodeTask<MetadataTypeArgs, MetadataMarshalled, MetadataMarshalled> {
+    /**
+     *
+     */
+    private static final long serialVersionUID = 0L;
+
+    /** {@inheritDoc} */
+    @Override protected VisorJob<MetadataTypeArgs, MetadataMarshalled> job(MetadataTypeArgs arg) {
+        return new MetadataRemoveJob(arg, debug);
+    }
+
+    /** {@inheritDoc} */
+    @Nullable @Override protected MetadataMarshalled reduce0(List<ComputeJobResult> results) {
+        if (results.get(0).getException() != null)
+            throw results.get(0).getException();
+        else
+            return results.get(0).getData();
+    }
+
+    /**
+     * Job for {@link CheckIndexInlineSizes} command.
+     */
+    private static class MetadataRemoveJob extends VisorJob<MetadataTypeArgs, MetadataMarshalled> {
+        /** */
+        private static final long serialVersionUID = 0L;
+
+        /** Auto-inject job context. */
+        @JobContextResource
+        private transient ComputeJobContext jobCtx;
+
+        /** Metadata future. */
+        private transient IgniteFuture<Void> future;
+
+        private transient MetadataMarshalled res;
+
+        /**
+         * @param arg Argument.
+         * @param debug Debug.
+         */
+        protected MetadataRemoveJob(@Nullable MetadataTypeArgs arg, boolean debug) {
+            super(arg, debug);
+        }
+
+        /** {@inheritDoc} */
+        @Override protected MetadataMarshalled run(@Nullable MetadataTypeArgs arg) throws IgniteException {
+            try {
+                if (future == null) {
+                    ignite.context().security().authorize(null, SecurityPermission.ADMIN_METADATA_OPS);
+
+                    assert Objects.nonNull(arg);
+
+                    int typeId = arg.typeId(ignite.context());
+
+                    BinaryMetadata meta = ((CacheObjectBinaryProcessorImpl)ignite.context().cacheObjects())
+                        .binaryMetadata(typeId);
+
+                    byte[] marshalled = U.marshal(ignite.context(), meta);
+
+                    res = new MetadataMarshalled(marshalled, meta);
+
+                    ignite.context().cacheObjects().removeType(typeId);
+
+                    future = ignite.compute().broadcastAsync(new DropAllThinSessionsJob());;// Drop all connection
+
+                    jobCtx.holdcc();
+
+                    future.listen(new IgniteInClosure<IgniteFuture<Void>>() {
+                        @Override public void apply(IgniteFuture<Void> f) {
+                            if (f.isDone())
+                                jobCtx.callcc();
+                        }
+                    });
+
+                    return null;
+                }
+
+                return res;
+            }
+            catch (IgniteCheckedException e) {
+                throw new IgniteException(e);
+            }
+        }
+    }
+
+    /**
+     * Job to drop all thin session.
+     */
+    @GridInternal
+    private static class DropAllThinSessionsJob implements IgniteRunnable {
+        /** */
+        private static final long serialVersionUID = 0L;
+
+        /** Grid */
+        @IgniteInstanceResource
+        private IgniteEx ignite;
+
+        /** {@inheritDoc} */
+        @Override public void run()
+            throws IgniteException {

Review comment:
       I believe method declaration length is less than 80 chars and can be one-liner.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442776770



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/jdbc/thin/JdbcThinConnection.java
##########
@@ -1333,6 +1333,9 @@ private void onDisconnect(JdbcThinTcpIo cliIo) {
 
             stmts.clear();
         }
+
+        // Clear local metadata cache on disconnect.
+        metaHnd = new JdbcBinaryMetadataHandler();

Review comment:
       What dou you think about re-initialize binary context here?
   e.g.:
   
   ```
           // Clear local metadata cache on disconnect.
           metaHnd = new JdbcBinaryMetadataHandler();
           ctx = createBinaryCtx(metaHnd, marshCtx);
   
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442756109



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataRemoveTask.java
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.compute.ComputeJobContext;
+import org.apache.ignite.compute.ComputeJobResult;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.commandline.cache.CheckIndexInlineSizes;
+import org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorMultiNodeTask;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteInClosure;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.plugin.security.SecurityPermission;
+import org.apache.ignite.resources.IgniteInstanceResource;
+import org.apache.ignite.resources.JobContextResource;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task for {@link MetadataRemoveTask} command.
+ */
+@GridInternal
+public class MetadataRemoveTask extends VisorMultiNodeTask<MetadataTypeArgs, MetadataMarshalled, MetadataMarshalled> {
+    /**
+     *
+     */
+    private static final long serialVersionUID = 0L;
+
+    /** {@inheritDoc} */
+    @Override protected VisorJob<MetadataTypeArgs, MetadataMarshalled> job(MetadataTypeArgs arg) {
+        return new MetadataRemoveJob(arg, debug);
+    }
+
+    /** {@inheritDoc} */
+    @Nullable @Override protected MetadataMarshalled reduce0(List<ComputeJobResult> results) {
+        if (results.get(0).getException() != null)
+            throw results.get(0).getException();
+        else
+            return results.get(0).getData();
+    }
+
+    /**
+     * Job for {@link CheckIndexInlineSizes} command.
+     */
+    private static class MetadataRemoveJob extends VisorJob<MetadataTypeArgs, MetadataMarshalled> {
+        /** */
+        private static final long serialVersionUID = 0L;
+
+        /** Auto-inject job context. */
+        @JobContextResource
+        private transient ComputeJobContext jobCtx;
+
+        /** Metadata future. */
+        private transient IgniteFuture<Void> future;
+
+        private transient MetadataMarshalled res;
+
+        /**
+         * @param arg Argument.
+         * @param debug Debug.
+         */
+        protected MetadataRemoveJob(@Nullable MetadataTypeArgs arg, boolean debug) {
+            super(arg, debug);
+        }
+
+        /** {@inheritDoc} */
+        @Override protected MetadataMarshalled run(@Nullable MetadataTypeArgs arg) throws IgniteException {
+            try {
+                if (future == null) {
+                    ignite.context().security().authorize(null, SecurityPermission.ADMIN_METADATA_OPS);
+
+                    assert Objects.nonNull(arg);
+
+                    int typeId = arg.typeId(ignite.context());
+
+                    BinaryMetadata meta = ((CacheObjectBinaryProcessorImpl)ignite.context().cacheObjects())
+                        .binaryMetadata(typeId);
+
+                    byte[] marshalled = U.marshal(ignite.context(), meta);
+
+                    res = new MetadataMarshalled(marshalled, meta);
+
+                    ignite.context().cacheObjects().removeType(typeId);
+
+                    future = ignite.compute().broadcastAsync(new DropAllThinSessionsJob());;// Drop all connection

Review comment:
       Double ";"




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442759603



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/cache/binary/BinaryMetadataHolder.java
##########
@@ -70,10 +91,19 @@ int acceptedVersion() {
         return acceptedVer;
     }
 
+    /**
+     *

Review comment:
       Javadoc




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] dmagda commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
dmagda commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r444554144



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/binary/BinaryUtils.java
##########
@@ -1044,8 +1044,12 @@ public static BinaryMetadata mergeMetadata(@Nullable BinaryMetadata oldMeta, Bin
                             "Type '" + oldMeta.typeName() + "' with typeId " + oldMeta.typeId()
                                 + " has a different/incorrect type for field '" + newField.getKey()
                                 + "'. Expected '" + oldFieldTypeName + "' but '" + newFieldTypeName
-                                + "' was provided. Field type's modification is unsupported, clean {root_path}/marshaller " +
-                                "and {root_path}/db/binary_meta directories if the type change is required."
+                                + "' was provided. Field type's modification is unsupported, if the type change is " +

Review comment:
       Taras, how about this version of the message that suggests a workaround? (I think it's enough to suggest the control.sh approach only skipping the option that explains how to clean the metadata manually):
   
   _The type of an existing field can not be changed. Use a different field name or follow this procedure to reuse the current name: 
   * Delete data records that use the old field type.
   * Update the system metadata by running this command: 'control.sh/bat --meta remove --typeId oldMeta.typeId()'_ 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442755970



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/tasks/MetadataRemoveTask.java
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.commandline.meta.tasks;
+
+import java.util.List;
+import java.util.Objects;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.compute.ComputeJobContext;
+import org.apache.ignite.compute.ComputeJobResult;
+import org.apache.ignite.internal.IgniteEx;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.commandline.cache.CheckIndexInlineSizes;
+import org.apache.ignite.internal.processors.cache.binary.CacheObjectBinaryProcessorImpl;
+import org.apache.ignite.internal.processors.task.GridInternal;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorJob;
+import org.apache.ignite.internal.visor.VisorMultiNodeTask;
+import org.apache.ignite.lang.IgniteFuture;
+import org.apache.ignite.lang.IgniteInClosure;
+import org.apache.ignite.lang.IgniteRunnable;
+import org.apache.ignite.plugin.security.SecurityPermission;
+import org.apache.ignite.resources.IgniteInstanceResource;
+import org.apache.ignite.resources.JobContextResource;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Task for {@link MetadataRemoveTask} command.
+ */
+@GridInternal
+public class MetadataRemoveTask extends VisorMultiNodeTask<MetadataTypeArgs, MetadataMarshalled, MetadataMarshalled> {
+    /**
+     *
+     */
+    private static final long serialVersionUID = 0L;
+
+    /** {@inheritDoc} */
+    @Override protected VisorJob<MetadataTypeArgs, MetadataMarshalled> job(MetadataTypeArgs arg) {
+        return new MetadataRemoveJob(arg, debug);
+    }
+
+    /** {@inheritDoc} */
+    @Nullable @Override protected MetadataMarshalled reduce0(List<ComputeJobResult> results) {
+        if (results.get(0).getException() != null)
+            throw results.get(0).getException();
+        else
+            return results.get(0).getData();
+    }
+
+    /**
+     * Job for {@link CheckIndexInlineSizes} command.
+     */
+    private static class MetadataRemoveJob extends VisorJob<MetadataTypeArgs, MetadataMarshalled> {
+        /** */
+        private static final long serialVersionUID = 0L;
+
+        /** Auto-inject job context. */
+        @JobContextResource
+        private transient ComputeJobContext jobCtx;
+
+        /** Metadata future. */
+        private transient IgniteFuture<Void> future;
+
+        private transient MetadataMarshalled res;
+
+        /**
+         * @param arg Argument.
+         * @param debug Debug.
+         */
+        protected MetadataRemoveJob(@Nullable MetadataTypeArgs arg, boolean debug) {
+            super(arg, debug);
+        }
+
+        /** {@inheritDoc} */
+        @Override protected MetadataMarshalled run(@Nullable MetadataTypeArgs arg) throws IgniteException {
+            try {
+                if (future == null) {
+                    ignite.context().security().authorize(null, SecurityPermission.ADMIN_METADATA_OPS);
+
+                    assert Objects.nonNull(arg);
+
+                    int typeId = arg.typeId(ignite.context());
+
+                    BinaryMetadata meta = ((CacheObjectBinaryProcessorImpl)ignite.context().cacheObjects())
+                        .binaryMetadata(typeId);
+
+                    byte[] marshalled = U.marshal(ignite.context(), meta);
+
+                    res = new MetadataMarshalled(marshalled, meta);
+
+                    ignite.context().cacheObjects().removeType(typeId);
+
+                    future = ignite.compute().broadcastAsync(new DropAllThinSessionsJob());;// Drop all connection
+
+                    jobCtx.holdcc();
+
+                    future.listen(new IgniteInClosure<IgniteFuture<Void>>() {
+                        @Override public void apply(IgniteFuture<Void> f) {

Review comment:
       Is lamda possible here or IgniteInClosure is non-functional interface?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #7936: IGNITE-13154 Introduce the ability for a user to manage binary types

Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #7936:
URL: https://github.com/apache/ignite/pull/7936#discussion_r442812634



##########
File path: modules/core/src/main/java/org/apache/ignite/internal/commandline/meta/subcommands/MetadataUpdateCommand.java
##########
@@ -0,0 +1,125 @@
+/*
+ * 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.commandline.meta.subcommands;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.Collection;
+import java.util.logging.Logger;
+import org.apache.ignite.internal.binary.BinaryMetadata;
+import org.apache.ignite.internal.client.GridClient;
+import org.apache.ignite.internal.client.GridClientCompute;
+import org.apache.ignite.internal.client.GridClientConfiguration;
+import org.apache.ignite.internal.client.GridClientDisconnectedException;
+import org.apache.ignite.internal.client.GridClientNode;
+import org.apache.ignite.internal.commandline.CommandArgIterator;
+import org.apache.ignite.internal.commandline.meta.MetadataSubCommandsList;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataMarshalled;
+import org.apache.ignite.internal.commandline.meta.tasks.MetadataUpdateTask;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.internal.visor.VisorTaskArgument;
+
+/** */
+public class MetadataUpdateCommand
+    extends MetadataAbstractSubCommand<MetadataMarshalled, MetadataMarshalled>
+{
+    /** Output file name. */
+    public static final String IN_FILE_NAME = "--in";
+
+    /** {@inheritDoc} */
+    @Override protected String taskName() {
+        return MetadataUpdateTask.class.getName();
+    }
+
+    /** {@inheritDoc} */
+    @Override public String confirmationPrompt() {
+        return "Warning: the command will update the binary metadata at the cluster.";
+    }
+
+    /** {@inheritDoc} */
+    @Override public MetadataMarshalled parseArguments0(CommandArgIterator argIter) {
+        String opt = argIter.nextArg("--in");
+
+        if (!IN_FILE_NAME.equalsIgnoreCase(opt))
+            throw new IllegalArgumentException("");
+
+        Path inFile = FS.getPath(argIter.nextArg("input file name"));
+
+        try (InputStream is = Files.newInputStream(inFile)) {
+            ByteArrayOutputStream buf = new ByteArrayOutputStream();
+
+            U.copy(is, buf);
+
+            return new MetadataMarshalled(buf.toByteArray(), null);
+        }
+        catch (IOException e) {
+            throw new IllegalArgumentException("Cannot read metadata from " + inFile, e);
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override protected MetadataMarshalled execute0(

Review comment:
       Great issue! Thanks a lot. Fixed.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org