yifan-c commented on code in PR #4601:
URL: https://github.com/apache/cassandra/pull/4601#discussion_r2785625292
##########
src/java/org/apache/cassandra/schema/SystemDistributedKeyspace.java:
##########
@@ -199,13 +200,14 @@ private SystemDistributedKeyspace()
public static final String COMPRESSION_DICTIONARIES_CQL = "CREATE TABLE IF
NOT EXISTS %s (" +
"keyspace_name
text," +
"table_name
text," +
+ "table_id text,"
+
"kind text," +
"dict_id
bigint," +
"dict blob," +
"dict_length
int," +
"dict_checksum
int," +
- "PRIMARY KEY
((keyspace_name, table_name), dict_id)) " +
- "WITH CLUSTERING
ORDER BY (dict_id DESC)"; // in order to retrieve the latest dictionary; the
contract is the newer the dictionary the larger the dict_id
+ "PRIMARY KEY
((keyspace_name, table_name), table_id, dict_id)) " +
+ "WITH CLUSTERING
ORDER BY (table_id DESC, dict_id DESC)"; // in order to retrieve the latest
dictionary; the contract is the newer the dictionary the larger the dict_id
Review Comment:
Should `table_id` be part of the partition key? (instead of being a
clustering key).
`dict_id` is used to order the dictionaries for the same table. The
`table_id` is not useful for the purpose.
##########
src/java/org/apache/cassandra/schema/SystemDistributedKeyspace.java:
##########
@@ -512,17 +522,87 @@ public static CompressionDictionary
retrieveCompressionDictionary(String keyspac
*
* @param keyspaceName the keyspace name to retrieve the dictionary for
* @param tableName the table name to retrieve the dictionary for
+ * @param tableId the table id to retrieve the dictionary for
* @return the compression dictionaries identified by the specified
keyspace and table,
- * or null if no dictionary exists or if an error occurs during
retrieval
+ * empty list if no dictionary exists or null if an error occurs
during retrieval
+ */
+ @Nullable
+ public static List<LightweightCompressionDictionary>
retrieveLightweightCompressionDictionaries(String keyspaceName, String
tableName, String tableId)
+ {
+ String query = "SELECT keyspace_name, table_name, table_id, kind,
dict_id, dict_length, dict_checksum FROM %s.%s WHERE keyspace_name='%s' AND
table_name='%s' AND table_id='%s'";
+ String fmtQuery = format(query,
SchemaConstants.DISTRIBUTED_KEYSPACE_NAME, COMPRESSION_DICTIONARIES,
keyspaceName, tableName, tableId);
+ return retrieveLightweightCompressionDictionariesInternal(fmtQuery);
+ }
+
+ /**
+ * Retrieves all compression dictionaries in a lightweight form.
+ *
+ * @return all compression dictionaries, lightweight form, empty list if
no dictionary exists
+ * or null if an error occurs during retrieval
*/
@Nullable
- public static List<LightweightCompressionDictionary>
retrieveLightweightCompressionDictionaries(String keyspaceName, String
tableName)
+ public static List<LightweightCompressionDictionary>
retrieveLightweightCompressionDictionaries()
Review Comment:
Is it only called by `retrieveOrphanedLightweightCompressionDictionaries`.
If so, should they consolidate?
##########
src/java/org/apache/cassandra/tools/nodetool/CompressionDictionaryCommandGroup.java:
##########
@@ -216,40 +219,43 @@ public static class ListDictionaries extends
AbstractCommand
@Override
protected void execute(NodeProbe probe)
{
- try
- {
- TableBuilder tableBuilder = new TableBuilder();
- TabularData tabularData =
probe.listCompressionDictionaries(keyspace, table);
- List<String> indexNames =
tabularData.getTabularType().getIndexNames();
-
- List<String> columns = new ArrayList<>(indexNames);
- // ignore raw dict
-
columns.remove(CompressionDictionaryDetailsTabularData.TABULAR_DATA_TYPE_RAW_DICTIONARY_INDEX);
- tableBuilder.add(columns);
-
- for (Object eachDict : tabularData.keySet())
- {
- final List<?> dictRow = (List<?>) eachDict;
-
- List<String> rowValues = new ArrayList<>();
-
- for (int i = 0; i < dictRow.size(); i++)
- {
- // ignore raw dict
- if (i ==
CompressionDictionaryDetailsTabularData.TABULAR_DATA_TYPE_RAW_DICTIONARY_INDEX)
- continue;
+ ListingHelper.list(probe,
+ () ->
+ {
+ try
+ {
+ return
probe.listCompressionDictionaries(keyspace, table);
+ }
+ catch (Throwable t)
+ {
+ throw new RuntimeException(t);
+ }
+ },
+
List.of(CompressionDictionaryDetailsTabularData.TABULAR_DATA_TYPE_TABLE_ID_INDEX,
+
CompressionDictionaryDetailsTabularData.TABULAR_DATA_TYPE_RAW_DICTIONARY_INDEX));
+ }
+ }
- rowValues.add(dictRow.get(i).toString());
- }
- tableBuilder.add(rowValues);
- }
+ @Command(name = "cleanup", description = "Clean up orphaned dictionaries
by deleting them from " + SystemDistributedKeyspace.NAME
+ + '.' +
SystemDistributedKeyspace.COMPRESSION_DICTIONARIES +
+ " table, these are ones for which
a table they were trained for was dropped.")
+ public static class CleanupDictionaries extends AbstractCommand
Review Comment:
How about `CleanupOrphanedDictionaries`? Just to be super clear
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
