ibessonov commented on a change in pull request #229:
URL: https://github.com/apache/ignite-3/pull/229#discussion_r672996990
##########
File path:
modules/network-api/src/main/java/org/apache/ignite/network/AbstractMessagingService.java
##########
@@ -17,26 +17,56 @@
package org.apache.ignite.network;
+import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReferenceArray;
+import org.apache.ignite.network.annotations.MessageGroup;
/**
* Base class for {@link MessagingService} implementations.
*/
public abstract class AbstractMessagingService implements MessagingService {
- /** */
- private final Collection<NetworkMessageHandler> messageHandlers = new
CopyOnWriteArrayList<>();
+ /** Mapping from group type (array index) to a list of registered message
handlers. */
+ private final AtomicReferenceArray<List<NetworkMessageHandler>>
handlersByGroupType =
+ new AtomicReferenceArray<>(Short.MAX_VALUE + 1);
/** {@inheritDoc} */
- @Override public void addMessageHandler(NetworkMessageHandler handler) {
- messageHandlers.add(handler);
+ @Override public void addMessageHandler(Class<?> messageGroup,
NetworkMessageHandler handler) {
+ handlersByGroupType.getAndUpdate(getMessageGroupType(messageGroup),
handlers -> {
+ if (handlers == null)
+ return List.of(handler);
+
+ var result = new ArrayList<NetworkMessageHandler>(handlers.size()
+ 1);
+
+ result.addAll(handlers);
+ result.add(handler);
+
+ return result;
+ });
+ }
+
+ /**
+ * Extracts the message group ID from a class annotated with {@link
MessageGroup}.
+ */
+ private static short getMessageGroupType(Class<?> messageGroup) {
Review comment:
Parameter javadoc is missing
##########
File path:
modules/network-api/src/main/java/org/apache/ignite/network/AbstractMessagingService.java
##########
@@ -17,26 +17,56 @@
package org.apache.ignite.network;
+import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReferenceArray;
+import org.apache.ignite.network.annotations.MessageGroup;
/**
* Base class for {@link MessagingService} implementations.
*/
public abstract class AbstractMessagingService implements MessagingService {
- /** */
- private final Collection<NetworkMessageHandler> messageHandlers = new
CopyOnWriteArrayList<>();
+ /** Mapping from group type (array index) to a list of registered message
handlers. */
+ private final AtomicReferenceArray<List<NetworkMessageHandler>>
handlersByGroupType =
+ new AtomicReferenceArray<>(Short.MAX_VALUE + 1);
/** {@inheritDoc} */
- @Override public void addMessageHandler(NetworkMessageHandler handler) {
- messageHandlers.add(handler);
+ @Override public void addMessageHandler(Class<?> messageGroup,
NetworkMessageHandler handler) {
+ handlersByGroupType.getAndUpdate(getMessageGroupType(messageGroup),
handlers -> {
+ if (handlers == null)
+ return List.of(handler);
+
+ var result = new ArrayList<NetworkMessageHandler>(handlers.size()
+ 1);
+
+ result.addAll(handlers);
+ result.add(handler);
+
+ return result;
+ });
+ }
+
+ /**
+ * Extracts the message group ID from a class annotated with {@link
MessageGroup}.
+ */
+ private static short getMessageGroupType(Class<?> messageGroup) {
+ MessageGroup annotation =
messageGroup.getAnnotation(MessageGroup.class);
+
+ assert annotation != null : "No MessageGroup annotation present";
Review comment:
Please add "messageGroup" class itself as assertion message
##########
File path:
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
##########
@@ -174,7 +174,7 @@ public MetaStorageManager(
// );
// TODO: IGNITE-14414 Cluster initialization flow. Here we should
complete metaStorageServiceFuture.
- clusterNetSvc.messagingService().addMessageHandler((message,
senderAddr, correlationId) -> {});
+// clusterNetSvc.messagingService().addMessageHandler((message,
senderAddr, correlationId) -> {});
Review comment:
What is this exactly? Can we simply remove this line?
##########
File path:
modules/network-api/src/main/java/org/apache/ignite/network/AbstractMessagingService.java
##########
@@ -17,26 +17,56 @@
package org.apache.ignite.network;
+import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReferenceArray;
+import org.apache.ignite.network.annotations.MessageGroup;
/**
* Base class for {@link MessagingService} implementations.
*/
public abstract class AbstractMessagingService implements MessagingService {
- /** */
- private final Collection<NetworkMessageHandler> messageHandlers = new
CopyOnWriteArrayList<>();
+ /** Mapping from group type (array index) to a list of registered message
handlers. */
+ private final AtomicReferenceArray<List<NetworkMessageHandler>>
handlersByGroupType =
+ new AtomicReferenceArray<>(Short.MAX_VALUE + 1);
/** {@inheritDoc} */
- @Override public void addMessageHandler(NetworkMessageHandler handler) {
- messageHandlers.add(handler);
+ @Override public void addMessageHandler(Class<?> messageGroup,
NetworkMessageHandler handler) {
+ handlersByGroupType.getAndUpdate(getMessageGroupType(messageGroup),
handlers -> {
+ if (handlers == null)
+ return List.of(handler);
+
+ var result = new ArrayList<NetworkMessageHandler>(handlers.size()
+ 1);
+
+ result.addAll(handlers);
+ result.add(handler);
+
+ return result;
+ });
+ }
+
+ /**
+ * Extracts the message group ID from a class annotated with {@link
MessageGroup}.
+ */
+ private static short getMessageGroupType(Class<?> messageGroup) {
+ MessageGroup annotation =
messageGroup.getAnnotation(MessageGroup.class);
+
+ assert annotation != null : "No MessageGroup annotation present";
+
+ short groupType = annotation.groupType();
+
+ assert groupType >= 0 : "Group type must not be negative";
+
+ return groupType;
}
/**
* @return registered message handlers.
*/
- public Collection<NetworkMessageHandler> getMessageHandlers() {
- return Collections.unmodifiableCollection(messageHandlers);
+ protected Collection<NetworkMessageHandler> getMessageHandlers(short
groupType) {
+ List<NetworkMessageHandler> result =
handlersByGroupType.get(groupType);
Review comment:
I'd add assertion that "groupType >= 0" here as well just in case.
Method should be final btw
##########
File path:
modules/network-api/src/main/java/org/apache/ignite/network/AbstractMessagingService.java
##########
@@ -17,26 +17,56 @@
package org.apache.ignite.network;
+import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReferenceArray;
+import org.apache.ignite.network.annotations.MessageGroup;
/**
* Base class for {@link MessagingService} implementations.
*/
public abstract class AbstractMessagingService implements MessagingService {
- /** */
- private final Collection<NetworkMessageHandler> messageHandlers = new
CopyOnWriteArrayList<>();
+ /** Mapping from group type (array index) to a list of registered message
handlers. */
+ private final AtomicReferenceArray<List<NetworkMessageHandler>>
handlersByGroupType =
+ new AtomicReferenceArray<>(Short.MAX_VALUE + 1);
/** {@inheritDoc} */
- @Override public void addMessageHandler(NetworkMessageHandler handler) {
- messageHandlers.add(handler);
+ @Override public void addMessageHandler(Class<?> messageGroup,
NetworkMessageHandler handler) {
+ handlersByGroupType.getAndUpdate(getMessageGroupType(messageGroup),
handlers -> {
+ if (handlers == null)
+ return List.of(handler);
+
+ var result = new ArrayList<NetworkMessageHandler>(handlers.size()
+ 1);
+
+ result.addAll(handlers);
+ result.add(handler);
+
+ return result;
+ });
+ }
+
+ /**
+ * Extracts the message group ID from a class annotated with {@link
MessageGroup}.
+ */
+ private static short getMessageGroupType(Class<?> messageGroup) {
Review comment:
You're right
##########
File path:
modules/metastorage/src/main/java/org/apache/ignite/internal/metastorage/MetaStorageManager.java
##########
@@ -174,7 +174,7 @@ public MetaStorageManager(
// );
// TODO: IGNITE-14414 Cluster initialization flow. Here we should
complete metaStorageServiceFuture.
- clusterNetSvc.messagingService().addMessageHandler((message,
senderAddr, correlationId) -> {});
+// clusterNetSvc.messagingService().addMessageHandler((message,
senderAddr, correlationId) -> {});
Review comment:
Ok
--
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.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
