belliottsmith commented on code in PR #50:
URL: https://github.com/apache/cassandra-accord/pull/50#discussion_r1240845089


##########
accord-core/src/test/java/accord/topology/TopologyManagerTest.java:
##########
@@ -191,13 +191,76 @@ void forKeysPartiallySynced()
 
         // no acks, so all epoch 1 shards should be included
         Assertions.assertEquals(topologies(topology2, topology1),
-                                service.withUnsyncedEpochs(keys(150, 
250).toUnseekables(), 2, 2));
+                                service.withUnsyncedEpochs(keys(150, 
250).toParticipants(), 2, 2));
 
         // first topology acked, so only the second shard should be included
-        service.onEpochSyncComplete(id(1), 1);
-        service.onEpochSyncComplete(id(2), 1);
-        Topologies actual = service.withUnsyncedEpochs(keys(150, 
250).toUnseekables(), 2, 2);
+        service.onEpochSyncComplete(id(1), 2);
+        service.onEpochSyncComplete(id(2), 2);
+        Topologies actual = service.withUnsyncedEpochs(keys(150, 
250).toParticipants(), 2, 2);
         Assertions.assertEquals(topologies(topology2, topology(1, 
shard(range(200, 300), idList(4, 5, 6), idSet(4, 5)))),
                                 actual);
     }
+
+    @Test
+    void incompleteTopologyHistory()
+    {
+        Topology topology5 = topology(5,
+                                      shard(range(100, 200), idList(1, 2, 3), 
idSet(1, 2)),
+                                      shard(range(200, 300), idList(4, 5, 6), 
idSet(4, 5)));
+        Topology topology6 = topology(6,
+                                      shard(range(100, 200), idList(1, 2, 3), 
idSet(1, 2)),
+                                      shard(range(200, 300), idList(4, 5, 6), 
idSet(5, 6)));
+
+        TopologyManager service = new TopologyManager(SUPPLIER, ID);
+        service.onTopologyUpdate(topology5);
+        service.onTopologyUpdate(topology6);
+
+        Assertions.assertSame(topology6, 
service.getEpochStateUnsafe(6).global());
+        Assertions.assertSame(topology5, 
service.getEpochStateUnsafe(5).global());
+        for (int i=1; i<=6; i++) service.onEpochSyncComplete(id(i), 6);
+        Assertions.assertTrue(service.getEpochStateUnsafe(5).syncComplete());
+        Assertions.assertNull(service.getEpochStateUnsafe(4));
+
+        service.onEpochSyncComplete(id(1), 4);
+    }
+
+    private static void markTopologySynced(TopologyManager service, long epoch)
+    {
+        service.getEpochStateUnsafe(epoch).global().nodes().forEach(id -> 
service.onEpochSyncComplete(id, epoch));
+    }
+
+    private static void addAndMarkSynced(TopologyManager service, Topology 
topology)
+    {
+        service.onTopologyUpdate(topology);
+        markTopologySynced(service, topology.epoch());
+    }
+
+    @Test
+    void truncateTopologyHistory()
+    {
+        Range range = range(100, 200);
+        TopologyManager service = new TopologyManager(SUPPLIER, ID);
+        addAndMarkSynced(service, topology(1, shard(range, idList(1, 2, 3), 
idSet(1, 2))));
+        addAndMarkSynced(service, topology(2, shard(range, idList(1, 2, 3), 
idSet(2, 3))));
+        addAndMarkSynced(service, topology(3, shard(range, idList(1, 2, 3), 
idSet(1, 2))));
+        addAndMarkSynced(service, topology(4, shard(range, idList(1, 2, 3), 
idSet(1, 3))));
+
+        Assertions.assertTrue(service.hasEpoch(1));
+        Assertions.assertTrue(service.hasEpoch(2));
+        Assertions.assertTrue(service.hasEpoch(3));
+        Assertions.assertTrue(service.hasEpoch(4));
+
+        service.truncateTopologyUntil(3);
+        Assertions.assertFalse(service.hasEpoch(1));
+        Assertions.assertFalse(service.hasEpoch(2));
+        Assertions.assertTrue(service.hasEpoch(3));
+        Assertions.assertTrue(service.hasEpoch(4));
+
+    }
+
+    @Test
+    void truncateTopologyCantTruncateUnsyncedEpochs()
+    {
+

Review Comment:
   I think this is a bad rebase, as the name of this method isn't related to 
this patch - it's related to _topology_ truncation, which was introduced as 
part of TCM integration. It is possible I intended to write a test, or this is 
some vestige I adopted in a rebase. I'm not sure. We probably should have this 
test, but it's not really in scope for this patch set. I'll see if I have time 
to flesh it out, but I make no promises.



-- 
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]

Reply via email to