CASSANALYTICS-177: Add instanceId query parameter to fix HTTP 421 err…

CHANGES.txt

@@ -1,5 +1,6 @@
 0.5.0
 -----
+ * Add sidecar.instance.id Spark conf to append an instanceId query parameter to outbound sidecar requests, fixing 421 errors when Sidecar is behind a load balancer; contact points can also declare their own per-instance id via a "host[:port]=<id>" suffix (CASSANALYTICS-177)
  * TokenPartitioner fails to detect range gap in reader (CASSANALYTICS-180)
  * CDC reader stats silently dropped in SidecarCdcBuilder (CASSANALYTICS-191)
  * Add CapturePublishedSchema metric to SidecarCdcStats (CASSANALYTICS-189)

analytics-sidecar-client-common/src/main/java/org/apache/cassandra/sidecar/client/SidecarInstance.java

@@ -32,4 +32,20 @@ public interface SidecarInstance
      * @return the hostname where the Cassandra Sidecar instance is running
      */
     String hostname();
+
+    /**
+     * Returns the identifier of the specific Cassandra instance that requests sent to this Sidecar
+     * endpoint should be routed to, or {@code null} when no per-instance identifier is configured.
+     *
+     * <p>When non-null, this value is used to populate the {@code instanceId} query parameter on outbound
+     * requests so the Sidecar can resolve the correct local Cassandra instance even when a shared address
+     * (for example a load balancer) hides the real target from the {@code Host} header. When {@code null},
+     * the client falls back to the job-level {@code instanceId} configured on the HTTP client, if any.
+     *
+     * @return the per-instance identifier, or {@code null} when not set
+     */
+    default Integer instanceId()
+    {
+        return null;
+    }
 }

analytics-sidecar-client-common/src/main/java/org/apache/cassandra/sidecar/client/SidecarInstanceImpl.java

@@ -27,22 +27,45 @@ public class SidecarInstanceImpl implements SidecarInstance
 {
     protected int port;
     protected String hostname;
+    protected Integer instanceId;
 
     /**
-     * Constructs a new Sidecar instance with the given {@code port} and {@code hostname}
+     * Constructs a new Sidecar instance with the given {@code port} and {@code hostname} and no
+     * per-instance identifier (requests fall back to the job-level {@code instanceId}, if any).
      *
      * @param hostname the host name where Sidecar is running
      * @param port     the port where Sidecar is running
      */
     public SidecarInstanceImpl(String hostname, int port)
+    {
+        this(hostname, port, null);
+    }
+
+    /**
+     * Constructs a new Sidecar instance with the given {@code hostname}, {@code port} and per-instance
+     * {@code instanceId}.
+     *
+     * @param hostname   the host name where Sidecar is running
+     * @param port       the port where Sidecar is running
+     * @param instanceId the identifier of the Cassandra instance that requests sent to this Sidecar
+     *                   endpoint should be routed to, or {@code null} to fall back to the job-level
+     *                   {@code instanceId}
+     */
+    public SidecarInstanceImpl(String hostname, int port, Integer instanceId)
     {
         if (port < 1 || port > 65535)
         {
             throw new IllegalArgumentException(String.format("Invalid port number for the Sidecar service: %d",
                                                              port));
         }
+        if (instanceId != null && instanceId < 0)
+        {
+            throw new IllegalArgumentException(String.format("Invalid instanceId for the Sidecar service: %d",
+                                                             instanceId));
+        }
         this.port = port;
         this.hostname = Objects.requireNonNull(hostname, "The Sidecar hostname must be non-null");
+        this.instanceId = instanceId;
     }
 
     /**
@@ -63,6 +86,15 @@ public String hostname()
         return hostname;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    public Integer instanceId()
+    {
+        return instanceId;
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -78,7 +110,7 @@ public boolean equals(Object o)
             return false;
         }
         SidecarInstanceImpl that = (SidecarInstanceImpl) o;
-        return port == that.port && Objects.equals(hostname, that.hostname);
+        return port == that.port && Objects.equals(hostname, that.hostname) && Objects.equals(instanceId, that.instanceId);
     }
 
     /**
@@ -87,7 +119,7 @@ public boolean equals(Object o)
     @Override
     public int hashCode()
     {
-        return Objects.hash(port, hostname);
+        return Objects.hash(port, hostname, instanceId);
     }
 
     /**
@@ -99,6 +131,7 @@ public String toString()
         return "SidecarInstanceImpl{" +
                "port=" + port +
                ", hostname='" + hostname + '\'' +
+               ", instanceId=" + instanceId +
                '}';
     }
 }

analytics-sidecar-client-common/src/main/java/org/apache/cassandra/sidecar/common/http/SidecarQueryParamNames.java

@@ -0,0 +1,40 @@
+/*
+ * 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.cassandra.sidecar.common.http;
+
+/**
+ * Custom query parameter names for sidecar HTTP requests.
+ */
+public final class SidecarQueryParamNames
+{
+    /**
+     * {@code "instanceId"} query parameter. When present on an outbound sidecar request it carries
+     * the job-level instance identifier supplied by the client (see the Spark conf key
+     * {@code spark.cassandra_analytics.sidecar.instance.id}).
+     *
+     * <p>Requires a Sidecar server &gt;= 0.2.0 (see {@code AbstractHandler#host}, introduced in
+     * CASSSIDECAR-208); older servers do not resolve this parameter and requests will fall back to
+     * Host-header-based instance resolution.
+     */
+    public static final String INSTANCE_ID = "instanceId";
+
+    private SidecarQueryParamNames()
+    {
+    }
+}

analytics-sidecar-client-common/src/main/java/org/apache/cassandra/sidecar/common/response/TokenRangeReplicasResponse.java

@@ -24,6 +24,7 @@
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import org.apache.cassandra.sidecar.common.request.TokenRangeReplicasRequest;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * Class response for the {@link TokenRangeReplicasRequest}
@@ -179,20 +180,24 @@ public static class ReplicaMetadata
         private final String address;
         private final int port;
         private final String datacenter;
+        @Nullable
+        private final Integer sidecarInstanceId;
 
         public ReplicaMetadata(@JsonProperty("state") String state,
                                @JsonProperty("status") String status,
                                @JsonProperty("fqdn") String fqdn,
                                @JsonProperty("address") String address,
                                @JsonProperty("port") int port,
-                               @JsonProperty("datacenter") String datacenter)
+                               @JsonProperty("datacenter") String datacenter,
+                               @JsonProperty("sidecarInstanceId") @Nullable Integer sidecarInstanceId)
         {
             this.state = state;
             this.status = status;
             this.fqdn = fqdn;
             this.address = address;
             this.port = port;
             this.datacenter = datacenter;
+            this.sidecarInstanceId = sidecarInstanceId;
         }
 
         /**
@@ -249,6 +254,18 @@ public String datacenter()
             return datacenter;
         }
 
+        /**
+         * @return the id of the Sidecar instance that manages this replica, or {@code null} when the Sidecar
+         * that served the request does not manage this replica. Used to route per-replica requests through a
+         * load balancer via the {@code instanceId} query parameter.
+         */
+        @JsonProperty("sidecarInstanceId")
+        @Nullable
+        public Integer sidecarInstanceId()
+        {
+            return sidecarInstanceId;
+        }
+
         /**
          * {@inheritDoc}
          */
@@ -261,6 +278,7 @@ public String toString()
                    ", address='" + address + '\'' +
                    ", port='" + port + '\'' +
                    ", datacenter='" + datacenter + '\'' +
+                   ", sidecarInstanceId=" + sidecarInstanceId +
                    '}';
         }
     }

analytics-sidecar-client/src/main/java/org/apache/cassandra/sidecar/client/HttpClientConfig.java

@@ -38,6 +38,7 @@ public class HttpClientConfig
     public static final String DEFAULT_TRUST_STORE_TYPE = "JKS";
     public static final String DEFAULT_KEY_STORE_TYPE = "PKCS12";
     public static final String DEFAULT_CASSANDRA_ROLE = null;
+    public static final Integer DEFAULT_INSTANCE_ID = null;
 
     private final long timeoutMillis;
     private final boolean ssl;
@@ -54,6 +55,7 @@ public class HttpClientConfig
     private final String keyStorePassword;
     private final String keyStoreType;
     private final String cassandraRole;
+    private final Integer instanceId;
 
     private HttpClientConfig(Builder<?> builder)
     {
@@ -72,6 +74,7 @@ private HttpClientConfig(Builder<?> builder)
         keyStorePassword = builder.keyStorePassword;
         keyStoreType = builder.keyStoreType;
         cassandraRole = builder.cassandraRole;
+        instanceId = builder.instanceId;
     }
 
     /**
@@ -192,6 +195,15 @@ public String cassandraRole()
         return cassandraRole;
     }
 
+    /**
+     * @return the job-level sidecar instance identifier, or {@code null} to omit the {@code instanceId} query parameter
+     */
+    @Nullable
+    public Integer instanceId()
+    {
+        return instanceId;
+    }
+
     /**
      * {@code HttpClient} builder static inner class.
      *
@@ -214,6 +226,7 @@ public static class Builder<T extends Builder<T>>
         private String keyStorePassword;
         private String keyStoreType = DEFAULT_KEY_STORE_TYPE;
         private String cassandraRole = DEFAULT_CASSANDRA_ROLE;
+        private Integer instanceId = DEFAULT_INSTANCE_ID;
 
         /**
          * @return a reference to itself
@@ -412,6 +425,26 @@ public T cassandraRole(String cassandraRole)
             return self();
         }
 
+        /**
+         * Sets the {@code instanceId} query parameter appended to every outbound sidecar request,
+         * and returns a reference to this Builder enabling method chaining. Non-null values must
+         * be greater than or equal to {@code 0}.
+         *
+         * @param instanceId the {@code instanceId} to set, or {@code null} to disable it
+         * @return a reference to this Builder
+         */
+        public T instanceId(Integer instanceId)
+        {
+            // Re-validated in BulkSparkConf.getSidecarInstanceId() to surface a Spark-conf-specific
+            // error message early; keep this constraint (>= 0) in sync with that check.
+            if (instanceId != null && instanceId < 0)
+            {
+                throw new IllegalArgumentException("instanceId must be greater than or equal to 0");
+            }
+            this.instanceId = instanceId;
+            return self();
+        }
+
         /**
          * Returns a {@code SidecarClientConfig} built from the parameters previously set.
          *

analytics-sidecar-client/src/test/java/org/apache/cassandra/sidecar/client/HttpClientConfigTest.java

@@ -23,6 +23,7 @@
 import org.junit.jupiter.api.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.Mockito.mock;
 
 /**
@@ -159,4 +160,40 @@ void testCassandraRole()
         HttpClientConfig config = new HttpClientConfig.Builder<>().cassandraRole("custom_role").build();
         assertThat(config.cassandraRole()).isEqualTo("custom_role");
     }
+
+    @Test
+    void testInstanceIdDefaultIsNull()
+    {
+        HttpClientConfig config = new HttpClientConfig.Builder<>().build();
+        assertThat(config.instanceId()).isNull();
+    }
+
+    @Test
+    void testInstanceId()
+    {
+        HttpClientConfig config = new HttpClientConfig.Builder<>().instanceId(42).build();
+        assertThat(config.instanceId()).isEqualTo(42);
+    }
+
+    @Test
+    void testInstanceIdZeroIsAllowed()
+    {
+        HttpClientConfig config = new HttpClientConfig.Builder<>().instanceId(0).build();
+        assertThat(config.instanceId()).isEqualTo(0);
+    }
+
+    @Test
+    void testInstanceIdNullDisablesIt()
+    {
+        HttpClientConfig config = new HttpClientConfig.Builder<>().instanceId(null).build();
+        assertThat(config.instanceId()).isNull();
+    }
+
+    @Test
+    void testInstanceIdNegativeThrows()
+    {
+        assertThatThrownBy(() -> new HttpClientConfig.Builder<>().instanceId(-1))
+        .isExactlyInstanceOf(IllegalArgumentException.class)
+        .hasMessageContaining("instanceId must be greater than or equal to 0");
+    }
 }

analytics-sidecar-client/src/test/java/org/apache/cassandra/sidecar/client/SidecarInstanceImplTest.java

@@ -18,6 +18,11 @@
 
 package org.apache.cassandra.sidecar.client;
 
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+
 /**
  * Unit tests for the {@link SidecarInstanceImpl} class
  */
@@ -28,4 +33,39 @@ protected SidecarInstance newInstance(String hostname, int port)
     {
         return new SidecarInstanceImpl(hostname, port);
     }
+
+    @Test
+    void testInstanceIdDefaultsToNull()
+    {
+        assertThat(new SidecarInstanceImpl("localhost", 8080).instanceId()).isNull();
+    }
+
+    @Test
+    void testInstanceIdIsRetained()
+    {
+        assertThat(new SidecarInstanceImpl("localhost", 8080, 2).instanceId()).isEqualTo(2);
+        assertThat(new SidecarInstanceImpl("localhost", 8080, 0).instanceId()).isEqualTo(0);
+    }
+
+    @Test
+    void testNegativeInstanceIdRejected()
+    {
+        assertThatExceptionOfType(IllegalArgumentException.class)
+        .isThrownBy(() -> new SidecarInstanceImpl("localhost", 8080, -1))
+        .withMessageContaining("Invalid instanceId for the Sidecar service: -1");
+    }
+
+    @Test
+    void testEqualityDistinguishesInstanceId()
+    {
+        SidecarInstance a = new SidecarInstanceImpl("localhost", 8080, 1);
+        SidecarInstance b = new SidecarInstanceImpl("localhost", 8080, 2);
+        SidecarInstance c = new SidecarInstanceImpl("localhost", 8080, 1);
+        SidecarInstance noId = new SidecarInstanceImpl("localhost", 8080);
+
+        assertThat(a).isEqualTo(c);
+        assertThat(a).hasSameHashCodeAs(c);
+        assertThat(a).isNotEqualTo(b);
+        assertThat(a).isNotEqualTo(noId);
+    }
 }