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 2020/09/11 01:33:09 UTC
[GitHub] [cassandra] dcapwell commented on pull request #733: CASSANDRA-15949 Add guardrails to the task that periodically recomputes the global speculative retry threshold.
dcapwell commented on pull request #733:
URL: https://github.com/apache/cassandra/pull/733#issuecomment-690820019
I took a stab and was able to replicate via a unit test with the proposed refactor
```
diff --git a/src/java/org/apache/cassandra/db/Keyspace.java b/src/java/org/apache/cassandra/db/Keyspace.java
index fc4f56f652..a264db92c2 100644
--- a/src/java/org/apache/cassandra/db/Keyspace.java
+++ b/src/java/org/apache/cassandra/db/Keyspace.java
@@ -24,6 +24,7 @@ import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.locks.Lock;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.collect.Iterables;
import org.slf4j.Logger;
@@ -50,6 +51,7 @@ import org.apache.cassandra.schema.Schema;
import org.apache.cassandra.schema.SchemaConstants;
import org.apache.cassandra.schema.TableId;
import org.apache.cassandra.schema.TableMetadata;
+import org.apache.cassandra.schema.TableMetadataProvider;
import org.apache.cassandra.schema.TableMetadataRef;
import org.apache.cassandra.tracing.Tracing;
import org.apache.cassandra.utils.*;
@@ -93,6 +95,7 @@ public class Keyspace
private final KeyspaceWriteHandler writeHandler;
private volatile ReplicationParams replicationParams;
private final KeyspaceRepairManager repairManager;
+ private final TableMetadataProvider schema;
public static final Function<String,Keyspace> keyspaceTransformer = new Function<String, Keyspace>()
{
@@ -121,7 +124,8 @@ public class Keyspace
return open(keyspaceName, Schema.instance, false);
}
- private static Keyspace open(String keyspaceName, Schema schema, boolean loadSSTables)
+ @VisibleForTesting
+ static Keyspace open(String keyspaceName, TableMetadataProvider schema, boolean loadSSTables)
{
Keyspace keyspaceInstance = schema.getKeyspaceInstance(keyspaceName);
@@ -135,7 +139,7 @@ public class Keyspace
if (keyspaceInstance == null)
{
// open and store the keyspace
- keyspaceInstance = new Keyspace(keyspaceName, loadSSTables);
+ keyspaceInstance = new Keyspace(keyspaceName, schema, loadSSTables);
schema.storeKeyspaceInstance(keyspaceInstance);
}
}
@@ -207,7 +211,7 @@ public class Keyspace
public ColumnFamilyStore getColumnFamilyStore(String cfName)
{
- TableMetadata table = Schema.instance.getTableMetadata(getName(), cfName);
+ TableMetadata table = schema.getTableMetadata(getName(), cfName);
if (table == null)
throw new IllegalArgumentException(String.format("Unknown keyspace/cf pair (%s.%s)", getName(), cfName));
return getColumnFamilyStore(table.id);
@@ -324,9 +328,10 @@ public class Keyspace
return list;
}
- private Keyspace(String keyspaceName, boolean loadSSTables)
+ private Keyspace(String keyspaceName, TableMetadataProvider schema, boolean loadSSTables)
{
- metadata = Schema.instance.getKeyspaceMetadata(keyspaceName);
+ this.schema = schema;
+ metadata = schema.getKeyspaceMetadata(keyspaceName);
assert metadata != null : "Unknown keyspace " + keyspaceName;
if (metadata.isVirtual())
throw new IllegalStateException("Cannot initialize Keyspace with virtual metadata " + keyspaceName);
@@ -337,7 +342,18 @@ public class Keyspace
for (TableMetadata cfm : metadata.tablesAndViews())
{
logger.trace("Initializing {}.{}", getName(), cfm.name);
- initCf(Schema.instance.getTableMetadataRef(cfm.id), loadSSTables);
+ TableMetadataRef tableMetadataRef = schema.getTableMetadataRef(cfm.id);
+ if (tableMetadataRef == null)
+ {
+ logger.info("Failed to initialize {}.{}, as no table metadata was available. This " +
+ "has likely occurred because the keyspace metadata for {} has not yet " +
+ "been updated to reflect the table being dropped.",
+ getName(), cfm.name, getName());
+ }
+ else
+ {
+ initCf(tableMetadataRef, loadSSTables);
+ }
}
this.viewManager.reload(false);
@@ -347,6 +363,7 @@ public class Keyspace
private Keyspace(KeyspaceMetadata metadata)
{
+ this.schema = Schema.instance;
this.metadata = metadata;
createReplicationStrategy(metadata);
this.metric = new KeyspaceMetrics(this);
diff --git a/src/java/org/apache/cassandra/schema/Schema.java b/src/java/org/apache/cassandra/schema/Schema.java
index 0498993d9d..48f57bd73f 100644
--- a/src/java/org/apache/cassandra/schema/Schema.java
+++ b/src/java/org/apache/cassandra/schema/Schema.java
@@ -192,6 +192,7 @@ public final class Schema implements TableMetadataProvider
*
* @return Keyspace object or null if keyspace was not found
*/
+ @Override
public Keyspace getKeyspaceInstance(String keyspaceName)
{
return keyspaceInstances.get(keyspaceName);
@@ -219,6 +220,7 @@ public final class Schema implements TableMetadataProvider
*
* @throws IllegalArgumentException if Keyspace is already stored
*/
+ @Override
public void storeKeyspaceInstance(Keyspace keyspace)
{
if (keyspaceInstances.containsKey(keyspace.getName()))
@@ -283,6 +285,7 @@ public final class Schema implements TableMetadataProvider
*
* @return The keyspace metadata or null if it wasn't found
*/
+ @Override
public KeyspaceMetadata getKeyspaceMetadata(String keyspaceName)
{
assert keyspaceName != null;
@@ -356,6 +359,7 @@ public final class Schema implements TableMetadataProvider
*
* @return TableMetadataRef object or null if it wasn't found
*/
+ @Override
public TableMetadataRef getTableMetadataRef(String keyspace, String table)
{
TableMetadata tm = getTableMetadata(keyspace, table);
@@ -381,16 +385,12 @@ public final class Schema implements TableMetadataProvider
*
* @return metadata about Table or View
*/
+ @Override
public TableMetadataRef getTableMetadataRef(TableId id)
{
return metadataRefs.get(id);
}
- public TableMetadataRef getTableMetadataRef(Descriptor descriptor)
- {
- return getTableMetadataRef(descriptor.ksname, descriptor.cfname);
- }
-
Map<TableId, TableMetadataRef> getTableMetadataRefs()
{
return metadataRefs;
@@ -406,6 +406,7 @@ public final class Schema implements TableMetadataProvider
*
* @return TableMetadata object or null if it wasn't found
*/
+ @Override
public TableMetadata getTableMetadata(String keyspace, String table)
{
assert keyspace != null;
diff --git a/src/java/org/apache/cassandra/schema/TableMetadataProvider.java b/src/java/org/apache/cassandra/schema/TableMetadataProvider.java
index 7c5ae8a2f0..e8056d9cba 100644
--- a/src/java/org/apache/cassandra/schema/TableMetadataProvider.java
+++ b/src/java/org/apache/cassandra/schema/TableMetadataProvider.java
@@ -2,13 +2,26 @@ package org.apache.cassandra.schema;
import javax.annotation.Nullable;
+import org.apache.cassandra.db.Keyspace;
import org.apache.cassandra.exceptions.UnknownTableException;
+import org.apache.cassandra.io.sstable.Descriptor;
public interface TableMetadataProvider
{
+ @Nullable
+ Keyspace getKeyspaceInstance(String keyspaceName);
+
+ void storeKeyspaceInstance(Keyspace keyspace);
+
+ @Nullable
+ KeyspaceMetadata getKeyspaceMetadata(String keyspaceName);
+
@Nullable
TableMetadata getTableMetadata(TableId id);
+ @Nullable
+ TableMetadata getTableMetadata(String keyspace, String table);
+
default TableMetadata getExistingTableMetadata(TableId id) throws UnknownTableException
{
TableMetadata metadata = getTableMetadata(id);
@@ -21,4 +34,16 @@ public interface TableMetadataProvider
id);
throw new UnknownTableException(message, id);
}
+
+ @Nullable
+ TableMetadataRef getTableMetadataRef(String keyspace, String table);
+
+ @Nullable
+ TableMetadataRef getTableMetadataRef(TableId id);
+
+ @Nullable
+ default TableMetadataRef getTableMetadataRef(Descriptor descriptor)
+ {
+ return getTableMetadataRef(descriptor.ksname, descriptor.cfname);
+ }
}
diff --git a/test/unit/org/apache/cassandra/db/KeyspaceUnitTest.java b/test/unit/org/apache/cassandra/db/KeyspaceUnitTest.java
index 25d32fc4e1..159879bf11 100644
--- a/test/unit/org/apache/cassandra/db/KeyspaceUnitTest.java
+++ b/test/unit/org/apache/cassandra/db/KeyspaceUnitTest.java
@@ -1,2 +1,50 @@
-package org.apache.cassandra.db;public class KeyspaceUnitTest {
+package org.apache.cassandra.db;
+
+import java.util.UUID;
+
+import org.junit.Test;
+
+import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.db.marshal.BytesType;
+import org.apache.cassandra.schema.KeyspaceMetadata;
+import org.apache.cassandra.schema.KeyspaceParams;
+import org.apache.cassandra.schema.TableId;
+import org.apache.cassandra.schema.TableMetadata;
+import org.apache.cassandra.schema.TableMetadataProvider;
+import org.apache.cassandra.schema.Tables;
+import org.assertj.core.api.Assertions;
+import org.mockito.Mockito;
+
+public class KeyspaceUnitTest
+{
+ static
+ {
+ DatabaseDescriptor.daemonInitialization();
+ }
+
+ /**
+ * Test behavior of trying to open a Keyspace when the table is missing from the schema; this can happen
+ * if the table is dropped while the call to open happens.
+ *
+ * @see <a href="https://issues.apache.org/jira/browse/CASSANDRA-15949">CASSANDRA-15949</a>
+ */
+ @Test
+ public void missingTable()
+ {
+ TableMetadataProvider schema = Mockito.mock(TableMetadataProvider.class, Mockito.CALLS_REAL_METHODS);
+ String ksName = "missingTable";
+ String missingTableName = "table1";
+ TableId tableId = TableId.fromUUID(UUID.randomUUID());
+ KeyspaceMetadata metadata = KeyspaceMetadata.create(ksName, KeyspaceParams.local(), Tables.of(
+ TableMetadata.builder(ksName, missingTableName)
+ .id(tableId)
+ .addPartitionKeyColumn("key", BytesType.instance)
+ .build()));
+ Mockito.when(schema.getKeyspaceMetadata(ksName)).thenReturn(metadata);
+
+ Keyspace ks = Keyspace.open(ksName, schema, false);
+ Assertions.assertThatThrownBy(() -> ks.getColumnFamilyStore(tableId))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("Unknown CF " + tableId);
+ }
}
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org