(improvement)TokenAwarePolicy::make_query_plan(): remove redundant check if a table is using tablets

table_has_tablets() performs the same self._tablets.get((keyspace, table) that
get_tablet_for_key() does which is a called a line later does, so it's redundant.
Removed the former.

Note - perhaps table_has_tablets() is not needed and can be removed? Unsure, it's unclear
if it's part of the API or not. It's now completely unused across the code.

Adjusted unit tests as well.

Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>

Author: mykaul

cassandra/policies.py

@@ -503,15 +503,14 @@ def make_query_plan(self, working_keyspace=None, query=None):
             return
 
         replicas = []
-        if self._cluster_metadata._tablets.table_has_tablets(keyspace, query.table):
-            tablet = self._cluster_metadata._tablets.get_tablet_for_key(
+        tablet = self._cluster_metadata._tablets.get_tablet_for_key(
             keyspace, query.table, self._cluster_metadata.token_map.token_class.from_key(query.routing_key))
 
-            if tablet is not None:
-                replicas_mapped = set(map(lambda r: r[0], tablet.replicas))
-                child_plan = child.make_query_plan(keyspace, query)
+        if tablet is not None:
+            replicas_mapped = set(map(lambda r: r[0], tablet.replicas))
+            child_plan = child.make_query_plan(keyspace, query)
 
-                replicas = [host for host in child_plan if host.host_id in replicas_mapped]
+            replicas = [host for host in child_plan if host.host_id in replicas_mapped]
         else:
             replicas = self._cluster_metadata.get_replicas(keyspace, query.routing_key)
 

tests/unit/test_policies.py

@@ -576,7 +576,7 @@ def test_wrap_round_robin(self):
         cluster = Mock(spec=Cluster)
         cluster.metadata = Mock(spec=Metadata)
         cluster.metadata._tablets = Mock(spec=Tablets)
-        cluster.metadata._tablets.table_has_tablets.return_value = []
+        cluster.metadata._tablets.get_tablet_for_key.return_value = None
         hosts = [Host(DefaultEndPoint(str(i)), SimpleConvictionPolicy, host_id=uuid.uuid4()) for i in range(4)]
         for host in hosts:
             host.set_up()
@@ -609,7 +609,7 @@ def test_wrap_dc_aware(self):
         cluster = Mock(spec=Cluster)
         cluster.metadata = Mock(spec=Metadata)
         cluster.metadata._tablets = Mock(spec=Tablets)
-        cluster.metadata._tablets.table_has_tablets.return_value = []
+        cluster.metadata._tablets.get_tablet_for_key.return_value = None
         hosts = [Host(DefaultEndPoint(str(i)), SimpleConvictionPolicy, host_id=uuid.uuid4()) for i in range(4)]
         for host in hosts:
             host.set_up()
@@ -658,7 +658,7 @@ def test_wrap_rack_aware(self):
         cluster = Mock(spec=Cluster)
         cluster.metadata = Mock(spec=Metadata)
         cluster.metadata._tablets = Mock(spec=Tablets)
-        cluster.metadata._tablets.table_has_tablets.return_value = []
+        cluster.metadata._tablets.get_tablet_for_key.return_value = None
         hosts = [Host(DefaultEndPoint(str(i)), SimpleConvictionPolicy, host_id=uuid.uuid4()) for i in range(8)]
         for host in hosts:
             host.set_up()
@@ -803,7 +803,7 @@ def test_statement_keyspace(self):
         cluster.metadata._tablets = Mock(spec=Tablets)
         replicas = hosts[2:]
         cluster.metadata.get_replicas.return_value = replicas
-        cluster.metadata._tablets.table_has_tablets.return_value = []
+        cluster.metadata._tablets.get_tablet_for_key.return_value = None
 
         child_policy = Mock()
         child_policy.make_query_plan.return_value = hosts
@@ -896,7 +896,7 @@ def _prepare_cluster_with_vnodes(self):
         cluster.metadata._tablets = Mock(spec=Tablets)
         cluster.metadata.all_hosts.return_value = hosts
         cluster.metadata.get_replicas.return_value = hosts[2:]
-        cluster.metadata._tablets.table_has_tablets.return_value = False
+        cluster.metadata._tablets.get_tablet_for_key.return_value = None
         return cluster
 
     def _prepare_cluster_with_tablets(self):
@@ -908,7 +908,6 @@ def _prepare_cluster_with_tablets(self):
         cluster.metadata._tablets = Mock(spec=Tablets)
         cluster.metadata.all_hosts.return_value = hosts
         cluster.metadata.get_replicas.return_value = hosts[2:]
-        cluster.metadata._tablets.table_has_tablets.return_value = True
         cluster.metadata._tablets.get_tablet_for_key.return_value = Tablet(replicas=[(h.host_id, 0) for h in hosts[2:]])
         return cluster
 
@@ -923,7 +922,7 @@ def _assert_shuffle(self, patched_shuffle, cluster, keyspace, routing_key):
         policy = TokenAwarePolicy(child_policy, shuffle_replicas=True)
         policy.populate(cluster, hosts)
 
-        is_tablets = cluster.metadata._tablets.table_has_tablets()
+        is_tablets = cluster.metadata._tablets.get_tablet_for_key() is not None
 
         cluster.metadata.get_replicas.reset_mock()
         child_policy.make_query_plan.reset_mock()
@@ -1630,7 +1629,7 @@ def get_replicas(keyspace, packed_key):
 
         cluster.metadata.get_replicas.side_effect = get_replicas
         cluster.metadata._tablets = Mock(spec=Tablets)
-        cluster.metadata._tablets.table_has_tablets.return_value = []
+        cluster.metadata._tablets.get_tablet_for_key.return_value = None
 
         child_policy = TokenAwarePolicy(RoundRobinPolicy())