-
Notifications
You must be signed in to change notification settings - Fork 1.9k
IGNITE-27684 Update IGNITE_HEADER for discovery protocol #12671
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| /* | ||
| * 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.compatibility.spi.discovery; | ||
|
|
||
| import org.apache.ignite.compatibility.IgniteReleasedVersion; | ||
| import org.apache.ignite.compatibility.testframework.junits.IgniteCompatibilityAbstractTest; | ||
| import org.apache.ignite.configuration.IgniteConfiguration; | ||
| import org.apache.ignite.testframework.GridTestUtils; | ||
| import org.apache.ignite.testframework.ListeningTestLogger; | ||
| import org.apache.ignite.testframework.LogListener; | ||
| import org.junit.Test; | ||
|
|
||
| /** */ | ||
| public class TcpDiscoveryDifferentClusterVersionsTest extends IgniteCompatibilityAbstractTest { | ||
| /** */ | ||
| private static final String LEGACY_PROTOCOL_MSG = "Remote node uses legacy discovery protocol"; | ||
|
|
||
| /** */ | ||
| private static final IgniteReleasedVersion OLD_VERSION = IgniteReleasedVersion.VER_2_17_0; | ||
|
|
||
| /** */ | ||
| private ListeningTestLogger listeningLog; | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override protected void afterTest() throws Exception { | ||
| super.afterTest(); | ||
|
|
||
| stopAllGrids(); | ||
| } | ||
|
|
||
| /** {@inheritDoc} */ | ||
| @Override protected IgniteConfiguration getConfiguration(String igniteInstanceName) throws Exception { | ||
| IgniteConfiguration cfg = super.getConfiguration(igniteInstanceName); | ||
|
|
||
| if (listeningLog != null) | ||
| cfg.setGridLogger(listeningLog); | ||
|
|
||
| return cfg; | ||
| } | ||
|
|
||
| /** Tests that connection from client of old version is properly refused. */ | ||
| @Test | ||
| public void testOldClientRejected() throws Exception { | ||
| setLoggerDebugLevel(); | ||
|
|
||
| listeningLog = new ListeningTestLogger(log); | ||
|
|
||
| LogListener logListener = LogListener.matches(LEGACY_PROTOCOL_MSG).build(); | ||
|
|
||
| listeningLog.registerListener(logListener); | ||
|
|
||
| startGrid(0); | ||
|
|
||
| GridTestUtils.assertThrows( | ||
| log, | ||
| () -> startGrid("old-client", OLD_VERSION.toString(), cfg -> cfg.setClientMode(true)), | ||
| AssertionError.class, | ||
| null | ||
| ); | ||
|
|
||
| assertTrue("Expected log about different protocol.", logListener.check(getTestTimeout())); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -228,6 +228,7 @@ | |
| import org.apache.ignite.spi.IgniteSpiException; | ||
| import org.apache.ignite.spi.discovery.DiscoverySpi; | ||
| import org.apache.ignite.spi.discovery.DiscoverySpiOrderSupport; | ||
| import org.apache.ignite.spi.discovery.tcp.TcpDiscoveryIoSession; | ||
| import org.apache.ignite.spi.discovery.tcp.TcpDiscoverySpi; | ||
| import org.apache.ignite.thread.IgniteThread; | ||
| import org.apache.ignite.thread.IgniteThreadFactory; | ||
|
|
@@ -349,8 +350,19 @@ public abstract class IgniteUtils extends CommonUtils { | |
| public static final String JMX_DOMAIN = IgniteUtils.class.getName().substring(0, IgniteUtils.class.getName(). | ||
| indexOf('.', IgniteUtils.class.getName().indexOf('.') + 1)); | ||
|
|
||
| /** Network packet header. */ | ||
| public static final byte[] IGNITE_HEADER = intToBytes(0x00004747); | ||
| /** | ||
| * Network packet header for discovery protocol V1 (legacy). | ||
| * V1 discovery messages use Java serialization (JdkMarshaller). | ||
| */ | ||
| public static final byte[] IGNITE_HEADER_V1 = intToBytes(0x00004747); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 00GG |
||
|
|
||
| /** | ||
| * Network packet header for discovery protocol V2. | ||
| * Used in discovery handshake to support rolling upgrade compatibility. | ||
| * V2 packets include a leading serialization mode byte ({@code serMode}) which defines how the payload is serialized. | ||
| * See {@link TcpDiscoveryIoSession#JAVA_SERIALIZATION} and {@link TcpDiscoveryIoSession#MESSAGE_SERIALIZATION}. | ||
| */ | ||
| public static final byte[] IGNITE_HEADER_V2 = intToBytes(0x0049474E); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0IGN
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really need any header? What it used for?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We still need a header as a simple magic prefix to quickly reject non-Ignite or wrong-endpoint connections before we try to parse any data. IGNITE_HEADER_v2 makes the new handshake path explicit and helps keep backward compatibility and clearer diagnostics for version mismatches. |
||
|
|
||
| /** Default buffer size = 4K. */ | ||
| private static final int BUF_SIZE = 4096; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,9 +28,9 @@ | |
|
|
||
| /** | ||
| * Verifies that first bytes received in accepted (incoming) | ||
| * NIO session are equal to {@link U#IGNITE_HEADER}. | ||
| * NIO session are equal to {@link U#IGNITE_HEADER_V1}. | ||
| * <p> | ||
| * First {@code U.IGNITE_HEADER.length} bytes are consumed by this filter | ||
| * First {@code U.IGNITE_HEADER_V1.length} bytes are consumed by this filter | ||
| * and all other bytes are forwarded through chain without any modification. | ||
| */ | ||
| public class GridConnectionBytesVerifyFilter extends GridNioFilterAdapter { | ||
|
|
@@ -99,27 +99,27 @@ public GridConnectionBytesVerifyFilter(IgniteLogger log) { | |
|
|
||
| Integer magic = ses.meta(MAGIC_META_KEY); | ||
|
|
||
| if (magic == null || magic < U.IGNITE_HEADER.length) { | ||
| if (magic == null || magic < U.IGNITE_HEADER_V1.length) { | ||
| byte[] magicBuf = ses.meta(MAGIC_BUF_KEY); | ||
|
|
||
| if (magicBuf == null) | ||
| magicBuf = new byte[U.IGNITE_HEADER.length]; | ||
| magicBuf = new byte[U.IGNITE_HEADER_V1.length]; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to use V2 header here?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The suggestion is to keep IGNITE_HEADER unchanged in the communication SPI and update it only in discovery. As I understand, this is not related to the discovery SPI |
||
|
|
||
| int magicRead = magic == null ? 0 : magic; | ||
|
|
||
| int cnt = buf.remaining(); | ||
|
|
||
| buf.get(magicBuf, magicRead, Math.min(U.IGNITE_HEADER.length - magicRead, cnt)); | ||
| buf.get(magicBuf, magicRead, Math.min(U.IGNITE_HEADER_V1.length - magicRead, cnt)); | ||
|
|
||
| if (cnt + magicRead < U.IGNITE_HEADER.length) { | ||
| if (cnt + magicRead < U.IGNITE_HEADER_V1.length) { | ||
| // Magic bytes are not fully read. | ||
| ses.addMeta(MAGIC_META_KEY, cnt + magicRead); | ||
| ses.addMeta(MAGIC_BUF_KEY, magicBuf); | ||
| } | ||
| else if (U.bytesEqual(magicBuf, 0, U.IGNITE_HEADER, 0, U.IGNITE_HEADER.length)) { | ||
| // Magic bytes read and equal to IGNITE_HEADER. | ||
| else if (U.bytesEqual(magicBuf, 0, U.IGNITE_HEADER_V1, 0, U.IGNITE_HEADER_V1.length)) { | ||
| // Magic bytes read and equal to IGNITE_HEADER_V1. | ||
| ses.removeMeta(MAGIC_BUF_KEY); | ||
| ses.addMeta(MAGIC_META_KEY, U.IGNITE_HEADER.length); | ||
| ses.addMeta(MAGIC_META_KEY, U.IGNITE_HEADER_V1.length); | ||
|
|
||
| proceedMessageReceived(ses, buf); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6610,7 +6610,16 @@ private class SocketReader extends IgniteSpiThread { | |
| } | ||
| } | ||
|
|
||
| if (!Arrays.equals(buf, U.IGNITE_HEADER)) { | ||
| if (!Arrays.equals(buf, U.IGNITE_HEADER_V2)) { | ||
| if (Arrays.equals(buf, U.IGNITE_HEADER_V1)) { | ||
| if (log.isDebugEnabled()) | ||
| log.debug("Remote node uses legacy discovery protocol (V1) (before the rolling upgrade compatibility). " + | ||
| "Local node expects V2. Verify that Ignite versions are compatible. " + | ||
| "[rmtAddr=" + rmtAddr + ", locAddr=" + sock.getLocalSocketAddress() + "]"); | ||
|
|
||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We discussed to send message to old node with legacy protocol. I tried it but new node fails with It is Caused by: org.apache.ignite.spi.discovery.tcp.messages.TcpDiscoveryAbstractTraceableMessage; local class incompatible: stream classdesc serialVersionUID = 5764055654892291162, local class serialVersionUID = -2609852714099079963 It looks like deserialization is impossible because we changed structure of our messages after refactoring. So it would be challenging to support all legacy formats. As a quick solution, let's just log message about legacy protocol |
||
| return; | ||
| } | ||
|
|
||
| if (log.isDebugEnabled()) | ||
| log.debug("Unknown connection detected (is some other software connecting to " + | ||
| "this Ignite port?" + | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we check only client node?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.