[ 
https://issues.apache.org/jira/browse/CASSANDRA-14459?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16775785#comment-16775785
 ] 

Joseph Lynch commented on CASSANDRA-14459:
------------------------------------------

Alright, finally got around to writing those messaging service tests you asked 
for :), although if you're ok with it I'd prefer to refactor the methods to be 
slightly more unit testable and not even bother doing an e2e on the latency 
probe components as in my opinion the tests of that are just gnarly (and while 
I think that they won't flake because I ran them 2k times locally to check for 
race conditions, those kinds of multi-threading tests just worry me).

Some notable changes in behavior since the last patch:
 * The StartupConnectivityChecker now sends a second ping that counts after all 
the first round ones come back. While I was testing this patch with CCM I 
noticed that the first Ping is not very good quality data (since it includes 
the handshake).
 * I refactored {{ScheduledExecutors}} so that we can't leak threads out of 
random STPE's (e.g. DES will not break the jvm dtests because it won't leak a 
thread). If you don't like this let me know and I can change the close method 
to be called from {{Instance.java}}
 * DES now has microsecond resolution instead of millisecond, I noticed that in 
my CCM clusters all the nodes have latency 0 (less than a millisecond) which 
also appears true on many of our production clusters.
 * When the DES lacks latency information about a peer we defer to the 
subsnitch instead of scoring it zero (which will essentially prefer it over 
everything), we then rely on the latency probes to find out where it belongs
 * Refactored and payed down a bunch of tech debt with a lot of tests

On my side I'd like to give this more testing using CCM and on a real cluster 
with real cross datacenter latency to ensure the probes are working as 
expected, and I'd like to refactor the unit tests of the probe logic so that we 
don't actually have to hook into the MessagingService (which leads to a bunch 
of semi-gross test code imo)

> DynamicEndpointSnitch should never prefer latent nodes
> ------------------------------------------------------
>
>                 Key: CASSANDRA-14459
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-14459
>             Project: Cassandra
>          Issue Type: Improvement
>          Components: Legacy/Coordination
>            Reporter: Joseph Lynch
>            Assignee: Joseph Lynch
>            Priority: Minor
>              Labels: 4.0-feature-freeze-review-requested, 
> pull-request-available
>             Fix For: 4.x
>
>          Time Spent: 22h 50m
>  Remaining Estimate: 0h
>
> The DynamicEndpointSnitch has two unfortunate behaviors that allow it to 
> provide latent hosts as replicas:
>  # Loses all latency information when Cassandra restarts
>  # Clears latency information entirely every ten minutes (by default), 
> allowing global queries to be routed to _other datacenters_ (and local 
> queries cross racks/azs)
> This means that the first few queries after restart/reset could be quite slow 
> compared to average latencies. I propose we solve this by resetting to the 
> minimum observed latency instead of completely clearing the samples and 
> extending the {{isLatencyForSnitch}} idea to a three state variable instead 
> of two, in particular {{YES}}, {{NO}}, {{MAYBE}}. This extension allows 
> {{EchoMessages}} and {{PingMessages}} to send {{MAYBE}} indicating that the 
> DS should use those measurements if it only has one or fewer samples for a 
> host. This fixes both problems because on process restart we send out 
> {{PingMessages}} / {{EchoMessages}} as part of startup, and we would reset to 
> effectively the RTT of the hosts (also at that point normal gossip 
> {{EchoMessages}} have an opportunity to add an additional latency 
> measurement).
> This strategy also nicely deals with the "a host got slow but now it's fine" 
> problem that the DS resets were (afaik) designed to stop because the 
> {{EchoMessage}} ping latency will count only after the reset for that host. 
> Ping latency is a more reasonable lower bound on host latency (as opposed to 
> status quo of zero).



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to