You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Sylvain Lebresne (Created) (JIRA)" <ji...@apache.org> on 2011/09/27 11:50:11 UTC
[jira] [Created] (CASSANDRA-3262) SimpleSnitch.compareEndpoints
doesn't respect the intent of the snitch
SimpleSnitch.compareEndpoints doesn't respect the intent of the snitch
----------------------------------------------------------------------
Key: CASSANDRA-3262
URL: https://issues.apache.org/jira/browse/CASSANDRA-3262
Project: Cassandra
Issue Type: Bug
Components: Core
Affects Versions: 0.8.6
Reporter: Sylvain Lebresne
Assignee: Sylvain Lebresne
Priority: Minor
Fix For: 0.8.7, 1.0.0
SimpleSnitch is supposed to not sort the input addresses, thus respecting the order of the partitioner. However, it's compareEndpoints instead uses IP addresses comparison. Note that this matter when the dynamicSnitch fall back to the wrapped snitch since it uses the compareEndpoint method then.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (CASSANDRA-3262) SimpleSnitch.compareEndpoints
doesn't respect the intent of the snitch
Posted by "Sylvain Lebresne (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3262?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Sylvain Lebresne updated CASSANDRA-3262:
----------------------------------------
Attachment: 3262.patch
Note that I refactor a bit the code because it felt weird to have compareEndpoint being concrete in AbstractEndpointSnitch but sortByProximity being abstract. It seems more natural to have compareEndpoint being the abstract method and sortByProximity being defined from it in AES.
> SimpleSnitch.compareEndpoints doesn't respect the intent of the snitch
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-3262
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3262
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 0.8.6
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Minor
> Fix For: 0.8.7, 1.0.0
>
> Attachments: 3262.patch
>
>
> SimpleSnitch is supposed to not sort the input addresses, thus respecting the order of the partitioner. However, it's compareEndpoints instead uses IP addresses comparison. Note that this matter when the dynamicSnitch fall back to the wrapped snitch since it uses the compareEndpoint method then.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-3262) SimpleSnitch.compareEndpoints
doesn't respect the intent of the snitch
Posted by "Sylvain Lebresne (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13115511#comment-13115511 ]
Sylvain Lebresne commented on CASSANDRA-3262:
---------------------------------------------
I think there is a misunderstanding. I agree with the pinning of replicas. The problem is that the current implementation of AbstractEndpointSnitch.compareEndpoints is:
{noformat}
public int compareEndpoints(InetAddress target, InetAddress a1, InetAddress a2)
{
return a1.getHostAddress().compareTo(a2.getHostAddress());
}
{noformat}
If you sort a list of hosts using that, you will always return the host that have the "smallest" IP. In other words, in a 3 nodes cluster with RF=3, all and every read will hit the exact same node.
What makes it kind of work today is that this compareEndpoints() method is barely used. It's used only in the case where the dynamic snitch have no scores for the endpoints. Otherwise, it's sortByProximity that is used (which doesn't rely on compareEndpoints -- this is confusing and my patch corrects it). And sortByProximity does *the right thing*, i.e, it doesn't sort the input list since it is supposed to be in token order (which effectively pin one range to every replica).
So the patch here proposes two things:
* If fixes the compareEndpoints method: comparing IP addresses is not a good idea.
* It refactors the code to make sortByProximity use compareEndpoint, to having getting in that situation again.
> SimpleSnitch.compareEndpoints doesn't respect the intent of the snitch
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-3262
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3262
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 0.8.6
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Minor
> Fix For: 0.8.7, 1.0.0
>
> Attachments: 3262.patch
>
>
> SimpleSnitch is supposed to not sort the input addresses, thus respecting the order of the partitioner. However, it's compareEndpoints instead uses IP addresses comparison. Note that this matter when the dynamicSnitch fall back to the wrapped snitch since it uses the compareEndpoint method then.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-3262) SimpleSnitch.compareEndpoints
doesn't respect the intent of the snitch
Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13116163#comment-13116163 ]
Hudson commented on CASSANDRA-3262:
-----------------------------------
Integrated in Cassandra-0.8 #346 (See [https://builds.apache.org/job/Cassandra-0.8/346/])
Keep SimpleSnitch proximity ordering unchanged from what the Strategy generates, as intended
patch by slebresne; reviewed by jbellis for CASSANDRA-3262
jbellis : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1176712
Files :
* /cassandra/branches/cassandra-0.8/CHANGES.txt
* /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/locator/AbstractEndpointSnitch.java
* /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/locator/AbstractNetworkTopologySnitch.java
* /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java
* /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/locator/SimpleSnitch.java
> SimpleSnitch.compareEndpoints doesn't respect the intent of the snitch
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-3262
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3262
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 0.8.6
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Minor
> Fix For: 0.8.7, 1.0.0
>
> Attachments: 3262.patch
>
>
> SimpleSnitch is supposed to not sort the input addresses, thus respecting the order of the partitioner. However, it's compareEndpoints instead uses IP addresses comparison. Note that this matter when the dynamicSnitch fall back to the wrapped snitch since it uses the compareEndpoint method then.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (CASSANDRA-3262) SimpleSnitch.compareEndpoints
doesn't respect the intent of the snitch
Posted by "Jonathan Ellis (Resolved) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3262?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Jonathan Ellis resolved CASSANDRA-3262.
---------------------------------------
Resolution: Fixed
Reviewer: jbellis
lgtm. made some minor updates to comments and committed.
> SimpleSnitch.compareEndpoints doesn't respect the intent of the snitch
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-3262
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3262
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 0.8.6
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Minor
> Fix For: 0.8.7, 1.0.0
>
> Attachments: 3262.patch
>
>
> SimpleSnitch is supposed to not sort the input addresses, thus respecting the order of the partitioner. However, it's compareEndpoints instead uses IP addresses comparison. Note that this matter when the dynamicSnitch fall back to the wrapped snitch since it uses the compareEndpoint method then.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CASSANDRA-3262) SimpleSnitch.compareEndpoints
doesn't respect the intent of the snitch
Posted by "Jonathan Ellis (Commented) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CASSANDRA-3262?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13115458#comment-13115458 ]
Jonathan Ellis commented on CASSANDRA-3262:
-------------------------------------------
The reason for this behavior (CASSANDRA-1314) is because for best performance, normal reads want exactly the opposite behavior of counters: we want to direct reads to the same replica so that the cache stays hot. Put another way, if each replica is serving a distinct range then you get (replica count) times as much cache memory [with CL.ONE and RR off) than if each is getting the full range of requests from different coordinators.
> SimpleSnitch.compareEndpoints doesn't respect the intent of the snitch
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-3262
> URL: https://issues.apache.org/jira/browse/CASSANDRA-3262
> Project: Cassandra
> Issue Type: Bug
> Components: Core
> Affects Versions: 0.8.6
> Reporter: Sylvain Lebresne
> Assignee: Sylvain Lebresne
> Priority: Minor
> Fix For: 0.8.7, 1.0.0
>
> Attachments: 3262.patch
>
>
> SimpleSnitch is supposed to not sort the input addresses, thus respecting the order of the partitioner. However, it's compareEndpoints instead uses IP addresses comparison. Note that this matter when the dynamicSnitch fall back to the wrapped snitch since it uses the compareEndpoint method then.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira