From 53b49b1180dd2299b76709b4e7cf2ac184d2190e Mon Sep 17 00:00:00 2001 From: Gary Dusbabek Date: Fri, 19 Nov 2010 16:26:16 +0000 Subject: [PATCH] resolve circular initializer dependency deadlock. patch by Erik Onnen and Gary Dusbabek, reviewed by Jonathan Ellis. CASSANDRA-1756 git-svn-id: https://svn.apache.org/repos/asf/cassandra/branches/cassandra-0.7@1036922 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 1 + .../locator/DynamicEndpointSnitch.java | 4 +- .../cassandra/service/StorageService.java | 11 ++++-- test/conf/cassandra.yaml | 1 + .../locator/DynamicEndpointSnitchTest.java | 7 +++- .../cassandra/service/InitClientTest.java | 37 +++++++++++++++++++ 6 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 test/unit/org/apache/cassandra/service/InitClientTest.java diff --git a/CHANGES.txt b/CHANGES.txt index e4d8d3f095..5bcf52cf58 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -41,6 +41,7 @@ dev * fix sstableimport regression (CASSANDRA-1753) * fix for bootstrap when no non-system tables are defined (CASSANDRA-1732) * handle replica unavailability in index scan (CASSANDRA-1755) + * fix service initialization order deadlock (CASSANDRA-1756) 0.7.0-beta3 diff --git a/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java b/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java index 3c9973bd89..db3d15b24c 100644 --- a/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java +++ b/src/java/org/apache/cassandra/locator/DynamicEndpointSnitch.java @@ -77,7 +77,7 @@ public class DynamicEndpointSnitch extends AbstractEndpointSnitch implements ILa MBeanServer mbs = ManagementFactory.getPlatformMBeanServer(); try { - mbs.registerMBean(this, new ObjectName("org.apache.cassandra.db:type=DynamicEndpointSnitch")); + mbs.registerMBean(this, new ObjectName("org.apache.cassandra.db:type=DynamicEndpointSnitch,instance="+hashCode())); } catch (Exception e) { @@ -178,6 +178,8 @@ public class DynamicEndpointSnitch extends AbstractEndpointSnitch implements ILa private void updateScores() // this is expensive { + if (!StorageService.instance.isInitialized()) + return; if (!registered) { ILatencyPublisher handler = (ILatencyPublisher)MessagingService.instance.getVerbHandler(StorageService.Verb.REQUEST_RESPONSE); diff --git a/src/java/org/apache/cassandra/service/StorageService.java b/src/java/org/apache/cassandra/service/StorageService.java index 8c28ad6d80..296f44874f 100644 --- a/src/java/org/apache/cassandra/service/StorageService.java +++ b/src/java/org/apache/cassandra/service/StorageService.java @@ -196,11 +196,11 @@ public class StorageService implements IEndpointStateChangeSubscriber, StorageSe }}; + public static final RetryingScheduledThreadPoolExecutor scheduledTasks = new RetryingScheduledThreadPoolExecutor("ScheduledTasks"); + private static IPartitioner partitioner_ = DatabaseDescriptor.getPartitioner(); public static VersionedValue.VersionedValueFactory valueFactory = new VersionedValue.VersionedValueFactory(partitioner_); - - public static RetryingScheduledThreadPoolExecutor scheduledTasks = new RetryingScheduledThreadPoolExecutor("ScheduledTasks"); - + public static final StorageService instance = new StorageService(); public static IPartitioner getPartitioner() { @@ -309,6 +309,11 @@ public class StorageService implements IEndpointStateChangeSubscriber, StorageSe MessagingService.shutdown(); StageManager.shutdownNow(); } + + public boolean isInitialized() + { + return initialized; + } public synchronized void initClient() throws IOException { diff --git a/test/conf/cassandra.yaml b/test/conf/cassandra.yaml index b74da12a61..bf5f1618c0 100644 --- a/test/conf/cassandra.yaml +++ b/test/conf/cassandra.yaml @@ -21,6 +21,7 @@ disk_access_mode: mmap seeds: - 127.0.0.2 endpoint_snitch: org.apache.cassandra.locator.SimpleSnitch +dynamic_snitch: true request_scheduler: org.apache.cassandra.scheduler.RoundRobinScheduler request_scheduler_id: keyspace keyspaces: diff --git a/test/unit/org/apache/cassandra/locator/DynamicEndpointSnitchTest.java b/test/unit/org/apache/cassandra/locator/DynamicEndpointSnitchTest.java index c7eb9824c5..daeacf921d 100644 --- a/test/unit/org/apache/cassandra/locator/DynamicEndpointSnitchTest.java +++ b/test/unit/org/apache/cassandra/locator/DynamicEndpointSnitchTest.java @@ -19,10 +19,11 @@ package org.apache.cassandra.locator; +import java.io.IOException; import java.net.InetAddress; -import java.net.UnknownHostException; import java.util.ArrayList; +import org.apache.cassandra.service.StorageService; import org.junit.Test; import org.apache.cassandra.utils.FBUtilities; @@ -30,8 +31,10 @@ import org.apache.cassandra.utils.FBUtilities; public class DynamicEndpointSnitchTest { @Test - public void testSnitch() throws UnknownHostException, InterruptedException + public void testSnitch() throws InterruptedException, IOException { + // do this because SS needs to be initialized before DES can work properly. + StorageService.instance.initClient(); int sleeptime = 150; DynamicEndpointSnitch dsnitch = new DynamicEndpointSnitch(new SimpleSnitch()); InetAddress self = FBUtilities.getLocalAddress(); diff --git a/test/unit/org/apache/cassandra/service/InitClientTest.java b/test/unit/org/apache/cassandra/service/InitClientTest.java new file mode 100644 index 0000000000..025a775b02 --- /dev/null +++ b/test/unit/org/apache/cassandra/service/InitClientTest.java @@ -0,0 +1,37 @@ +package org.apache.cassandra.service; + +import org.junit.Test; + +import java.io.IOException; + +/** + * 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. + */ + + +public class InitClientTest // extends CleanupHelper +{ + @Test + public void testInitClientStartup() + { + try { + StorageService.instance.initClient(); + } catch (IOException ex) { + throw new AssertionError(ex.getMessage()); + } + } +}