You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "Vladsz83 (via GitHub)" <gi...@apache.org> on 2026/06/15 17:10:25 UTC
[PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Vladsz83 opened a new pull request, #13243:
URL: https://github.com/apache/ignite/pull/13243
(no 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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3455768197
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * A manager of {@link OperationContextAttribute} which are required to be propagated through a cluster.
+ * Has own attributes ids compared to a local node's {@link OperationContext}.
+ *
+ * @see OperationContext
+ * @see DistributedOperationContextMessage
+ */
+public class DistributedOperationContextManager {
+ /** */
+ private static final DistributedOperationContextManager INSTANCE = new DistributedOperationContextManager();
+
+ /** Maximal number of supported distributed attributes. */
+ static final byte MAX_DISTRIBUTED_ATTR_CNT = Byte.SIZE;
+
+ /** Registered distributed attributes by their cluster-wide id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentSkipListMap<>();
+
+ /** */
+ public static DistributedOperationContextManager instance() {
+ return INSTANCE;
+ }
+
+ /**
+ * Creates and registers a distributable {@link OperationContextAttribute}.
+ *
+ * @param id Cluster-wide id of a distributed operation context attribute.
+ * @param initVal The attribute's unitial value.
+ */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttribute(byte id, @Nullable T initVal) {
+ assert id >= 0 && id < MAX_DISTRIBUTED_ATTR_CNT : "Invalid distributed attributed id [id=" + id + ']';
+
+ return (OperationContextAttribute<T>)attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id [id=" + id + ']');
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+ }
+
+ /**
+ * Requests current {@link OperationContext} for its effective attributes and collects ones which are also registered
+ * as distbibued attributes.
+ *
+ * @return A message to send current effective distributed attributes. {@code null}, if there are no
+ * effective attributes in {@link OperationContext} or none of them is a distributed attribute.
+ */
+ public @Nullable DistributedOperationContextMessage collectDistributedAttributes() {
+ DistributedOperationContextMessage res = null;
+ List<Message> vals = null;
+
+ for (Map.Entry<Byte, OperationContextAttribute<? extends Message>> e : attrs.entrySet()) {
+ OperationContextAttribute<? extends Message> attr = e.getValue();
+
+ Message curVal = OperationContext.get(attr);
+
+ if (curVal != attr.initialValue()) {
+ if (res == null) {
+ res = new DistributedOperationContextMessage();
+
+ vals = new ArrayList<>(MAX_DISTRIBUTED_ATTR_CNT / 2);
+ }
+
+ byte mask = (byte)(1 << e.getKey());
+
+ assert (res.idBitmap & mask) == 0;
+
+ vals.add(curVal);
+ res.idBitmap |= mask;
+ }
+ }
+
+ if (res != null)
+ res.vals = vals.toArray(vals.toArray(new Message[vals.size()]));
+
+ return res;
+ }
+
+ /** Sets the received distributed operation context attributes (if any) into current {@link OperationContext}. */
Review Comment:
```
/** Restores distributed {@link OperationContextAttribute} values received from a remote node. */
```
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3457315177
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ClientImpl.java:
##########
@@ -1757,276 +1761,290 @@ private MessageWorker(IgniteLogger log) {
blockingSectionEnd();
}
- if (msg instanceof JoinTimeout) {
- int joinCnt0 = ((JoinTimeout)msg).joinCnt;
-
- if (joinCnt == joinCnt0) {
- if (state == STARTING) {
- joinError(new IgniteSpiException("Join process timed out, did not receive response for " +
- "join request (consider increasing 'joinTimeout' configuration property) " +
- "[joinTimeout=" + spi.joinTimeout + ", sock=" + currSock + ']'));
-
+ if (msg instanceof TcpDiscoveryAbstractMessage msg0 && msg0.opCtxMsg != null) {
Review Comment:
Obviously, that's not what I meant. Let's wrap the code related to processing of the message from the moment it is polled from the queue, but not move anything to other new methods.
In the current state of the code, moving the message handling logic into a separate method seems awkward. It's better to refactor this later in a separate ticket.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441562036
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
Review Comment:
I suggest a sorted map. I don't like structures like List<T2<Byte, OperationContextAttribute<? extends Message>>>
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441768229
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextAttributeManager.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationContextAttributeManager {
+ /** */
+ static final byte MAX_DISTRIBUTED_ATTR_CNT = 7;
Review Comment:
7 -> 8 ?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452846530
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationContextManager {
+ /** */
+ static final byte MAX_DISTRIBUTED_ATTR_CNT = Byte.SIZE;
+
+ /** */
+ private static final DistributedOperationContextManager INSTANCE = new DistributedOperationContextManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentSkipListMap<>();
+
+ /** */
+ public static DistributedOperationContextManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttribute(byte id, @Nullable T initVal) {
+ assert id >= 0 && id < MAX_DISTRIBUTED_ATTR_CNT : "Invalid distributed attributed id [id=" + id + "].";
+
+ if (attrs.size() == MAX_DISTRIBUTED_ATTR_CNT)
+ throw new IgniteException("Maximum number of distributed attributes is exceeded [max=" + MAX_DISTRIBUTED_ATTR_CNT + "].");
Review Comment:
Let's remove dot from the end of the error message.
It seems we can safely remove this error because in this case `Duplicated distributed attribute id` will be raised.
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationContextManager {
+ /** */
+ static final byte MAX_DISTRIBUTED_ATTR_CNT = Byte.SIZE;
+
+ /** */
+ private static final DistributedOperationContextManager INSTANCE = new DistributedOperationContextManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentSkipListMap<>();
+
+ /** */
+ public static DistributedOperationContextManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttribute(byte id, @Nullable T initVal) {
+ assert id >= 0 && id < MAX_DISTRIBUTED_ATTR_CNT : "Invalid distributed attributed id [id=" + id + "].";
+
+ if (attrs.size() == MAX_DISTRIBUTED_ATTR_CNT)
+ throw new IgniteException("Maximum number of distributed attributes is exceeded [max=" + MAX_DISTRIBUTED_ATTR_CNT + "].");
+
+ return (OperationContextAttribute<T>)attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id [id=" + id + "].");
Review Comment:
Let's remove dot from the end of the error message.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452383921
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
Review Comment:
It still exists.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3455763632
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * A manager of {@link OperationContextAttribute} which are required to be propagated through a cluster.
+ * Has own attributes ids compared to a local node's {@link OperationContext}.
+ *
+ * @see OperationContext
+ * @see DistributedOperationContextMessage
+ */
+public class DistributedOperationContextManager {
+ /** */
+ private static final DistributedOperationContextManager INSTANCE = new DistributedOperationContextManager();
+
+ /** Maximal number of supported distributed attributes. */
+ static final byte MAX_DISTRIBUTED_ATTR_CNT = Byte.SIZE;
+
+ /** Registered distributed attributes by their cluster-wide id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentSkipListMap<>();
+
+ /** */
+ public static DistributedOperationContextManager instance() {
+ return INSTANCE;
+ }
+
+ /**
Review Comment:
```
/**
* Creates a new {@link OperationContext} attribute with the specified distributed ID and initial value.
*
* <p>The distributed ID is used to consistently identify the attribute across all nodes in the cluster.
* It must be unique, and its value must be in the range from {@code 0} (inclusive) to {@code Byte.SIZE} (exclusive).</p>
*
* <p>The value of the created attribute is automatically captured and propagated between cluster nodes
* during message transmission.</p>
*
* @see OperationContextAttribute#newInstance(Object)
*/
```
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452432513
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ClientImpl.java:
##########
@@ -2001,7 +2005,11 @@ else if (discoMsg instanceof TcpDiscoveryCheckFailedMessage)
}
}
- processDiscoveryMessage((TcpDiscoveryAbstractMessage)msg);
+ TcpDiscoveryAbstractMessage msg0 = (TcpDiscoveryAbstractMessage)msg;
+
+ try (Scope ignored = DistributedOperationContextManager.instance().restoreDistributedAttributes(msg0.opCtxMsg)) {
+ processDiscoveryMessage(msg0);
Review Comment:
It looks like we need to wrap not just the `processDiscoveryMessage` call, but the entire processing of the message from the moment it is polled from the queue.
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distibuted attributes. */
+public class DistributedOperationContextAttributesMessage implements Message {
+ /** Values of operation context attributes. */
+ @Order(0)
+ public List<Message> vals;
Review Comment:
> extra GC load
This doesn't seem to be the case unless the created list goes outside the scope of the method. Even if this is not, short-lived objects are extremely optimized in modern JVMs.
But keeping tons of messages with unused arrays in discovery and communication queues seems to be a more realistic problem.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441629884
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractMessage.java:
##########
@@ -76,6 +77,11 @@ public abstract class TcpDiscoveryAbstractMessage implements Message {
@Order(4)
Set<UUID> failedNodes;
+ /** Operation context attributes message. */
+ @GridToStringInclude
+ @Order(5)
+ public @Nullable DistributedOperationContextAttributesMessage opCtxMsg;
Review Comment:
We do not use getters and setters in the messages
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3458119350
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ClientImpl.java:
##########
@@ -1757,276 +1761,290 @@ private MessageWorker(IgniteLogger log) {
blockingSectionEnd();
}
- if (msg instanceof JoinTimeout) {
- int joinCnt0 = ((JoinTimeout)msg).joinCnt;
-
- if (joinCnt == joinCnt0) {
- if (state == STARTING) {
- joinError(new IgniteSpiException("Join process timed out, did not receive response for " +
- "join request (consider increasing 'joinTimeout' configuration property) " +
- "[joinTimeout=" + spi.joinTimeout + ", sock=" + currSock + ']'));
-
+ if (msg instanceof TcpDiscoveryAbstractMessage msg0 && msg0.opCtxMsg != null) {
Review Comment:
We've already moved message processing in a dedicated methods in same manner at other places. Message is a Object here, not even a Message. I need to check its type first. An `if` is required. That is why I introduce a method or did the processing in the part of code before.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441615464
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id: " + id);
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+
+ return (OperationContextAttribute<T>)attrs.get(id);
+ }
+
+ /** */
+ public @Nullable DistributedOperationContextAttributesMessage collectDistributedAttributes() {
+ DistributedOperationContextAttributesMessage res = null;
+
+ for (Map.Entry<Byte, OperationContextAttribute<? extends Message>> e : attrs.entrySet()) {
+ OperationContextAttribute<? extends Message> attr = e.getValue();
+
+ Message curVal = OperationContext.get(attr);
+
+ assert attr.initialValue() == null || curVal == null || curVal.getClass().isAssignableFrom(attr.initialValue().getClass());
Review Comment:
We check that a value of other type wouldn't set.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441611594
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
Review Comment:
We use instance() everywhere. I don't think that just one call should be without instance()
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3455766492
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * A manager of {@link OperationContextAttribute} which are required to be propagated through a cluster.
+ * Has own attributes ids compared to a local node's {@link OperationContext}.
+ *
+ * @see OperationContext
+ * @see DistributedOperationContextMessage
+ */
+public class DistributedOperationContextManager {
+ /** */
+ private static final DistributedOperationContextManager INSTANCE = new DistributedOperationContextManager();
+
+ /** Maximal number of supported distributed attributes. */
+ static final byte MAX_DISTRIBUTED_ATTR_CNT = Byte.SIZE;
+
+ /** Registered distributed attributes by their cluster-wide id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentSkipListMap<>();
+
+ /** */
+ public static DistributedOperationContextManager instance() {
+ return INSTANCE;
+ }
+
+ /**
+ * Creates and registers a distributable {@link OperationContextAttribute}.
+ *
+ * @param id Cluster-wide id of a distributed operation context attribute.
+ * @param initVal The attribute's unitial value.
+ */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttribute(byte id, @Nullable T initVal) {
+ assert id >= 0 && id < MAX_DISTRIBUTED_ATTR_CNT : "Invalid distributed attributed id [id=" + id + ']';
+
+ return (OperationContextAttribute<T>)attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id [id=" + id + ']');
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+ }
+
+ /**
+ * Requests current {@link OperationContext} for its effective attributes and collects ones which are also registered
Review Comment:
```
/**
* Collects the values of all distributed {@link OperationContextAttribute}s registered by this manager
* in a format suitable for transmission between cluster nodes.
*
* @see OperationContext#get(OperationContextAttribute)
*/
```
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441817503
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
Review Comment:
So we declare that the distributed ID is 0 <= ID < MAX_DISTRIBUTED_ATTR_CNT
And the map ensures that attributes with duplicate identifiers are not created.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3456958107
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ClientImpl.java:
##########
@@ -1757,276 +1761,290 @@ private MessageWorker(IgniteLogger log) {
blockingSectionEnd();
}
- if (msg instanceof JoinTimeout) {
- int joinCnt0 = ((JoinTimeout)msg).joinCnt;
-
- if (joinCnt == joinCnt0) {
- if (state == STARTING) {
- joinError(new IgniteSpiException("Join process timed out, did not receive response for " +
- "join request (consider increasing 'joinTimeout' configuration property) " +
- "[joinTimeout=" + spi.joinTimeout + ", sock=" + currSock + ']'));
-
+ if (msg instanceof TcpDiscoveryAbstractMessage msg0 && msg0.opCtxMsg != null) {
Review Comment:
I cannot wrap entire code because there would not be the message yet.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452462497
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distibuted attributes. */
+public class DistributedOperationContextAttributesMessage implements Message {
+ /** Values of operation context attributes. */
+ @Order(0)
+ public List<Message> vals;
Review Comment:
One extra transport byte at kilobyte-like messages vs one new extra data copy, new operator, extra GC load. I'm not sure it is better.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on PR #13243:
URL: https://github.com/apache/ignite/pull/13243#issuecomment-4768979085
It's also worth adding meaningful comments describing the purpose of DistributedOperationContextManager and its methods.
What is DistributedOperationContextMessage and why was it decided to use a bitmap?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441582360
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
Review Comment:
Why?
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441562036
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
Review Comment:
I guggest a sorted map. I donk like structured like List<T2<Byte, OperationContextAttribute<? extends Message>>>
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441651823
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
Review Comment:
Typo.
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
Review Comment:
Sorry, static -> synchronized
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441113614
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
Review Comment:
DistributedOperationAttributeManager -> DistributedOperationContextAttributeManager
##########
modules/commons/src/main/java/org/apache/ignite/internal/thread/context/OperationContextAttribute.java:
##########
@@ -32,7 +32,7 @@ public class OperationContextAttribute<T> {
static final AtomicInteger ID_GEN = new AtomicInteger();
/** */
- static final int MAX_ATTR_CNT = Integer.SIZE;
+ public static final int MAX_ATTR_CNT = Integer.SIZE;
Review Comment:
We can revert this change.
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
Review Comment:
Let's use MAX_DISTRIBUTED_ATTR_**CNT** = 8;
And change it to package private
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id: " + id);
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+
+ return (OperationContextAttribute<T>)attrs.get(id);
+ }
+
+ /** */
+ public @Nullable DistributedOperationContextAttributesMessage collectDistributedAttributes() {
+ DistributedOperationContextAttributesMessage res = null;
+
+ for (Map.Entry<Byte, OperationContextAttribute<? extends Message>> e : attrs.entrySet()) {
+ OperationContextAttribute<? extends Message> attr = e.getValue();
+
+ Message curVal = OperationContext.get(attr);
+
+ assert attr.initialValue() == null || curVal == null || curVal.getClass().isAssignableFrom(attr.initialValue().getClass());
Review Comment:
Frankly, I don't understand the purpose of this assertion.
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
Review Comment:
Typo.
Let's use a single style - Error msg [param=val] and remove the dot at the end
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id: " + id);
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+
+ return (OperationContextAttribute<T>)attrs.get(id);
+ }
+
+ /** */
+ public @Nullable DistributedOperationContextAttributesMessage collectDistributedAttributes() {
+ DistributedOperationContextAttributesMessage res = null;
+
+ for (Map.Entry<Byte, OperationContextAttribute<? extends Message>> e : attrs.entrySet()) {
+ OperationContextAttribute<? extends Message> attr = e.getValue();
+
+ Message curVal = OperationContext.get(attr);
+
+ assert attr.initialValue() == null || curVal == null || curVal.getClass().isAssignableFrom(attr.initialValue().getClass());
+
+ if (!Objects.equals(curVal, attr.initialValue())) {
Review Comment:
We need to use `==` not equals. The the same way the org.apache.ignite.internal.thread.context.OperationContext.ContextUpdater#set does.
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
Review Comment:
We can make the method static and simplify this logic.
Check if distributed attribute ID is vacant.
Reserve attribute.
Register attribute.
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
Review Comment:
It seems that we should replace it with `id < MAX_DISTRIBUTED_ATTR_CNT`
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
Review Comment:
We can replace it with an array and keep it sorted by ID.
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextAttributesMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distibuted attributes. */
+public class DistributedOperationContextAttributesMessage implements Message {
+ /** Values of operation context attributes. */
+ @Order(0)
+ public List<Message> vals;
+
+ /** Bitmask of effective attributes ids. */
+ @Order(1)
+ public byte idBitmask;
Review Comment:
Bitmask -> bitmap
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id: " + id);
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+
+ return (OperationContextAttribute<T>)attrs.get(id);
+ }
+
+ /** */
+ public @Nullable DistributedOperationContextAttributesMessage collectDistributedAttributes() {
+ DistributedOperationContextAttributesMessage res = null;
+
+ for (Map.Entry<Byte, OperationContextAttribute<? extends Message>> e : attrs.entrySet()) {
+ OperationContextAttribute<? extends Message> attr = e.getValue();
+
+ Message curVal = OperationContext.get(attr);
+
+ assert attr.initialValue() == null || curVal == null || curVal.getClass().isAssignableFrom(attr.initialValue().getClass());
+
+ if (!Objects.equals(curVal, attr.initialValue())) {
+ if (res == null) {
+ res = new DistributedOperationContextAttributesMessage();
+
+ res.vals = new ArrayList<>(MAX_DISTRIBUTED_ATTR_ID / 2);
+ }
+
+ byte mask = (byte)(1 << e.getKey());
+
+ assert (res.idBitmask & mask) == 0;
+
+ res.vals.add(curVal);
+ res.idBitmask |= mask;
+ }
+ }
+
+ return res;
+ }
+
+ /** */
+ public Scope restoreDistributedAttributes(@Nullable DistributedOperationContextAttributesMessage msg) {
+ if (msg == null)
+ return Scope.NOOP_SCOPE;
+
+ assert msg.idBitmask != 0;
+ assert !F.isEmpty(msg.vals);
+ assert msg.vals.size() <= MAX_DISTRIBUTED_ATTR_ID;
+
+ OperationContext.ContextUpdater updater = OperationContext.ContextUpdater.create();
+
+ for (byte valIdx = 0, maskIdx = -1; valIdx < msg.vals.size(); ++valIdx) {
Review Comment:
Do we really need to start maskIdx at -1?
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryAbstractMessage.java:
##########
@@ -76,6 +77,11 @@ public abstract class TcpDiscoveryAbstractMessage implements Message {
@Order(4)
Set<UUID> failedNodes;
+ /** Operation context attributes message. */
+ @GridToStringInclude
+ @Order(5)
+ public @Nullable DistributedOperationContextAttributesMessage opCtxMsg;
Review Comment:
I would prefer to have getters and setters just like for all other variables.
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextAttributesMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distibuted attributes. */
+public class DistributedOperationContextAttributesMessage implements Message {
+ /** Values of operation context attributes. */
+ @Order(0)
+ public List<Message> vals;
Review Comment:
I would prefer just plain array 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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452756444
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
Review Comment:
There are an assert now
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3455753234
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * A manager of {@link OperationContextAttribute} which are required to be propagated through a cluster.
Review Comment:
`/**
* Provides the ability to manage {@link OperationContext} attributes in a distributed manner.
*
* <p>This mechanism is primarily used to propagate {@link OperationContext} state across the cluster by
* capturing it before a message is sent, transferring it together with the message, and restoring it on
* the receiving node before message processing begins.</p>
*
* <p>The implementation relies on a mapping between a distributed identifier and an
* {@link OperationContextAttribute} instance that is consistent across all cluster nodes.</p>
*
* <p>To enable propagation of an {@link OperationContextAttribute} value across cluster nodes, the
* attribute must be created using the {@link #createDistributedAttribute(byte, Message)} method.
*
* <p> Note, that the maximum number of distributed attribute instances that can be created is currently limited to
* {@link #MAX_DISTRIBUTED_ATTR_CNT} for implementation reasons.</p>
*
* @see OperationContext
* @see DistributedOperationContextMessage
*/`
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3456958107
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ClientImpl.java:
##########
@@ -1757,276 +1761,290 @@ private MessageWorker(IgniteLogger log) {
blockingSectionEnd();
}
- if (msg instanceof JoinTimeout) {
- int joinCnt0 = ((JoinTimeout)msg).joinCnt;
-
- if (joinCnt == joinCnt0) {
- if (state == STARTING) {
- joinError(new IgniteSpiException("Join process timed out, did not receive response for " +
- "join request (consider increasing 'joinTimeout' configuration property) " +
- "[joinTimeout=" + spi.joinTimeout + ", sock=" + currSock + ']'));
-
+ if (msg instanceof TcpDiscoveryAbstractMessage msg0 && msg0.opCtxMsg != null) {
Review Comment:
I cannot wrap entire code because there would not be a message yet.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441532590
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextAttributesMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distibuted attributes. */
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441351001
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
Review Comment:
We can make the method synchronized and simplify this logic.
Check if distributed attribute ID is vacant.
Reserve attribute.
Register attribute.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441351001
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
Review Comment:
We can make the method synchronize and simplify this logic.
Check if distributed attribute ID is vacant.
Reserve attribute.
Register attribute.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "Vladsz83 (via GitHub)" <gi...@apache.org>.
Vladsz83 commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441642878
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextAttributesMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distibuted attributes. */
+public class DistributedOperationContextAttributesMessage implements Message {
+ /** Values of operation context attributes. */
+ @Order(0)
+ public List<Message> vals;
Review Comment:
I've reverted plain array. It cannot change its size while we do not know actual number of effective context attributes. Or we have to do another re-copy of it with actual size. It looks not better to me.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452339180
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distibuted attributes. */
+public class DistributedOperationContextAttributesMessage implements Message {
+ /** Values of operation context attributes. */
+ @Order(0)
+ public List<Message> vals;
Review Comment:
> re-copy of it with actual size.
This doesn't seem to be a problem, since the List will be created on the stack for the duration of the collectDistributedAttributes method.
`DistributedOperationAttributesMessage` can be attached to ALL communication and discovery messages.
But serializing a List currently requires one extra byte for the collection type.
So i think it worth making `DistributedOperationAttributesMessage` as compact as possible.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452364447
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
Review Comment:
The current implementation allows creating a distributed attribute with an index of '100', using it, and then getting a NullPointerException during mesage deserialization. This is unacceptable.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452387656
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationContextAttributeManager {
+ /** */
+ static final byte MAX_DISTRIBUTED_ATTR_CNT = 7;
Review Comment:
Event better - `Byte.MAX_VALUE`
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452533322
##########
modules/core/src/main/java/org/apache/ignite/internal/DistributedOperationContextMessage.java:
##########
@@ -0,0 +1,38 @@
+/*
+ * 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;
+
+import java.util.List;
+import org.apache.ignite.internal.thread.context.OperationContext;
+import org.apache.ignite.plugin.extensions.communication.Message;
+
+/** Transport for {@link OperationContext} distributed attributes. */
+public class DistributedOperationContextMessage implements Message {
+ /** Values of operation context attributes. */
+ @Order(0)
+ public List<Message> vals;
+
+ /** Bitmask of effective attributes ids. */
Review Comment:
Bitmask -> Bitmap.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3455753234
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * A manager of {@link OperationContextAttribute} which are required to be propagated through a cluster.
Review Comment:
/**
* Provides the ability to manage {@link OperationContext} attributes in a distributed manner.
*
* <p>This mechanism is primarily used to propagate {@link OperationContext} state across the cluster by
* capturing it before a message is sent, transferring it together with the message, and restoring it on
* the receiving node before message processing begins.</p>
*
* <p>The implementation relies on a mapping between a distributed identifier and an
* {@link OperationContextAttribute} instance that is consistent across all cluster nodes.</p>
*
* <p>To enable propagation of an {@link OperationContextAttribute} value across cluster nodes, the
* attribute must be created using the {@link #createDistributedAttribute(byte, Message)} method.
*
* <p> Note, that the maximum number of distributed attribute instances that can be created is currently limited to
* {@link #MAX_DISTRIBUTED_ATTR_CNT} for implementation reasons.</p>
*
* @see OperationContext
* @see DistributedOperationContextMessage
*/
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452373975
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id: " + id);
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+
+ return (OperationContextAttribute<T>)attrs.get(id);
+ }
+
+ /** */
+ public @Nullable DistributedOperationContextAttributesMessage collectDistributedAttributes() {
+ DistributedOperationContextAttributesMessage res = null;
+
+ for (Map.Entry<Byte, OperationContextAttribute<? extends Message>> e : attrs.entrySet()) {
+ OperationContextAttribute<? extends Message> attr = e.getValue();
+
+ Message curVal = OperationContext.get(attr);
+
+ assert attr.initialValue() == null || curVal == null || curVal.getClass().isAssignableFrom(attr.initialValue().getClass());
Review Comment:
I emphasize again that the asserted guarantee is already provided by OperationContext and is not necessary.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441645362
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
Review Comment:
Sorry, static -> synchronized
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3452734689
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
Review Comment:
I'm still observing the mentioned issue. It looks like the patch requires more thorough testing.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441788603
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
+
+ /** */
+ public static DistributedOperationAttributeManager instance() {
+ return INSTANCE;
+ }
+
+ /** */
+ public <T extends Message> OperationContextAttribute<T> createDistributedAttriubte(byte id, @Nullable T initVal) {
+ assert id >= 0;
+
+ if (attrs.size() == OperationContextAttribute.MAX_ATTR_CNT)
+ throw new IgniteException("Maximum number of ributed attributes is exceeded [" + OperationContextAttribute.MAX_ATTR_CNT + "].");
+
+ attrs.compute(id, (id0, attr0) -> {
+ if (attr0 != null)
+ throw new IgniteException("Duplicated distributed attribute id: " + id);
+
+ return OperationContextAttribute.newInstance(initVal);
+ });
+
+ return (OperationContextAttribute<T>)attrs.get(id);
+ }
+
+ /** */
+ public @Nullable DistributedOperationContextAttributesMessage collectDistributedAttributes() {
+ DistributedOperationContextAttributesMessage res = null;
+
+ for (Map.Entry<Byte, OperationContextAttribute<? extends Message>> e : attrs.entrySet()) {
+ OperationContextAttribute<? extends Message> attr = e.getValue();
+
+ Message curVal = OperationContext.get(attr);
+
+ assert attr.initialValue() == null || curVal == null || curVal.getClass().isAssignableFrom(attr.initialValue().getClass());
Review Comment:
We already have this guarantee - OperationContextAttribute is a template class. OperationContext#set is a template method.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3441764812
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationAttributeManager.java:
##########
@@ -0,0 +1,117 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextAttributesMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/** */
+public class DistributedOperationAttributeManager {
+ /** */
+ public static final byte MAX_DISTRIBUTED_ATTR_ID = 7;
+
+ /** */
+ private static final DistributedOperationAttributeManager INSTANCE = new DistributedOperationAttributeManager();
+
+ /** Attributes by their id. */
+ private final Map<Byte, OperationContextAttribute<? extends Message>> attrs = new ConcurrentHashMap<>();
Review 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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3455753234
##########
modules/core/src/main/java/org/apache/ignite/internal/thread/context/DistributedOperationContextManager.java:
##########
@@ -0,0 +1,128 @@
+/*
+ * 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.thread.context;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentSkipListMap;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.DistributedOperationContextMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * A manager of {@link OperationContextAttribute} which are required to be propagated through a cluster.
Review Comment:
```
/**
* Provides the ability to manage {@link OperationContext} attributes in a distributed manner.
*
* <p>This mechanism is primarily used to propagate {@link OperationContext} state across the cluster by
* capturing it before a message is sent, transferring it together with the message, and restoring it on
* the receiving node before message processing begins.</p>
*
* <p>The implementation relies on a mapping between a distributed identifier and an
* {@link OperationContextAttribute} instance that is consistent across all cluster nodes.</p>
*
* <p>To enable propagation of an {@link OperationContextAttribute} value across cluster nodes, the
* attribute must be created using the {@link #createDistributedAttribute(byte, Message)} method.
*
* <p> Note, that the maximum number of distributed attribute instances that can be created is currently limited to
* {@link #MAX_DISTRIBUTED_ATTR_CNT} for implementation reasons.</p>
*
* @see OperationContext
* @see DistributedOperationContextMessage
*/
```
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 : Introduce a operation context attributes registry v2 [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg commented on code in PR #13243:
URL: https://github.com/apache/ignite/pull/13243#discussion_r3455790193
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/ClientImpl.java:
##########
@@ -1757,276 +1761,290 @@ private MessageWorker(IgniteLogger log) {
blockingSectionEnd();
}
- if (msg instanceof JoinTimeout) {
- int joinCnt0 = ((JoinTimeout)msg).joinCnt;
-
- if (joinCnt == joinCnt0) {
- if (state == STARTING) {
- joinError(new IgniteSpiException("Join process timed out, did not receive response for " +
- "join request (consider increasing 'joinTimeout' configuration property) " +
- "[joinTimeout=" + spi.joinTimeout + ", sock=" + currSock + ']'));
-
+ if (msg instanceof TcpDiscoveryAbstractMessage msg0 && msg0.opCtxMsg != null) {
Review Comment:
Let's just leave the method as is, but wrap all of its code in a try-with-resource block.
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] IGNITE-28770 Added propagation of OperationContext attributes on remote nodes via Discovery [ignite]
Posted by "petrov-mg (via GitHub)" <gi...@apache.org>.
petrov-mg merged PR #13243:
URL: https://github.com/apache/ignite/pull/13243
--
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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org