[ 
https://issues.apache.org/jira/browse/CASSANDRA-21697?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Michael Semb Wever updated CASSANDRA-21697:
-------------------------------------------
    Description: 
{{Keyspace.unload}} unloads the tables and then releases the keyspace metrics:

{code:java}
public void unload(boolean dropData)
{
    for (ColumnFamilyStore cfs : getColumnFamilyStores())
        unloadCf(cfs, dropData);
    metric.release();
}
{code}

Neither step is guarded. {{unloadCf}} calls {{cfs.unloadCf()}}, which does a 
blocking flush, and {{cfs.invalidate(true, dropData)}}, which shuts the 
compaction strategy down, drops the sstables and invalidates the indexes. Any 
of those can throw: a write error on the flush, an undeletable file in 
{{LifecycleTransaction.waitForDeletions()}}, or a custom index whose invalidate 
task fails.

One such failure aborts the loop. The tables after it keep their MBeans 
registered and their compaction strategy running, and {{metric.release()}} 
never runs. The keyspace gauges stay in the metrics registry holding a 
reference to the dead {{Keyspace}} object. 
{{CassandraMetricsRegistry.register}} returns the existing metric on a name 
clash rather than replacing it, so a keyspace later created under the same name 
reports the values of the dropped one, and the dropped {{Keyspace}} is never 
collected.

The only caller is {{Schema.dropKeyspace}}, so an operator meets this on a 
{{DROP KEYSPACE}} that hits a disk error, or on a keyspace that holds an index 
whose invalidation fails.

The patch accumulates the failures with {{Throwables.perform}}, releases the 
metrics, and rethrows the first failure, so every table is unloaded and the 
caller still learns of the error.

Patch: 
[mck/CASSANDRA-21697/5.0|https://github.com/thelastpickle/cassandra/tree/mck/CASSANDRA-21697/5.0]
Provenance: 
[f9d34a98a6|https://github.com/datastax/cassandra/commit/f9d34a98a611a35cded1a77c04937d73c26713dd]
 by [~mfleming]. That commit guards {{unloadCf}} itself and logs the failure 
instead of rethrowing it, which suits a service that must never let a schema 
change fail. This patch keeps the failure visible to the caller, because 
dropping it silently would change what {{DROP KEYSPACE}} reports. That commit 
carries no test; this patch adds one.

  was:
{{Keyspace.unload}} unloads the tables and then releases the keyspace metrics:

{code:java}
public void unload(boolean dropData)
{
    for (ColumnFamilyStore cfs : getColumnFamilyStores())
        unloadCf(cfs, dropData);
    metric.release();
}
{code}

Neither step is guarded. {{unloadCf}} calls {{cfs.unloadCf()}}, which does a 
blocking flush, and {{cfs.invalidate(true, dropData)}}, which shuts the 
compaction strategy down, drops the sstables and invalidates the indexes. Any 
of those can throw: a write error on the flush, an undeletable file in 
{{LifecycleTransaction.waitForDeletions()}}, or a custom index whose invalidate 
task fails.

One such failure aborts the loop. The tables after it keep their MBeans 
registered and their compaction strategy running, and {{metric.release()}} 
never runs. The keyspace gauges stay in the metrics registry holding a 
reference to the dead {{Keyspace}} object. 
{{CassandraMetricsRegistry.register}} returns the existing metric on a name 
clash rather than replacing it, so a keyspace later created under the same name 
reports the values of the dropped one, and the dropped {{Keyspace}} is never 
collected.

The only caller is {{Schema.dropKeyspace}}, so an operator meets this on a 
{{DROP KEYSPACE}} that hits a disk error, or on a keyspace that holds an index 
whose invalidation fails.

The patch accumulates the failures with {{Throwables.perform}}, releases the 
metrics, and rethrows the first failure, so every table is unloaded and the 
caller still learns of the error.

Patch: 
[mck/upstream/keyspace-unload-partial-failure/5.0|https://github.com/thelastpickle/cassandra/tree/mck/upstream/keyspace-unload-partial-failure/5.0]
Provenance: 
[f9d34a98a6|https://github.com/datastax/cassandra/commit/f9d34a98a611a35cded1a77c04937d73c26713dd]
 by [~mfleming]. That commit guards {{unloadCf}} itself and logs the failure 
instead of rethrowing it, which suits a service that must never let a schema 
change fail. This patch keeps the failure visible to the caller, because 
dropping it silently would change what {{DROP KEYSPACE}} reports. That commit 
carries no test; this patch adds one.


> A table that fails to unload leaves the rest of the keyspace loaded and its 
> metrics registered
> ----------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-21697
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-21697
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Local/Other
>            Reporter: Michael Semb Wever
>            Priority: Normal
>             Fix For: 5.0.x, 6.0.x, 7.x
>
>
> {{Keyspace.unload}} unloads the tables and then releases the keyspace metrics:
> {code:java}
> public void unload(boolean dropData)
> {
>     for (ColumnFamilyStore cfs : getColumnFamilyStores())
>         unloadCf(cfs, dropData);
>     metric.release();
> }
> {code}
> Neither step is guarded. {{unloadCf}} calls {{cfs.unloadCf()}}, which does a 
> blocking flush, and {{cfs.invalidate(true, dropData)}}, which shuts the 
> compaction strategy down, drops the sstables and invalidates the indexes. Any 
> of those can throw: a write error on the flush, an undeletable file in 
> {{LifecycleTransaction.waitForDeletions()}}, or a custom index whose 
> invalidate task fails.
> One such failure aborts the loop. The tables after it keep their MBeans 
> registered and their compaction strategy running, and {{metric.release()}} 
> never runs. The keyspace gauges stay in the metrics registry holding a 
> reference to the dead {{Keyspace}} object. 
> {{CassandraMetricsRegistry.register}} returns the existing metric on a name 
> clash rather than replacing it, so a keyspace later created under the same 
> name reports the values of the dropped one, and the dropped {{Keyspace}} is 
> never collected.
> The only caller is {{Schema.dropKeyspace}}, so an operator meets this on a 
> {{DROP KEYSPACE}} that hits a disk error, or on a keyspace that holds an 
> index whose invalidation fails.
> The patch accumulates the failures with {{Throwables.perform}}, releases the 
> metrics, and rethrows the first failure, so every table is unloaded and the 
> caller still learns of the error.
> Patch: 
> [mck/CASSANDRA-21697/5.0|https://github.com/thelastpickle/cassandra/tree/mck/CASSANDRA-21697/5.0]
> Provenance: 
> [f9d34a98a6|https://github.com/datastax/cassandra/commit/f9d34a98a611a35cded1a77c04937d73c26713dd]
>  by [~mfleming]. That commit guards {{unloadCf}} itself and logs the failure 
> instead of rethrowing it, which suits a service that must never let a schema 
> change fail. This patch keeps the failure visible to the caller, because 
> dropping it silently would change what {{DROP KEYSPACE}} reports. That commit 
> carries no test; this patch adds one.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to