Skip to content

[CASSANALYTICS-181] - Support Sidecar behind a load balancer for time… - #223

Open
bianca-stanciu29 wants to merge 1 commit into
apache:trunkfrom
bianca-stanciu29:CASSANALYTICS-181
Open

[CASSANALYTICS-181] - Support Sidecar behind a load balancer for time…#223
bianca-stanciu29 wants to merge 1 commit into
apache:trunkfrom
bianca-stanciu29:CASSANALYTICS-181

Conversation

@bianca-stanciu29

Copy link
Copy Markdown

…-skew validation in coordinated writes

Problem

Time-skew validation before a coordinated bulk write fans out HTTP calls directly to per-replica FQDNs resolved from the Cassandra token map. In deployments where Sidecar is fronted by a load balancer, those FQDNs are not routable from Spark executors, only the load balancer address is. This causes time-skew validation to fail at the start of every coordinated write job, blocking bulk writes entirely in load-balanced Sidecar deployments.

Solution

Add a SIDECAR_BEHIND_LOAD_BALANCER writer option (default: false). When true, time-skew validation skips the per-replica fan-out and routes through the preconfigured load balancer contact points instead

if (conf.sidecarBehindLoadBalancer)
{
LOGGER.info("Using CoordinatedCassandraClusterInfo for load-balanced Sidecar. clusterId={}", clusterId);
return new CoordinatedCassandraClusterInfo(conf, clusterId);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name CoordinatedCassandraClusterInfo is misleading. For the reader, this naming sounds like Coordinated work is happening only when sidecarBehindLoadBalancer is true, and else case below doing non-coordinated work. Need to pick appropriate name instead of CoordinatedCassandraClusterInfo

private static CassandraClusterInfo createClusterInfo(BroadcastableClusterInfo bci)
{
return bci.getConf().sidecarBehindLoadBalancer
? new CoordinatedCassandraClusterInfo(bci)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

@Override
protected CompletableFuture<TimeSkewResponse> fetchTimeSkew(Range<BigInteger> range)
{
return getCassandraContext().getSidecarClient().timeSkew();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same problem exists for Single cluster behind a load balancer. Hence instead of using 'Coordinated' word, good to develop this as a generic framework for handling cluster/clusters behind a load balancer, then we can invoke it from anywhere needed

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, range param is unused, hence mention a comment something like 'range is irrelevant; contact points are queried directly'

this.coordinatedWriteConf = buildCoordinatedWriteConf(dataTransportInfo.getTransport(), logger);
if (this.sidecarBehindLoadBalancer && this.coordinatedWriteConf == null && logger != null)
{
logger.warn("{} is set but {} is not configured; the flag is only honored for coordinated writes and will be ignored on the single-cluster path.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about single cluster behind a load balancer? Can you create a Jira for supporting this case and add Jira here as a TODO ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend making LB handling as generic framework and invoke it from both coordinated and single cluster case, then we will have uniformity and support for both cases.

* {@link org.apache.cassandra.spark.bulkwriter.WriterOptions#SIDECAR_BEHIND_LOAD_BALANCER}
* is set.
*/
public class CoordinatedCassandraClusterInfo extends CassandraClusterInfo

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mentioned above, this naming is incorrect, this class is for Cluster behind a LB, nothing specific to Corodinated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants