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 2021/02/15 11:31:21 UTC
[GitHub] [ignite-3] akalash opened a new pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
akalash opened a new pull request #53:
URL: https://github.com/apache/ignite-3/pull/53
… based on ScaleCube
----------------------------------------------------------------
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-3] sergey-chugunov-1985 commented on pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
Posted by GitBox <gi...@apache.org>.
sergey-chugunov-1985 commented on pull request #53:
URL: https://github.com/apache/ignite-3/pull/53#issuecomment-780472134
I went through the code, it looks good as a starting point from my perspective. Some abstractions like NetworkCluster are questionable and may need clarification but as a whole piece code looks good.
----------------------------------------------------------------
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-3] ygerzhedovich commented on a change in pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #53:
URL: https://github.com/apache/ignite-3/pull/53#discussion_r587221528
##########
File path: modules/network/src/main/java/org/apache/ignite/network/MessageHandlerHolder.java
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.network;
+
+import java.util.Collection;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+/**
+ * Encapsulation of all cluster handlers for centralized management.
+ */
+public class MessageHandlerHolder {
+ /** Handler for processing incoming messages. */
+ private final Collection<NetworkMessageHandler> messageHandlers = new CopyOnWriteArrayList<>();
+
+ /** Handler for processing all cluster events. */
+ private final Collection<NetworkClusterEventHandler> clusterEventHandlers = new CopyOnWriteArrayList<>();
+
+ /**
+ * @param handler Handler for processing incoming messages.
+ */
+ public void addmessageHandlers(NetworkMessageHandler handler) {
+ messageHandlers.add(handler);
+ }
+
+ /**
+ * @param handler Handler for processing all cluster events.
+ */
+ public void addClusterEventHandlers(NetworkClusterEventHandler handler) {
+ clusterEventHandlers.add(handler);
+ }
+
+ /**
+ * @return All handlers for processing incoming messages.
+ */
+ public Collection<NetworkMessageHandler> messageHandlers() {
Review comment:
should we return unmodifiable collection here?
----------------------------------------------------------------
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-3] ibessonov commented on a change in pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
Posted by GitBox <gi...@apache.org>.
ibessonov commented on a change in pull request #53:
URL: https://github.com/apache/ignite-3/pull/53#discussion_r576140523
##########
File path: modules/network/src/main/java/org/apache/ignite/network/NetworkCluster.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.network;
+
+import java.util.Collection;
+import java.util.concurrent.Future;
+
+/**
+ * Main interface for interaction with network. It allows to get information about network members and send messages to
+ * them.
+ */
+public interface NetworkCluster {
+ /**
+ * Stop the processing of network connection immediately. Sending and receiving messages or obtaining network
+ * members information after this method successfully finished will be impossible.
+ *
+ * @throws Exception If something went wrong.
+ */
+ void shutdown() throws Exception;
+
+ /**
+ * @return Information about local network member.
+ */
+ NetworkMember localMember();
+
+ /**
+ * @return Information about all members which have seen by the local member(including local member itself).
+ */
+ Collection<NetworkMember> allMembers();
+
+ /**
+ * Try to send the message asynchronously to the specific member without any guarantees that this message would be
+ * delivered.
+ *
+ * @param member Netwrok member which should receive the message.
+ * @param msg Message which should be delivered.
+ */
+ void weakSend(NetworkMember member, Object msg);
+
+ /**
+ * Try to send the message asynchronously to the specific member with next guarantees:
+ * * Messages which was sent from one thread to one member will be delivered in the same order as they were sent.
Review comment:
Exra "*"
##########
File path: modules/network/pom.xml
##########
@@ -0,0 +1,99 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<!--
+ POM file.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-parent</artifactId>
+ <version>1</version>
+ <relativePath>../../parent/pom.xml</relativePath>
+ </parent>
+
+ <artifactId>ignite-network</artifactId>
+ <version>3.0.0-SNAPSHOT</version>
+
+ <dependencies>
+ <!-- Internal module dependencies. -->
+ <dependency>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-configuration</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <!-- 3-rd party dependencies. -->
+ <dependency>
+ <groupId>io.scalecube</groupId>
+ <artifactId>scalecube-cluster</artifactId>
+ </dependency>
+
+ <!-- Test dependencies. -->
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.hamcrest</groupId>
+ <artifactId>hamcrest-library</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-configuration-annotation-processor</artifactId>
+ <version>${project.version}</version>
+ <scope>compile</scope>
+ </dependency>
+ </dependencies>
+ <build>
+ <resources>
+ <resource>
+ <directory>src/main/resources</directory>
+ <filtering>true</filtering>
+ </resource>
+ </resources>
+
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
Review comment:
Version should be removed.
##########
File path: modules/network/src/main/java/org/apache/ignite/network/NetworkCluster.java
##########
@@ -0,0 +1,70 @@
+/*
+ * 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.network;
+
+import java.util.Collection;
+import java.util.concurrent.Future;
+
+/**
+ * Main interface for interaction with network. It allows to get information about network members and send messages to
+ * them.
+ */
+public interface NetworkCluster {
+ /**
+ * Stop the processing of network connection immediately. Sending and receiving messages or obtaining network
+ * members information after this method successfully finished will be impossible.
+ *
+ * @throws Exception If something went wrong.
+ */
+ void shutdown() throws Exception;
+
+ /**
+ * @return Information about local network member.
+ */
+ NetworkMember localMember();
+
+ /**
+ * @return Information about all members which have seen by the local member(including local member itself).
+ */
+ Collection<NetworkMember> allMembers();
+
+ /**
+ * Try to send the message asynchronously to the specific member without any guarantees that this message would be
+ * delivered.
+ *
+ * @param member Netwrok member which should receive the message.
Review comment:
Typo
----------------------------------------------------------------
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-3] ibessonov commented on pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
Posted by GitBox <gi...@apache.org>.
ibessonov commented on pull request #53:
URL: https://github.com/apache/ignite-3/pull/53#issuecomment-790624948
Code is already merged. All the comments will be addresses in other PRs.
----------------------------------------------------------------
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-3] ygerzhedovich commented on a change in pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
Posted by GitBox <gi...@apache.org>.
ygerzhedovich commented on a change in pull request #53:
URL: https://github.com/apache/ignite-3/pull/53#discussion_r587222377
##########
File path: modules/network/src/main/java/org/apache/ignite/network/MessageHandlerHolder.java
##########
@@ -0,0 +1,59 @@
+/*
+ * 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.network;
+
+import java.util.Collection;
+import java.util.concurrent.CopyOnWriteArrayList;
+
+/**
+ * Encapsulation of all cluster handlers for centralized management.
+ */
+public class MessageHandlerHolder {
+ /** Handler for processing incoming messages. */
+ private final Collection<NetworkMessageHandler> messageHandlers = new CopyOnWriteArrayList<>();
+
+ /** Handler for processing all cluster events. */
+ private final Collection<NetworkClusterEventHandler> clusterEventHandlers = new CopyOnWriteArrayList<>();
+
+ /**
+ * @param handler Handler for processing incoming messages.
+ */
+ public void addmessageHandlers(NetworkMessageHandler handler) {
+ messageHandlers.add(handler);
+ }
+
+ /**
+ * @param handler Handler for processing all cluster events.
+ */
+ public void addClusterEventHandlers(NetworkClusterEventHandler handler) {
Review comment:
we have 'add' method, but don't have 'remove'
----------------------------------------------------------------
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-3] SammyVimes commented on a change in pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
Posted by GitBox <gi...@apache.org>.
SammyVimes commented on a change in pull request #53:
URL: https://github.com/apache/ignite-3/pull/53#discussion_r576629261
##########
File path: modules/network/pom.xml
##########
@@ -0,0 +1,99 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+ 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.
+-->
+
+<!--
+ POM file.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+
+ <parent>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-parent</artifactId>
+ <version>1</version>
+ <relativePath>../../parent/pom.xml</relativePath>
+ </parent>
+
+ <artifactId>ignite-network</artifactId>
+ <version>3.0.0-SNAPSHOT</version>
+
+ <dependencies>
+ <!-- Internal module dependencies. -->
+ <dependency>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-configuration</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+
+ <!-- 3-rd party dependencies. -->
+ <dependency>
+ <groupId>io.scalecube</groupId>
+ <artifactId>scalecube-cluster</artifactId>
+ </dependency>
+
+ <!-- Test dependencies. -->
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.hamcrest</groupId>
+ <artifactId>hamcrest-library</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.apache.ignite</groupId>
+ <artifactId>ignite-configuration-annotation-processor</artifactId>
+ <version>${project.version}</version>
+ <scope>compile</scope>
+ </dependency>
+ </dependencies>
+ <build>
+ <resources>
+ <resource>
+ <directory>src/main/resources</directory>
+ <filtering>true</filtering>
+ </resource>
+ </resources>
+
+ <plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.8.1</version>
Review comment:
Compiler plugin is defined and configured in parent pom
----------------------------------------------------------------
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-3] ibessonov closed pull request #53: IGNITE-14110 Module ignite-network created with simple implementation…
Posted by GitBox <gi...@apache.org>.
ibessonov closed pull request #53:
URL: https://github.com/apache/ignite-3/pull/53
----------------------------------------------------------------
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