Return-Path: X-Original-To: apmail-cloudstack-commits-archive@www.apache.org Delivered-To: apmail-cloudstack-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 16C7910B2E for ; Mon, 7 Oct 2013 18:31:03 +0000 (UTC) Received: (qmail 866 invoked by uid 500); 7 Oct 2013 18:30:47 -0000 Delivered-To: apmail-cloudstack-commits-archive@cloudstack.apache.org Received: (qmail 490 invoked by uid 500); 7 Oct 2013 18:30:37 -0000 Mailing-List: contact commits-help@cloudstack.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cloudstack.apache.org Delivered-To: mailing list commits@cloudstack.apache.org Received: (qmail 169 invoked by uid 99); 7 Oct 2013 18:30:26 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 07 Oct 2013 18:30:26 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id AA13891191C; Mon, 7 Oct 2013 18:30:26 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: bfederle@apache.org To: commits@cloudstack.apache.org Date: Mon, 07 Oct 2013 18:30:33 -0000 Message-Id: In-Reply-To: <29a32d812e6f4b29a60c3ac9e7f0dd1c@git.apache.org> References: <29a32d812e6f4b29a60c3ac9e7f0dd1c@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [08/27] git commit: updated refs/heads/ui-restyle to 205f22b ConstantTimeBackoff test and cleanup - javadoc changed - the old one was copy-pasted from AgentShell - start and stop method removed - they did the same as the overridden methods - _counter removed as it was only written, but never read - remove from _asleep map was moved to a finally block, to make sure it is removed even in case of the thread gets interrupted - Tests created for the above scenarios. Signed-off-by: Laszlo Hornyak Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/826c69fd Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/826c69fd Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/826c69fd Branch: refs/heads/ui-restyle Commit: 826c69fd29bb98d3f631596f72a6eb3525d83aa2 Parents: bd85367 Author: Laszlo Hornyak Authored: Mon Sep 30 15:43:42 2013 +0200 Committer: Darren Shepherd Committed: Fri Oct 4 11:24:43 2013 -0700 ---------------------------------------------------------------------- agent/src/com/cloud/agent/AgentShell.java | 6 - .../utils/backoff/impl/ConstantTimeBackoff.java | 36 +++--- .../backoff/impl/ConstantTimeBackoffTest.java | 110 +++++++++++++++++++ 3 files changed, 125 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/826c69fd/agent/src/com/cloud/agent/AgentShell.java ---------------------------------------------------------------------- diff --git a/agent/src/com/cloud/agent/AgentShell.java b/agent/src/com/cloud/agent/AgentShell.java index bf1e818..42adac0 100644 --- a/agent/src/com/cloud/agent/AgentShell.java +++ b/agent/src/com/cloud/agent/AgentShell.java @@ -19,14 +19,11 @@ package com.cloud.agent; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; -import java.io.FileOutputStream; import java.io.IOException; -import java.io.InputStream; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collections; -import java.util.Date; import java.util.Enumeration; import java.util.HashMap; import java.util.List; @@ -39,9 +36,7 @@ import javax.naming.ConfigurationException; import org.apache.commons.daemon.Daemon; import org.apache.commons.daemon.DaemonContext; import org.apache.commons.daemon.DaemonInitException; -import org.apache.commons.httpclient.HttpClient; import org.apache.commons.httpclient.MultiThreadedHttpConnectionManager; -import org.apache.commons.httpclient.methods.GetMethod; import org.apache.log4j.Logger; import org.apache.log4j.xml.DOMConfigurator; @@ -56,7 +51,6 @@ import com.cloud.utils.PropertiesUtil; import com.cloud.utils.backoff.BackoffAlgorithm; import com.cloud.utils.backoff.impl.ConstantTimeBackoff; import com.cloud.utils.exception.CloudRuntimeException; -import com.cloud.utils.script.Script; public class AgentShell implements IAgentShell, Daemon { private static final Logger s_logger = Logger.getLogger(AgentShell.class http://git-wip-us.apache.org/repos/asf/cloudstack/blob/826c69fd/utils/src/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java b/utils/src/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java index 976e369..4386d4d 100755 --- a/utils/src/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java +++ b/utils/src/com/cloud/utils/backoff/impl/ConstantTimeBackoff.java @@ -19,16 +19,19 @@ package com.cloud.utils.backoff.impl; import java.util.Collection; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.ConcurrentMap; import javax.ejb.Local; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; + import com.cloud.utils.NumbersUtil; import com.cloud.utils.backoff.BackoffAlgorithm; import com.cloud.utils.component.AdapterBase; /** - * Implementation of the Agent Manager. This class controls the connection + * An implementation of BackoffAlgorithm that waits for some seconds. + * After the time the client can try to perform the operation again. * * @config * {@table @@ -38,27 +41,29 @@ import com.cloud.utils.component.AdapterBase; **/ @Local(value={BackoffAlgorithm.class}) public class ConstantTimeBackoff extends AdapterBase implements BackoffAlgorithm, ConstantTimeBackoffMBean { - int _count = 0; long _time; - ConcurrentHashMap _asleep = new ConcurrentHashMap(); + private final ConcurrentHashMap _asleep = new ConcurrentHashMap(); + private final static Log LOG = LogFactory.getLog(ConstantTimeBackoff.class); @Override public void waitBeforeRetry() { - _count++; + Thread current = Thread.currentThread(); try { - Thread current = Thread.currentThread(); _asleep.put(current.getName(), current); Thread.sleep(_time); + } catch (InterruptedException e) { + // JMX or other threads may interrupt this thread, but let's log it + // anyway, no exception to log as this is not an error + LOG.info("Thread " + current.getName() + + " interrupted while waiting for retry"); + } finally { _asleep.remove(current.getName()); - } catch(InterruptedException e) { - } return; } @Override public void reset() { - _count = 0; } @Override @@ -71,7 +76,7 @@ public class ConstantTimeBackoff extends AdapterBase implements BackoffAlgorithm public Collection getWaiters() { return _asleep.keySet(); } - + @Override public boolean wakeup(String threadName) { Thread th = _asleep.get(threadName); @@ -84,17 +89,6 @@ public class ConstantTimeBackoff extends AdapterBase implements BackoffAlgorithm } @Override - public boolean start() { - _count = 0; - return true; - } - - @Override - public boolean stop() { - return true; - } - - @Override public long getTimeToWait() { return _time; } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/826c69fd/utils/test/com/cloud/utils/backoff/impl/ConstantTimeBackoffTest.java ---------------------------------------------------------------------- diff --git a/utils/test/com/cloud/utils/backoff/impl/ConstantTimeBackoffTest.java b/utils/test/com/cloud/utils/backoff/impl/ConstantTimeBackoffTest.java new file mode 100644 index 0000000..4b2b64d --- /dev/null +++ b/utils/test/com/cloud/utils/backoff/impl/ConstantTimeBackoffTest.java @@ -0,0 +1,110 @@ +// 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 +// 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 com.cloud.utils.backoff.impl; + +import java.util.HashMap; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.junit.Assert; +import org.junit.Test; + +public class ConstantTimeBackoffTest { + final static private Log LOG = LogFactory + .getLog(ConstantTimeBackoffTest.class); + + @Test + public void waitBeforeRetryWithInterrupt() throws InterruptedException { + final ConstantTimeBackoff backoff = new ConstantTimeBackoff(); + backoff.setTimeToWait(10); + Assert.assertTrue(backoff.getWaiters().isEmpty()); + Thread waitThread = new Thread(new Runnable() { + @Override + public void run() { + backoff.waitBeforeRetry(); + } + }); + waitThread.start(); + Thread.sleep(100); + Assert.assertFalse(backoff.getWaiters().isEmpty()); + waitThread.interrupt(); + Thread.sleep(100); + Assert.assertTrue(backoff.getWaiters().isEmpty()); + } + + @Test + public void waitBeforeRetry() throws InterruptedException { + final ConstantTimeBackoff backoff = new ConstantTimeBackoff(); + // let's not wait too much in a test + backoff.setTimeToWait(0); + // check if the list of waiters is empty + Assert.assertTrue(backoff.getWaiters().isEmpty()); + // call the waitBeforeRetry which will wait 0 ms and return + backoff.waitBeforeRetry(); + // on normal exit the list of waiters should be cleared + Assert.assertTrue(backoff.getWaiters().isEmpty()); + } + + @Test + public void configureEmpty() { + // at this point this is the only way rhe configure method gets invoked, + // therefore have to make sure it works correctly + final ConstantTimeBackoff backoff = new ConstantTimeBackoff(); + backoff.configure("foo", new HashMap()); + Assert.assertEquals(5000, backoff.getTimeToWait()); + } + + @Test + public void configureWithValue() { + final ConstantTimeBackoff backoff = new ConstantTimeBackoff(); + HashMap params = new HashMap(); + params.put("seconds", "100"); + backoff.configure("foo", params); + Assert.assertEquals(100000, backoff.getTimeToWait()); + } + + /** + * Test that wakeup returns false when trying to wake a non existing thread. + */ + @Test + public void wakeupNotExisting() { + final ConstantTimeBackoff backoff = new ConstantTimeBackoff(); + Assert.assertFalse(backoff.wakeup("NOT EXISTING THREAD")); + } + + /** + * Test that wakeup will return true if the thread is waiting. + */ + @Test + public void wakeupExisting() throws InterruptedException { + final ConstantTimeBackoff backoff = new ConstantTimeBackoff(); + backoff.setTimeToWait(10); + Thread thread = new Thread(new Runnable() { + @Override + public void run() { + LOG.debug("before"); + backoff.waitBeforeRetry(); + LOG.debug("after"); + } + }); + thread.start(); + LOG.debug("thread started"); + Thread.sleep(100); + LOG.debug("testing wakeup"); + Assert.assertTrue(backoff.wakeup(thread.getName())); + } +}