From dff453350aaaca3f85555d13bd7b2e1b999204db Mon Sep 17 00:00:00 2001 From: Stefan Miklosovic Date: Thu, 13 Mar 2025 14:22:36 +0100 Subject: [PATCH] Optionally skip exception logging on invalid legacy protocol magic exception patch by Stefan Miklosovic; reviewed by David Capwell, Brad Schoening for CASSANDRA-19483 --- CHANGES.txt | 1 + .../org/apache/cassandra/config/Config.java | 1 + .../cassandra/config/DatabaseDescriptor.java | 5 +++ .../net/InboundConnectionInitiator.java | 7 ++++ .../test/InternodeErrorExclusionTest.java | 40 ++++++++++++++----- 5 files changed, 45 insertions(+), 9 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index ff0be82b8d..3f972dd9e3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 4.1.9 + * Optionally skip exception logging on invalid legacy protocol magic exception (CASSANDRA-19483) * Fix SimpleClient ability to release acquired capacity (CASSANDRA-20202) * Fix WaitQueue.Signal.awaitUninterruptibly may block forever if invoking thread is interrupted (CASSANDRA-20084) Merged from 4.0: diff --git a/src/java/org/apache/cassandra/config/Config.java b/src/java/org/apache/cassandra/config/Config.java index a2324f76dd..b841d9b7c0 100644 --- a/src/java/org/apache/cassandra/config/Config.java +++ b/src/java/org/apache/cassandra/config/Config.java @@ -820,6 +820,7 @@ public class Config public volatile SubnetGroups client_error_reporting_exclusions = new SubnetGroups(); public volatile SubnetGroups internode_error_reporting_exclusions = new SubnetGroups(); + public volatile boolean invalid_legacy_protocol_magic_no_spam_enabled = false; public volatile int keyspaces_warn_threshold = -1; public volatile int keyspaces_fail_threshold = -1; diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java index cfec737585..853b29f6df 100644 --- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java +++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java @@ -4339,6 +4339,11 @@ public class DatabaseDescriptor return conf.internode_error_reporting_exclusions; } + public static boolean getInvalidLegacyProtocolMagicNoSpamEnabled() + { + return conf.invalid_legacy_protocol_magic_no_spam_enabled; + } + public static boolean getReadThresholdsEnabled() { return conf.read_thresholds_enabled; diff --git a/src/java/org/apache/cassandra/net/InboundConnectionInitiator.java b/src/java/org/apache/cassandra/net/InboundConnectionInitiator.java index 3067b587c9..2257d00050 100644 --- a/src/java/org/apache/cassandra/net/InboundConnectionInitiator.java +++ b/src/java/org/apache/cassandra/net/InboundConnectionInitiator.java @@ -23,9 +23,11 @@ import java.net.SocketAddress; import java.util.List; import java.util.NoSuchElementException; import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Throwables; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -56,6 +58,7 @@ import org.apache.cassandra.security.SSLFactory; import org.apache.cassandra.streaming.StreamDeserializingTask; import org.apache.cassandra.streaming.StreamingChannel; import org.apache.cassandra.streaming.async.NettyStreamingChannel; +import org.apache.cassandra.utils.NoSpamLogger; import org.apache.cassandra.utils.memory.BufferPools; import static java.lang.Math.*; @@ -69,6 +72,8 @@ public class InboundConnectionInitiator { private static final Logger logger = LoggerFactory.getLogger(InboundConnectionInitiator.class); + private static final NoSpamLogger noSpam5m = NoSpamLogger.getLogger(logger, 5, TimeUnit.MINUTES); + private static class Initializer extends ChannelInitializer { private static final String PIPELINE_INTERNODE_ERROR_EXCLUSIONS = "Internode Error Exclusions"; @@ -396,6 +401,8 @@ public class InboundConnectionInitiator if (reportingExclusion) logger.debug("Excluding internode exception for {}; address contained in internode_error_reporting_exclusions", remoteAddress, cause); + else if (cause != null && Throwables.getRootCause(cause) instanceof Message.InvalidLegacyProtocolMagic && DatabaseDescriptor.getInvalidLegacyProtocolMagicNoSpamEnabled()) + noSpam5m.warn("Failed to properly handshake with peer {}. Closing the channel. Invalid legacy protocol magic.", ((InetSocketAddress) channel.remoteAddress()).getHostName()); else logger.error("Failed to properly handshake with peer {}. Closing the channel.", remoteAddress, cause); diff --git a/test/distributed/org/apache/cassandra/distributed/test/InternodeErrorExclusionTest.java b/test/distributed/org/apache/cassandra/distributed/test/InternodeErrorExclusionTest.java index 08fd1229a5..81d0e80eaf 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/InternodeErrorExclusionTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/InternodeErrorExclusionTest.java @@ -53,16 +53,38 @@ public class InternodeErrorExclusionTest extends TestBaseImpl .set("internode_error_reporting_exclusions", ImmutableMap.of("subnets", Arrays.asList("127.0.0.1")))) .start()) { - try (SimpleClient client = SimpleClient.builder("127.0.0.1", 7012).build()) - { - client.connect(true); - Assert.fail("Connection should fail"); - } - catch (Exception e) - { - // expected - } + + causeException(); assertThat(cluster.get(1).logs().watchFor("address contained in internode_error_reporting_exclusions").getResult()).hasSize(1); } } + + @Test + public void testNoSpammingInvalidLegacyProtocolMagicException() throws Throwable + { + try (Cluster cluster = Cluster.build(1) + .withConfig(c -> c + .with(Feature.NETWORK) + .set("invalid_legacy_protocol_magic_no_spam_enabled", true)) + .start()) + { + causeException(); + causeException(); + // we used no spam logger so the second message will not be emitted (the size is still 1). + assertThat(cluster.get(1).logs().watchFor("Failed to properly handshake with peer localhost. Closing the channel. Invalid legacy protocol magic.").getResult()).hasSize(1); + } + } + + private void causeException() + { + try (SimpleClient client = SimpleClient.builder("127.0.0.1", 7012).build()) + { + client.connect(true); + Assert.fail("Connection should fail"); + } + catch (Exception e) + { + // expected + } + } }