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

[bianca-stanciu29]

Problem

Bulk-write requests sent through a load balancer can fail with HTTP 421
Misdirected Request when Sidecar cannot identify the intended Cassandra
instance from the Host header.

Solution

Attach an optional instanceId query parameter using the ID of the specific
instance targeted by each request.

• Prefer the per-replica sidecarInstanceId reported in Sidecar topology responses.
• Fall back to explicit contact-point IDs using host[:port]= when Sidecar
  does not report an ID.
• Preserve the resolved ID through bulk-writer topology, RingInstance
  serialization, and Sidecar request construction.
• When no ID is resolved, omit the query parameter and retain existing
  Host-header routing.
• Remove the originally proposed global sidecar.instance.id setting to avoid
  applying the same ID to requests targeting different Cassandra instances.
• Update RingInstance.serialVersionUID for the serialization change.

Scope and compatibility

This PR wires instance-ID routing into the bulk-writer path, including
per-cluster contact points for coordinated writes. It does not complete
bulk-reader support behind load balancers.

Dynamic ID discovery depends on apache/cassandra-sidecar#380
(CASSSIDECAR-495), which adds per-instance IDs to Sidecar topology responses.

Older Sidecars can use explicit contact-point IDs, for example:

node1:9043=1,node2:9043=2

The static fallback is keyed by hostname. The configured hostname must
exactly match the replica's reported fqdn to resolve its ID. Different
instance IDs sharing the same hostname cannot be represented by this
fallback.

This change supplies instance IDs for request routing; it does not rewrite
Sidecar destination addresses. The receiving Sidecar must recognize the
supplied ID. Initial topology discovery through a load balancer may also
require an explicit ID on the contact point.

Existing contact points without an ID suffix remain valid. Requests without
a resolved ID continue to use Host-header routing and may still receive
HTTP 421 in the affected load-balancer setup.

Test coverage

The patch adds or updates unit tests covering:

• Contact-point ID parsing and invalid IDs.
• Request query-parameter handling with present and absent IDs, including
  requests that already contain query parameters.
• Preference for dynamically reported IDs over static contact-point IDs.
• Static fallback behavior and hostname-matching limitations.
• RingInstance ID preservation through serialization.
• Bulk-writer data-transfer request routing.

[skoppu22]

This PR stamps the same instanceId on every request — so if the job writes to more than one instance, all but one get mislabeled.

Example:
Setup (a normal 3-node bulk write)
Cassandra ring: instance 1 | instance 2 | instance 3
Sidecar (one config knows all 3, ids 1/2/3), fronted by a load balancer at sidecar-lb:9043
Operator sets: spark.cassandra_analytics.sidecar.instance.id = 2
The writer builds ONE client over all three (CassandraContext.java:90):

AnalyticsSidecarClient.from(new SimpleSidecarInstancesProvider(clusterConfig /* 3 instances */), conf)
Copy
What happens per request (with this PR)
request meant for instance 1 ──▶ ...?instanceId=2 ──▶ server: instanceFromId(2) ──▶ instance 2 ❌ WRONG
request meant for instance 2 ──▶ ...?instanceId=2 ──▶ server: instanceFromId(2) ──▶ instance 2 ✅ right
request meant for instance 3 ──▶ ...?instanceId=2 ──▶ server: instanceFromId(2) ──▶ instance 2 ❌ WRONG
The ?instanceId=2 overrides the Host header on the server, so the server confidently routes the instance-1 and instance-3 work to instance 2. Because config.instanceId() is a single job-global value, vertxRequest(...) applies it uniformly even though it already knows the real target (sidecarInstance) and could distinguish them.

Net: a fixed id is only correct when the job touches exactly one instance. For any real multi-node ring it silently misroutes → the data-correctness risk behind the WARNING.

[bianca-stanciu29]

hi @skoppu22
Thanks for flagging this, it's addressed now.

vertxRequest prefers the target instance's own resolved id over the job-level one, for
any ring size (not just 3): if every instance is individually addressable and tagged via
host[:port]=<id>, each request gets the right id regardless of instance count. If any
instance isn't resolved, validateSidecarInstanceIdCoverage fails fast with a clear error
instead of silently misrouting.

To be upfront: your exact topology, multiple instances behind one shared address, still
can't work with this PR alone, at any scale. A hostname-keyed lookup can't disambiguate targets sharing one address; it now fails fast with "Duplicate key" instead of resolving(see testSidecarInstanceIdsByHostnameThrowsWhenSharedHostnameHasDifferentIds).
Making that work needs Sidecar to report each instance's own id in the ring response, tracking that as a follow-up.

This is the same gap you flagged on #223 (CASSANALYTICS-181), single-cluster jobs behind a load balancer hit it too, and a real fix should cover both paths generically rather than being patched per-PR.
Appreciate a re-review when you get a chance.

[bianca-stanciu29]

Hi @skoppu22
Follow-up is up: apache/cassandra-sidecar#380 (CASSSIDECAR-495).

It adds sidecarInstanceId to both the ring and token-range-replicas
responses, reported per node/replica by the Sidecar itself (via its
local instances config), instead of the client trying to derive it
from a hostname it observes.

This covers the shared-address case you flagged here: multiple
Cassandra instances fronted by one Sidecar address (e.g. behind a
load balancer). The client no longer needs a hostname-keyed lookup at
all, it reads the correct instanceId straight off the ring/
token-range response for each node

[skoppu22]

Hi @skoppu22 Follow-up is up: apache/cassandra-sidecar#380 (CASSSIDECAR-495).

It adds sidecarInstanceId to both the ring and token-range-replicas responses, reported per node/replica by the Sidecar itself (via its local instances config), instead of the client trying to derive it from a hostname it observes.

This covers the shared-address case you flagged here: multiple Cassandra instances fronted by one Sidecar address (e.g. behind a load balancer). The client no longer needs a hostname-keyed lookup at all, it reads the correct instanceId straight off the ring/ token-range response for each node

Sorry for the delay, I am taking a look now

[skoppu22]

Why do we need global instance ID? Why don't we drop sidecar.instance.id, rely on dynamic ids and the host:port= suffix ? That deletes the possibility of incorrectly using global instance ID.

[bianca-stanciu29]

Yes, you're right, I missed this. Good catch!

I removed sidecar.instance.id and the whole global ID path, including instanceId from HttpClientConfig and validateSidecarInstanceIdCoverage.

Routing now only uses the per-instance ID reported by Sidecar (#380) or the host:port= contact-point suffix as a fallback for older Sidecars.

The behavioral change is that a single instance behind a load balancer now needs the ID on the contact point or a Sidecar that reports it dynamically.

Tests are updated accordingly. Ready for another look, thanks!

[bianca-stanciu29]

Hi @skoppu22, could you please take another look at the latest revision for a +1 to merge into trunk?

Following your feedback, I removed the global sidecar.instance.id setting and updated RingInstance.serialVersionUID. Routing now uses the target instance's own ID, preferring Sidecar topology metadata and falling back to explicit host[:port]= contact points.

I've also updated the PR description to clarify the bulk-writer scope, the static fallback limitations, and the dependency on apache/cassandra-sidecar#380 (CASSSIDECAR-495).

Could you confirm whether the Sidecar change should land first and whether there are any remaining blockers to your +1?

Thanks!