anton-vinogradov commented on code in PR #13095:
URL: https://github.com/apache/ignite/pull/13095#discussion_r3596386239
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/GridCacheIoManager.java:
##########
@@ -741,7 +743,7 @@ else if (cacheMsg instanceof
GridNearAtomicCheckUpdateRequest)
* @param plc Message policy.
* @throws IgniteCheckedException If failed.
*/
- private void processFailedMessage(UUID nodeId,
+ public void processFailedMessage(UUID nodeId,
Review Comment:
Done — the first parameter moved to the next line.
##########
modules/core/src/main/java/org/apache/ignite/plugin/extensions/communication/CacheIdAware.java:
##########
@@ -0,0 +1,30 @@
+/*
+ * 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 that carry cache data ({@code CacheObject}s) to
transfer. The cache ID lets the generated
Review Comment:
Fixed.
##########
modules/core/src/main/java/org/apache/ignite/internal/IgniteKernal.java:
##########
@@ -1318,8 +1318,8 @@ else if (e instanceof IgniteCheckedException)
startTimer.finishGlobalStage("Await exchange");
}
- /** */
- private void initMessageFactory() throws IgniteCheckedException {
+ /** Public for a cross-package test; the production call is {@link #start}
below. */
+ public void initMessageFactory() throws IgniteCheckedException {
Review Comment:
Done — `initMessageFactory()` is `private` now, with a public `@TestOnly`
delegate `initMessageFactoryForTest()` for the cross-package test
(`GridCacheIoManagerRetryTest` updated).
##########
modules/core/src/test/java/org/apache/ignite/plugin/extensions/communication/MessageUnmarshalOnceTest.java:
##########
@@ -0,0 +1,73 @@
+/*
+ * 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;
+
+import org.apache.ignite.internal.MarshallableMessage;
+import org.apache.ignite.internal.managers.communication.MessageUnmarshalDedup;
+import org.apache.ignite.marshaller.Marshaller;
+import org.apache.ignite.testframework.junits.common.GridCommonAbstractTest;
+import org.junit.Test;
+
+/**
+ * Verifies the no-double-unmarshal check itself, not its coverage: {@link
MessageUnmarshalDedup#firstUnmarshal} must
+ * detect a second finish-unmarshal of the same instance within one pass,
while allowing the two legitimate passes
+ * (cache-free and cache-aware), and the check must be enabled for every test
— otherwise the suite-wide guard would
+ * silently turn off and pass every test vacuously. The actual coverage (no
real receive path unmarshals an instance
+ * twice in the same pass) comes from running the check across the whole suite
via {@code GridAbstractTest}.
+ */
+public class MessageUnmarshalOnceTest extends GridCommonAbstractTest {
+ /** The suite-wide guard must be on, so a silently-disabled check cannot
pass every test without verifying anything. */
+ @Test
+ public void testCheckEnabled() {
Review Comment:
Agreed — `testCheckEnabled` became a `beforeTestsStarted()` override with
the same assert, so a silently-disabled check now fails the class's real tests
instead of a separate test method.
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/communication/IgniteMessageFactory.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * 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.managers.communication;
+
+import java.util.function.Supplier;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.processors.cache.GridCacheMessageDeployer;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageFactory;
+import org.apache.ignite.plugin.extensions.communication.MessageMarshaller;
+import org.apache.ignite.plugin.extensions.communication.MessageSerializer;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Internal extension of {@link MessageFactory} that also registers and
resolves the marshal and deployment
+ * companions of a message. Kept out of the public factory interface:
marshallers and deployers are bound to
+ * kernal and cache internals ({@code GridKernalContext}, {@code
GridCacheSharedContext}) the public SPI module
+ * must not depend on.
+ *
+ * @see MessageMarshaller
+ * @see GridCacheMessageDeployer
+ */
+public interface IgniteMessageFactory extends MessageFactory {
+ /** {@inheritDoc} */
+ @Override default void register(short directType, Supplier<Message>
supplier, MessageSerializer serializer)
+ throws IgniteException {
+ register(directType, supplier, serializer, null, null);
+ }
+
+ /**
+ * Register message factory with given direct type, serializer, and
marshaller. All messages must be registered
+ * during construction of class which implements this interface.
+ *
+ * @param directType Direct type.
+ * @param supplier Message factory.
+ * @param serializer Message serializer.
+ * @param marshaller Message marshaller, or {@code null} for
non-marshallable messages.
+ * @throws IgniteException In case of attempt to register message with
direct type which is already registered.
+ */
+ default void register(short directType, Supplier<Message> supplier,
MessageSerializer serializer,
+ @Nullable MessageMarshaller marshaller) throws IgniteException {
+ register(directType, supplier, serializer, marshaller, null);
+ }
+
+ /**
+ * Register message factory with given direct type, serializer,
marshaller, and deployer. All messages must be
+ * registered during construction of class which implements this interface.
+ *
+ * @param directType Direct type.
+ * @param supplier Message factory.
+ * @param serializer Message serializer.
+ * @param marshaller Message marshaller, or {@code null} for
non-marshallable messages.
+ * @param deployer Message deployer, or {@code null} for messages without
deployable fields.
+ * @throws IgniteException In case of attempt to register message with
direct type which is already registered.
+ */
+ public void register(short directType, Supplier<Message> supplier,
MessageSerializer serializer,
Review Comment:
Covered by your parametrization patch — applied in this PR (see the reply in
the other thread).
##########
modules/core/src/main/java/org/apache/ignite/internal/managers/eventstorage/GridEventStorageMessage.java:
##########
Review Comment:
The deferral is by design rather than a leftover: filters need the
peer-deployment class loader, which `GridEventStorageManager` resolves
per-filter at processing time — the generic receive pass only has the
configuration loader (same reason `GridIoManager.unmarshalPayload` skips cache
messages). So there's no pending work for a TODO to point at; if you'd like the
filters marshalling revised anyway, I'd file it as a subticket rather than an
untracked TODO.
##########
modules/core/src/test/java/org/apache/ignite/spi/communication/tcp/TestDelayMessageSerializer.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * 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.spi.communication.tcp;
+
+import org.apache.ignite.internal.IgniteInterruptedCheckedException;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.plugin.extensions.communication.MessageReader;
+import org.apache.ignite.plugin.extensions.communication.MessageSerializer;
+import org.apache.ignite.plugin.extensions.communication.MessageWriter;
+
+/** Serializer for {@link TestDelayMessage} that injects an optional write
delay for testing. */
+public class TestDelayMessageSerializer implements
MessageSerializer<TestDelayMessage> {
Review Comment:
It's resolved by naming convention: `MessagesPluginProvider` registers
`TestDelayMessage` with `GridTestUtils.loadSerializer(msg)`, which loads this
class. Codegen intentionally skips the message (`SKIP_MESSAGES` in
`MessageProcessor`) because the hand-written serializer injects a write delay a
generated one can't.
--
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]
