From 7ab1e3827cc588aeb4c69ed1cf59e07b875dcc43 Mon Sep 17 00:00:00 2001 From: jaydeepkumar1984 Date: Fri, 16 May 2025 14:45:08 -0700 Subject: [PATCH] Stop AutoRepair monitoring thread upon Cassandra shutdown patch by Jaydeepkumar Chovatia; reviewed by Bernardo Botella, Andrew Tolbert for CASSANDRA-20623 --- CHANGES.txt | 1 + .../repair/autorepair/AutoRepair.java | 34 +++++++++ .../cassandra/service/StorageService.java | 2 + .../autorepair/AutoRepairShutdownTest.java | 75 +++++++++++++++++++ 4 files changed, 112 insertions(+) create mode 100644 test/unit/org/apache/cassandra/repair/autorepair/AutoRepairShutdownTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 0a4a4c9ac2..4ae4657a94 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Stop AutoRepair monitoring thread upon Cassandra shutdown (CASSANDRA-20623) * Avoid duplicate hardlink error upon forceful taking of ephemeral snapshots during repair (CASSANDRA-20490) * When a custom disk error handler fails to initiate, fail the startup of a node instead of using the no-op handler (CASSANDRA-20614) * Rewrite constraint framework to remove column specification from constraint definition, introduce SQL-like NOT NULL (CASSANDRA-20563) diff --git a/src/java/org/apache/cassandra/repair/autorepair/AutoRepair.java b/src/java/org/apache/cassandra/repair/autorepair/AutoRepair.java index e5923e3c9c..10d18fae63 100644 --- a/src/java/org/apache/cassandra/repair/autorepair/AutoRepair.java +++ b/src/java/org/apache/cassandra/repair/autorepair/AutoRepair.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -102,6 +103,8 @@ public class AutoRepair public boolean isSetupDone = false; public static AutoRepair instance = new AutoRepair(); + public volatile boolean isShutDown = false; + private AutoRepair() { // Private constructor to prevent instantiation @@ -566,4 +569,35 @@ public class AutoRepair } } } + + public synchronized void shutdownBlocking() throws ExecutionException, InterruptedException + { + if (!isSetupDone) + { + // By default, executors within AutoRepair are not initialized as the feature is opt-in. + // If the AutoRepair has not been set up, then there is no need to worry about shutting it down + return; + } + if (isShutDown) + { + throw new IllegalStateException("AutoRepair has already been shut down"); + } + isShutDown = true; + for (AutoRepairConfig.RepairType repairType : AutoRepairConfig.RepairType.values()) + { + repairRunnableExecutors.get(repairType).shutdown(); + repairExecutors.get(repairType).shutdown(); + } + logger.info("Paused AutoRepair"); + } + + public Map getRepairExecutors() + { + return repairExecutors; + } + + public Map getRepairRunnableExecutors() + { + return repairRunnableExecutors; + } } diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 8ef0c7fc8b..0caf26a3a6 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -3913,6 +3913,8 @@ public class StorageService extends NotificationBroadcasterSupport implements IE CommitLog.instance.shutdownBlocking(); + AutoRepair.instance.shutdownBlocking(); + // wait for miscellaneous tasks like sstable and commitlog segment deletion ColumnFamilyStore.shutdownPostFlushExecutor(); diff --git a/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairShutdownTest.java b/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairShutdownTest.java new file mode 100644 index 0000000000..c7de17e0e4 --- /dev/null +++ b/test/unit/org/apache/cassandra/repair/autorepair/AutoRepairShutdownTest.java @@ -0,0 +1,75 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.repair.autorepair; + +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.cql3.CQLTester; +import org.apache.cassandra.repair.autorepair.AutoRepairConfig.RepairType; + +import static org.apache.cassandra.Util.setAutoRepairEnabled; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +/** + * Unit tests to validate the executor shutdown inside {@link AutoRepair} + */ +public class AutoRepairShutdownTest extends CQLTester +{ + @BeforeClass + public static void setupClass() throws Exception + { + setAutoRepairEnabled(true); + requireNetwork(); + } + + @Test + public void testAutoRepairShutdown() throws Exception + { + AutoRepair.instance.setup(); + + for (RepairType type : RepairType.values()) + { + assertFalse("RepairRunnableExecutor should not have been shut down", AutoRepair.instance.getRepairRunnableExecutors().get(type).isShutdown()); + assertFalse("RepairExecutor should not have been shut down", AutoRepair.instance.getRepairExecutors().get(type).isShutdown()); + } + assertFalse("AutoRepair should not be marked as shut down", AutoRepair.instance.isShutDown); + + AutoRepair.instance.shutdownBlocking(); + + for (RepairType type : RepairType.values()) + { + assertTrue("RepairRunnableExecutor should be shut down", AutoRepair.instance.getRepairRunnableExecutors().get(type).isShutdown()); + assertTrue("RepairExecutor should be shut down", AutoRepair.instance.getRepairExecutors().get(type).isShutdown()); + } + assertTrue("AutoRepair should be marked as shut down", AutoRepair.instance.isShutDown); + + try + { + AutoRepair.instance.shutdownBlocking(); + fail("A second call to shutdown should have thrown an exception"); + } + catch (IllegalStateException e) + { + // expected + } + } +}