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 2022/06/07 11:07:28 UTC
[GitHub] [cassandra] adelapena commented on a diff in pull request #1662: CASSANDRA-17584
adelapena commented on code in PR #1662:
URL: https://github.com/apache/cassandra/pull/1662#discussion_r891077216
##########
src/java/org/apache/cassandra/db/virtual/GossipInfoTable.java:
##########
@@ -76,10 +89,10 @@ final class GossipInfoTable extends AbstractVirtualTable
public DataSet data()
{
SimpleDataSet result = new SimpleDataSet(metadata());
- for (Map.Entry<InetAddressAndPort, EndpointState> entry : Gossiper.instance.endpointStateMap.entrySet())
+ for (Map.Entry<InetAddressAndPort, EndpointState> entry : endpointStateMapSupplier.get().entrySet())
{
InetAddressAndPort endpoint = entry.getKey();
- EndpointState localState = entry.getValue();
+ EndpointState localState = new EndpointState(entry.getValue());
Review Comment:
I think we don't need to create a new `EndpointState` here, the `EndpointState` is already copied by the `testSelectAllWithStateTransitions` test.
##########
src/java/org/apache/cassandra/db/virtual/GossipInfoTable.java:
##########
@@ -19,8 +19,12 @@
package org.apache.cassandra.db.virtual;
import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.HashSet;
Review Comment:
Nit: unused imports
##########
test/unit/org/apache/cassandra/db/virtual/GossipInfoTableTest.java:
##########
@@ -33,64 +36,68 @@
import org.apache.cassandra.gms.Gossiper;
import org.apache.cassandra.locator.InetAddressAndPort;
+import static com.google.common.collect.ImmutableList.of;
import static org.assertj.core.api.Assertions.assertThat;
public class GossipInfoTableTest extends CQLTester
{
- private static final String KS_NAME = "vts";
-
- @SuppressWarnings("FieldCanBeLocal")
- private GossipInfoTable table;
-
@BeforeClass
public static void setUpClass()
{
CQLTester.setUpClass();
}
- @Before
- public void config()
- {
- table = new GossipInfoTable(KS_NAME);
- VirtualKeyspaceRegistry.instance.register(new VirtualKeyspace(KS_NAME, ImmutableList.of(table)));
- }
-
@Test
public void testSelectAllWhenGossipInfoIsEmpty() throws Throwable
{
- assertEmpty(execute("SELECT * FROM vts.gossip_info"));
+ // we have not triggered gossiper yet
+ VirtualKeyspaceRegistry.instance.register(new VirtualKeyspace("vts_1",
+ of(new GossipInfoTable("vts_1", HashMap::new))));
+ assertEmpty(execute("SELECT * FROM vts_1.gossip_info"));
}
- @SuppressWarnings("deprecation")
@Test
public void testSelectAllWithStateTransitions() throws Throwable
{
try
{
requireNetwork(); // triggers gossiper
- UntypedResultSet resultSet = execute("SELECT * FROM vts.gossip_info");
+ final AtomicReference<Map<InetAddressAndPort, EndpointState>> stateRef = new AtomicReference<>(null);
+
+ Awaitility.await().until(() -> {
+ ConcurrentMap<InetAddressAndPort, EndpointState> endpointStateMap = Gossiper.instance.endpointStateMap;
+ stateRef.set(endpointStateMap);
+ return endpointStateMap.size() == 1;
+ });
+
+ Optional<Map.Entry<InetAddressAndPort, EndpointState>> entry = stateRef.get().entrySet().stream().findFirst();
+ assertThat(entry).isNotEmpty();
+
+ InetAddressAndPort endpoint = entry.get().getKey();
+ EndpointState localState = new EndpointState(entry.get().getValue());
Review Comment:
This might be simplified to:
```java
ConcurrentMap<InetAddressAndPort, EndpointState> states = Gossiper.instance.endpointStateMap;
Awaitility.await().until(() -> !states.isEmpty());
Map.Entry<InetAddressAndPort, EndpointState> entry = states.entrySet().stream().findFirst()
.orElseThrow(AssertionError::new);
InetAddressAndPort endpoint = entry.getKey();
EndpointState localState = new EndpointState(entry.getValue());
```
##########
test/unit/org/apache/cassandra/db/virtual/GossipInfoTableTest.java:
##########
@@ -33,64 +36,68 @@
import org.apache.cassandra.gms.Gossiper;
import org.apache.cassandra.locator.InetAddressAndPort;
+import static com.google.common.collect.ImmutableList.of;
import static org.assertj.core.api.Assertions.assertThat;
public class GossipInfoTableTest extends CQLTester
{
- private static final String KS_NAME = "vts";
-
- @SuppressWarnings("FieldCanBeLocal")
- private GossipInfoTable table;
-
@BeforeClass
public static void setUpClass()
{
CQLTester.setUpClass();
}
- @Before
- public void config()
- {
- table = new GossipInfoTable(KS_NAME);
- VirtualKeyspaceRegistry.instance.register(new VirtualKeyspace(KS_NAME, ImmutableList.of(table)));
- }
-
@Test
public void testSelectAllWhenGossipInfoIsEmpty() throws Throwable
{
- assertEmpty(execute("SELECT * FROM vts.gossip_info"));
+ // we have not triggered gossiper yet
+ VirtualKeyspaceRegistry.instance.register(new VirtualKeyspace("vts_1",
+ of(new GossipInfoTable("vts_1", HashMap::new))));
+ assertEmpty(execute("SELECT * FROM vts_1.gossip_info"));
}
- @SuppressWarnings("deprecation")
@Test
public void testSelectAllWithStateTransitions() throws Throwable
{
try
{
requireNetwork(); // triggers gossiper
- UntypedResultSet resultSet = execute("SELECT * FROM vts.gossip_info");
+ final AtomicReference<Map<InetAddressAndPort, EndpointState>> stateRef = new AtomicReference<>(null);
+
+ Awaitility.await().until(() -> {
+ ConcurrentMap<InetAddressAndPort, EndpointState> endpointStateMap = Gossiper.instance.endpointStateMap;
+ stateRef.set(endpointStateMap);
+ return endpointStateMap.size() == 1;
+ });
+
+ Optional<Map.Entry<InetAddressAndPort, EndpointState>> entry = stateRef.get().entrySet().stream().findFirst();
+ assertThat(entry).isNotEmpty();
Review Comment:
This check is duplicated a few lines below.
--
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: pr-unsubscribe@cassandra.apache.org
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