dcapwell commented on code in PR #3157:
URL: https://github.com/apache/cassandra/pull/3157#discussion_r1511826671
##########
test/unit/org/apache/cassandra/service/accord/AccordKeyspaceTest.java:
##########
@@ -97,4 +134,157 @@ public void serde()
Command loaded = AccordKeyspace.loadCommand(store, id);
Assertions.assertThat(loaded).isEqualTo(committed);
}
+
+ @Test
+ public void findOverlappingKeys()
+ {
+ var tableIdGen = fromQT(CassandraGenerators.TABLE_ID_GEN);
+ var partitionGen = fromQT(CassandraGenerators.partitioners());
+
+ var sstableFormats = DatabaseDescriptor.getSSTableFormats();
+ List<String> sstableFormatNames = new
ArrayList<>(sstableFormats.keySet());
+ sstableFormatNames.sort(Comparator.naturalOrder());
+
+ List<String> memtableFormats =
MemtableParams.knownDefinitions().stream()
+ .filter(name ->
!name.startsWith("test_") && !name.equals("default"))
+ .sorted()
+
.collect(Collectors.toList());
+
+ qt().check(rs -> {
+ AccordKeyspace.unsafeClear();
+ // control SSTable format
+
setSelectedSSTableFormat(sstableFormats.get(rs.pick(sstableFormatNames)));
+ // control memtable format
+ setMemtable(ACCORD_KEYSPACE_NAME, "commands_for_key",
rs.pick(memtableFormats));
+
+ // define the tables w/ partitioners for the test
+ // this uses the ability to override the SchemaProvider for the
keyspace and only defines the single API call expected: getTablePartitioner
+ TreeMap<TableId, IPartitioner> tables = new TreeMap<>();
+ int numTables = rs.nextInt(1, 3);
+ for (int i = 0; i < numTables; i++)
+ {
+ var tableId = tableIdGen.next(rs);
+ while (tables.containsKey(tableId))
+ tableId = tableIdGen.next(rs);
+ tables.put(tableId, partitionGen.next(rs));
+ }
+ SchemaProvider schema = Mockito.mock(SchemaProvider.class);
+
Mockito.when(schema.getTablePartitioner(Mockito.any())).thenAnswer((Answer<IPartitioner>)
invocationOnMock -> tables.get(invocationOnMock.getArgument(0)));
+ AccordKeyspace.unsafeSetSchema(schema);
+
+ int numStores = rs.nextInt(1, 3);
+
+ // The model of the DB
+ TreeMap<Integer, SortedSet<PartitionKey>> storesToKeys = new
TreeMap<>();
+ // write to the table and the model
+ for (int i = 0, numKeys = rs.nextInt(10, 20); i < numKeys; i++)
+ {
+ int store = rs.nextInt(0, numStores);
+ var keys = storesToKeys.computeIfAbsent(store, ignore -> new
TreeSet<>());
+ PartitionKey pk = null;
+ // LocalPartitioner may have a type with a very small domain
(boolean, vector<boolean, 1>, etc.), so need to bound the attempts
+ // else this will loop forever...
+ for (int attempt = 0; attempt < 10; attempt++)
+ {
+ TableId tableId = rs.pick(tables.keySet());
+ IPartitioner partitioner = tables.get(tableId);
+ ByteBuffer data = !(partitioner instanceof
LocalPartitioner) ? Int32Type.instance.decompose(rs.nextInt())
+
: fromQT(getTypeSupport(partitioner.getTokenValidator()).bytesGen()).next(rs);
+ PartitionKey key = new PartitionKey(tableId,
tables.get(tableId).decorateKey(data));
+ if (keys.add(key))
+ {
+ pk = key;
+ break;
+ }
+ }
+ if (pk != null)
+ {
+ try
+ {
+ // using Mutation directly (what we do in Accord) can
break when user data is too large; leading to data loss
+ // The memtable will allow the write, but it will be
dropped when writing to the SSTable...
+ //TODO (now, correctness): since we store the user
token + user key, if a key is close to the PK limits then we could tip over and
loose our CFK
+// new
Mutation(AccordKeyspace.getCommandsForKeyPartitionUpdate(store, pk, 42,
ByteBufferUtil.EMPTY_BYTE_BUFFER)).apply();
Review Comment:
this is the logic CFK uses, but this can cause problems down the road. For
this test these keys are filtered out and CQL is used directly to avoid these
issues...
There is a task to fix CFK to not have size limits that different than
normal CQL
--
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]
