scylladb / scylla-tools-java

Apache Cassandra, supplying tools for Scylla
Apache License 2.0
53 stars 85 forks source link

Use ReplicaOrdering.NEUTRAL in TokenAwarePolicy to respect RackAwareness #367

Closed sylwiaszunejko closed 10 months ago

sylwiaszunejko commented 11 months ago

If we specify rack, we want to have replicas sorted based on which one is local. This happens on the java-driver side if we specify ordering in TokenAwarePolicy. Specifically, it has to use ReplicaOrdering.NEUTRAL.

This option was not used before, and that led to broken behavior. Now, Rack Awareness is properly respected.

Fixes: https://github.com/scylladb/java-driver/issues/255

To test I have used case with 6 nodes, and c-s is using RackAwareRoundRobinPolicy that targets only rack rack1 and all stress commands are showing the following logs:

===== Using optimized driver!!! =====
WARN  13:57:02,324 Some contact points don't match local rack. Local rack = rack1. Non-conforming contact points: /127.0.0.2:9042 (dc=datacenter1, rack=rack2),/127.0.0.3:9042 (dc=datacenter1, rack=rack3),/127.0.0.5:9042 (dc=datacenter1, rack=rack2),/127.0.0.6:9042 (dc=datacenter1, rack=rack3)
Connected to cluster: , max pending requests per connection null, max connections per host 8
Datatacenter: datacenter1; Host: /127.0.0.1; Rack: rack1
Datatacenter: datacenter1; Host: /127.0.0.2; Rack: rack2
Datatacenter: datacenter1; Host: /127.0.0.3; Rack: rack3
Datatacenter: datacenter1; Host: /127.0.0.4; Rack: rack1
Datatacenter: datacenter1; Host: /127.0.0.5; Rack: rack2
Datatacenter: datacenter1; Host: /127.0.0.6; Rack: rack3

Before the change for rf=3 it looked like that (first write operation, after that some reads): rack_before After the change for the same parameters we see that majority of the requests goes to 127.0.0.1 and 127.0.0.4 (orange and green lines) as it should. rack_after

sylwiaszunejko commented 11 months ago

@avelanarius I added some graphs to show how the change affects them.

roydahan commented 11 months ago

@nyh / @denesb can you please review and merge if approved?

BTW, what's the process to ping scylla maintainers and how would one get a PR merged without pinging maintainers?

mykaul commented 10 months ago

@nyh / @denesb can you please review and merge if approved?

BTW, what's the process to ping scylla maintainers and how would one get a PR merged without pinging maintainers?

Give them a call. I think pinging the maintainers is a standard practice. (but I do see some unresolved comments here, that might confuse them)

sylwiaszunejko commented 10 months ago

@mykaul all comments are resolved now

mykaul commented 10 months ago

@scylladb/scylla-maint (or is it @scylladb/scylla-jmx-maint ?) - please consider merging.

denesb commented 10 months ago

submodule update: scylladb/scylladb@f6f1ee70ed8df7aba3cfcb941b537fc85ec4da03

denesb commented 10 months ago

submodule update: scylladb/scylladb@f6f1ee7

I dequeued this, I forgot we cannot update tools-java until https://github.com/scylladb/java-driver/pull/267 is fixed.

roydahan commented 10 months ago

https://github.com/scylladb/java-driver/pull/267 passed the CI and now merged.

mykaul commented 10 months ago

scylladb/java-driver#267 passed the CI and now merged.

Now we need to (1) release and (2) update the Java driver version in scylla-tools-java.

avelanarius commented 10 months ago

Java Driver 3.11.5.1 is now released so the version number can be bumped in scylla-tools-java.