You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/05/06 06:51:35 UTC
[GitHub] [cassandra] nastra commented on a change in pull request #575: CASSANDRA-15776 port nodetool_test.py test_refresh_size_estimates_clears_invalid_entries to jvm dtest and fixed bug where size_estimates isn't used for clearing tables
nastra commented on a change in pull request #575:
URL: https://github.com/apache/cassandra/pull/575#discussion_r420574041
##########
File path: src/java/org/apache/cassandra/db/SystemKeyspace.java
##########
@@ -1344,12 +1344,17 @@ public static void clearEstimates(String keyspace)
public static synchronized SetMultimap<String, String> getTablesWithSizeEstimates()
{
SetMultimap<String, String> keyspaceTableMap = HashMultimap.create();
- String cql = String.format("SELECT keyspace_name, table_name FROM %s", TableEstimates.toString(), TABLE_ESTIMATES_TYPE_PRIMARY);
- UntypedResultSet rs = executeInternal(cql);
- for (UntypedResultSet.Row row : rs)
+ // Its possible that size_estimates knows about a different set of keyspace/tables than table_estimates (mostly
+ // caused by external systems modifying the tables, such as dtest) so query both
+ for (String cql : Arrays.asList(
+ "SELECT keyspace_name, table_name FROM " + TableEstimates.toString(),
+ "SELECT keyspace_name, table_name FROM " + LegacySizeEstimates.toString()))
Review comment:
I believe we have to do 2 table scans now until the table `size_estimates` goes away.
##########
File path: src/java/org/apache/cassandra/db/SystemKeyspace.java
##########
@@ -1344,12 +1344,17 @@ public static void clearEstimates(String keyspace)
public static synchronized SetMultimap<String, String> getTablesWithSizeEstimates()
{
SetMultimap<String, String> keyspaceTableMap = HashMultimap.create();
- String cql = String.format("SELECT keyspace_name, table_name FROM %s", TableEstimates.toString(), TABLE_ESTIMATES_TYPE_PRIMARY);
- UntypedResultSet rs = executeInternal(cql);
- for (UntypedResultSet.Row row : rs)
+ // Its possible that size_estimates knows about a different set of keyspace/tables than table_estimates (mostly
+ // caused by external systems modifying the tables, such as dtest) so query both
+ for (String cql : Arrays.asList(
+ "SELECT keyspace_name, table_name FROM " + TableEstimates.toString(),
+ "SELECT keyspace_name, table_name FROM " + LegacySizeEstimates.toString()))
{
- keyspaceTableMap.put(row.getString("keyspace_name"), row.getString("table_name"));
+ UntypedResultSet rs = executeInternal(cql);
+ for (UntypedResultSet.Row row : rs)
+ keyspaceTableMap.put(row.getString("keyspace_name"), row.getString("table_name"));
Review comment:
are we potentially overwriting estimates that we read from `TABLE_ESTIMATES` with estimates that we read from `LEGACY_SIZE_ESTIMATES` on purpose?
##########
File path: src/java/org/apache/cassandra/db/SystemKeyspace.java
##########
@@ -1344,12 +1344,17 @@ public static void clearEstimates(String keyspace)
public static synchronized SetMultimap<String, String> getTablesWithSizeEstimates()
{
SetMultimap<String, String> keyspaceTableMap = HashMultimap.create();
- String cql = String.format("SELECT keyspace_name, table_name FROM %s", TableEstimates.toString(), TABLE_ESTIMATES_TYPE_PRIMARY);
- UntypedResultSet rs = executeInternal(cql);
- for (UntypedResultSet.Row row : rs)
+ // Its possible that size_estimates knows about a different set of keyspace/tables than table_estimates (mostly
+ // caused by external systems modifying the tables, such as dtest) so query both
+ for (String cql : Arrays.asList(
+ "SELECT keyspace_name, table_name FROM " + TableEstimates.toString(),
+ "SELECT keyspace_name, table_name FROM " + LegacySizeEstimates.toString()))
Review comment:
nit: imo it would be more obvious to use `SchemaConstants.SYSTEM_KEYSPACE_NAME, TABLE_ESTIMATES` / `SchemaConstants.SYSTEM_KEYSPACE_NAME, LEGACY_SIZE_ESTIMATES` in the constructed CQL query than using `TableEstimates.toString()` / `LegacySizeEstimates.toString()`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org