ekaterinadimitrova2 commented on a change in pull request #870:
URL: https://github.com/apache/cassandra/pull/870#discussion_r572350596
##########
File path: src/java/org/apache/cassandra/dht/RangeStreamer.java
##########
@@ -17,7 +17,14 @@
*/
package org.apache.cassandra.dht;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Properties;
Review comment:
Unused import
##########
File path:
src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
##########
@@ -75,8 +85,16 @@ public Keyspaces apply(Keyspaces schema)
throw ire("Unable to use given strategy class: LocalStrategy is
reserved for internal use.");
keyspace.params.validate(keyspaceName);
+ Keyspaces ksps = schema.withAddedOrUpdated(keyspace);
Review comment:
nit: I would name this keyspaces
##########
File path:
test/unit/org/apache/cassandra/cql3/validation/operations/AlterTest.java
##########
@@ -269,6 +276,63 @@ public void testCreateAlterKeyspaces() throws Throwable
"max_threshold", "32")));
}
+ @Test
+ public void testCreateAlterKeyspacesRFWarnings() throws Throwable
+ {
+ requireNetwork();
+ Thread.sleep(1000); // Hack: requireNetwork() will clear warnings
async so give some time to complete
Review comment:
I am also in favor to the polling. Less prone to flakiness even if it is
not perfect
##########
File path:
src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
##########
@@ -75,8 +85,16 @@ public Keyspaces apply(Keyspaces schema)
throw ire("Unable to use given strategy class: LocalStrategy is
reserved for internal use.");
keyspace.params.validate(keyspaceName);
+ Keyspaces ksps = schema.withAddedOrUpdated(keyspace);
- return schema.withAddedOrUpdated(keyspace);
+ if (ClientWarn.instance.getWarnings() != null)
Review comment:
Was the idea revised again? :-)
##########
File path:
src/java/org/apache/cassandra/cql3/statements/schema/CreateKeyspaceStatement.java
##########
@@ -56,6 +62,10 @@ public CreateKeyspaceStatement(String keyspaceName,
KeyspaceAttributes attrs, bo
public Keyspaces apply(Keyspaces schema)
{
+ if (ClientWarn.instance.get() == null)
+ ClientWarn.instance.captureWarnings();
+ int previousWarningsSize = ClientWarn.instance.getWarnings() == null ?
0 : ClientWarn.instance.getWarnings().size();
Review comment:
nit: I was wondering what happened to this suggestion? I see similar one
in AlterKeyspaceStatement and it seems reasonable to me and in fashion with the
C* code style :-)
##########
File path: src/java/org/apache/cassandra/locator/TokenMetadata.java
##########
@@ -1331,6 +1331,14 @@ public EndpointsForToken getWriteEndpoints(Token token,
String keyspaceName, End
}
}
+ /**
+ * @return a (stable copy, won't be modified) datacenter to Endpoint map
for all the nodes in the cluster.
+ */
Review comment:
nit: I would probably say "an unmodifiable Data Center to Endpoint
map... "
Decided to mention it only as it will appear in the java doc, feel free to
ignore :-)
##########
File path: test/unit/org/apache/cassandra/schema/CreateTableValidationTest.java
##########
@@ -72,7 +69,7 @@ public void testCreateKeyspaceTableWarning() throws
IOException
try (SimpleClient client =
newSimpleClient(ProtocolVersion.CURRENT).connect(false))
{
- String createKeyspace = "CREATE KEYSPACE createkswarning%d WITH
REPLICATION={'class':'org.apache.cassandra.locator.NetworkTopologyStrategy','datacenter1':'2'}";
+ String createKeyspace = "CREATE KEYSPACE createkswarning%d WITH
REPLICATION={'class':'org.apache.cassandra.locator.NetworkTopologyStrategy','datacenter1':'1'}";
Review comment:
Do we really want this change?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
