Vladsz83 commented on code in PR #13095:
URL: https://github.com/apache/ignite/pull/13095#discussion_r3554540262
##########
modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoverySpi.java:
##########
@@ -1819,7 +1820,9 @@ protected <T> T readMessage(TcpDiscoveryIoSession ses,
long timeout) throws IOEx
try {
sock.setSoTimeout((int)timeout);
- return ses.readMessage();
Review Comment:
Why changed?
##########
modules/core/src/test/resources/codegen/ChildMessageSerializer.java:
##########
@@ -29,6 +29,10 @@
* @see org.apache.ignite.internal.MessageProcessor
*/
public class ChildMessageSerializer implements MessageSerializer<ChildMessage>
{
+ /** */
+ public ChildMessageSerializer() {
Review Comment:
Do we need this?
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/CacheIdAware.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * 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.plugin.extensions.communication;
+
+/**
+ * Implemented by messages targeting a specific cache. The cache ID lets the
generated marshaller and deployer resolve
+ * the cache object context used to (un)marshal and deploy the message's cache
objects.
Review Comment:
What is deploy btw? Might be clear later.
##########
modules/core/src/main/java/org/apache/ignite/internal/TxEntriesInfo.java:
##########
@@ -20,14 +20,14 @@
import java.util.Collection;
import java.util.HashSet;
import java.util.Objects;
-import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.internal.processors.cache.GridCacheContext;
import org.apache.ignite.internal.processors.cache.GridCacheMapEntry;
import org.apache.ignite.internal.processors.cache.KeyCacheObject;
import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.plugin.extensions.communication.CacheIdAware;
-/** */
-public final class TxEntriesInfo extends
IgniteDiagnosticRequest.DiagnosticBaseInfo {
+/** Diagnostic request carrying cache keys pending transaction lock info for a
specific cache. */
Review Comment:
Lets rewrite/split/simplify `cache keys pending transaction lock info`
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/service/ServiceSingleNodeDeploymentResult.java:
##########
@@ -76,8 +77,14 @@ public Collection<Throwable> errors() {
* @param errors Exceptions.
*/
public void errors(@Nullable Collection<Throwable> errors) {
- if (!F.isEmpty(errors))
- this.errors = F.viewReadOnly(errors, ErrorMessage::new);
Review Comment:
Why changed?
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/CacheIdAware.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * 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.plugin.extensions.communication;
+
+/**
+ * Implemented by messages targeting a specific cache. The cache ID lets the
generated marshaller and deployer resolve
+ * the cache object context used to (un)marshal and deploy the message's cache
objects.
+ */
Review Comment:
Wy might also refer some `@see` to a factory or a serializer.
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cluster/ClusterProcessor.java:
##########
@@ -379,6 +380,13 @@ public void initDiagnosticListeners() throws
IgniteCheckedException {
if (msg instanceof IgniteDiagnosticRequest) {
IgniteDiagnosticRequest infoReq =
(IgniteDiagnosticRequest)msg;
+ try {
+ MessageMarshaller.unmarshal(ctx.messageFactory(),
infoReq, ctx, null, U.gridClassLoader());
Review Comment:
Why is not unmarshalled automatically?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/DeployableMessage.java:
##########
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.cache;
+
+import org.apache.ignite.IgniteCheckedException;
+
+/**
+ * A {@link GridCacheMessage} with custom deployment logic that cannot be
inferred from field types (conditional
Review Comment:
Can we clarify what deployment is. User custom code? Uploadable to cluster
classes? What is `inferred field deployment`?
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/MessageSerializer.java:
##########
@@ -38,4 +38,36 @@ public interface MessageSerializer<M extends Message> {
* @return Whether message was fully read.
*/
public boolean readFrom(M msg, MessageReader reader);
+
+ /**
+ * Writes the message using the serializer resolved from the factory.
+ *
+ * @param factory Message factory.
+ * @param msg Message instance.
+ * @param writer Writer.
+ * @param <M> Message type.
+ * @return Whether message was fully written.
+ */
+ static <M extends Message> boolean writeTo(MessageFactory factory, M msg,
MessageWriter writer) {
+ return resolve(factory, msg).writeTo(msg, writer);
+ }
+
+ /**
+ * Reads the message using the serializer resolved from the factory.
+ *
+ * @param factory Message factory.
+ * @param msg Message instance.
+ * @param reader Reader.
+ * @param <M> Message type.
+ * @return Whether message was fully read.
+ */
+ static <M extends Message> boolean readFrom(MessageFactory factory, M msg,
MessageReader reader) {
+ return resolve(factory, msg).readFrom(msg, reader);
+ }
+
+ /** @return the serializer registered for {@code msg}'s direct type. */
+ @SuppressWarnings("unchecked")
+ private static <M extends Message> MessageSerializer<M>
resolve(MessageFactory factory, M msg) {
Review Comment:
Is used only within the interface. Do we need to expose it? Or we need some
`SerializationUtil/Helper?`
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/CacheIdAware.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * 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.plugin.extensions.communication;
+
+/**
+ * Implemented by messages targeting a specific cache. The cache ID lets the
generated marshaller and deployer resolve
+ * the cache object context used to (un)marshal and deploy the message's cache
objects.
+ */
+public interface CacheIdAware {
+ /** @return Cache ID identifying the target cache. */
Review Comment:
`target cache` -> smth. like `cache data/CacheObject to transfer`
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/MessageSerializer.java:
##########
@@ -38,4 +38,36 @@ public interface MessageSerializer<M extends Message> {
* @return Whether message was fully read.
*/
public boolean readFrom(M msg, MessageReader reader);
+
+ /**
+ * Writes the message using the serializer resolved from the factory.
+ *
+ * @param factory Message factory.
+ * @param msg Message instance.
+ * @param writer Writer.
+ * @param <M> Message type.
+ * @return Whether message was fully written.
+ */
+ static <M extends Message> boolean writeTo(MessageFactory factory, M msg,
MessageWriter writer) {
Review Comment:
Do we need to expose it? Or we need some `SerializationUtil/Helper` or an
abstract class instead of declaring static methods in an interface?
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/CacheIdAware.java:
##########
@@ -0,0 +1,27 @@
+/*
+ * 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.plugin.extensions.communication;
+
+/**
+ * Implemented by messages targeting a specific cache. The cache ID lets the
generated marshaller and deployer resolve
Review Comment:
` targeting a specific cache` -> smth. like `holding certain cache
data/CacheObject to transfer`?
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/KeyCacheObjectImpl.java:
##########
@@ -108,19 +108,20 @@ public KeyCacheObjectImpl(Object val, byte[] valBytes,
int part) {
/** {@inheritDoc} */
@Override public int hashCode() {
- assert val != null;
Review Comment:
Is `assert` isn't enough?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
