From db32c0568997b738a49d9e6a3f54de5e121219f2 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 27 May 2013 10:44:43 +0200 Subject: [PATCH 1/7] use guava for lock handling --- pom.xml | 15 ----------- .../backend/feeds/FeedRefreshUpdater.java | 25 +++++++++---------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/pom.xml b/pom.xml index 51a6d554..3e9168bc 100644 --- a/pom.xml +++ b/pom.xml @@ -31,16 +31,6 @@ true - - jklm.releases - http://mvn.jkeylockmanager.de - - true - - - false - - @@ -244,11 +234,6 @@ joda-time 2.2 - - de.jkeylockmanager - jkeylockmanager - 1.0.0 - net.java.dev.rome diff --git a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java index c47ec935..b7da4e45 100644 --- a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java +++ b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java @@ -6,6 +6,7 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Lock; import javax.annotation.PostConstruct; import javax.annotation.PreDestroy; @@ -26,10 +27,7 @@ import com.commafeed.backend.model.FeedSubscription; import com.commafeed.backend.pubsubhubbub.SubscriptionHandler; import com.commafeed.backend.services.ApplicationSettingsService; import com.commafeed.backend.services.FeedUpdateService; - -import de.jkeylockmanager.manager.KeyLockManager; -import de.jkeylockmanager.manager.KeyLockManagers; -import de.jkeylockmanager.manager.LockCallback; +import com.google.common.util.concurrent.Striped; @Singleton public class FeedRefreshUpdater { @@ -37,8 +35,6 @@ public class FeedRefreshUpdater { protected static Logger log = LoggerFactory .getLogger(FeedRefreshUpdater.class); - private static final KeyLockManager lockManager = KeyLockManagers.newLock(); - @Inject FeedUpdateService feedUpdateService; @@ -53,7 +49,7 @@ public class FeedRefreshUpdater { @Inject ApplicationSettingsService applicationSettingsService; - + @Inject MetricsBean metricsBean; @@ -61,12 +57,14 @@ public class FeedRefreshUpdater { FeedSubscriptionDAO feedSubscriptionDAO; private ThreadPoolExecutor pool; + private Striped locks; @PostConstruct public void init() { ApplicationSettings settings = applicationSettingsService.get(); int threads = Math.max(settings.getDatabaseUpdateThreads(), 1); log.info("Creating database pool with {} threads", threads); + locks = Striped.lock(threads); pool = new ThreadPoolExecutor(threads, threads, 0, TimeUnit.MILLISECONDS, new ArrayBlockingQueue( 100 * threads)); @@ -130,12 +128,13 @@ public class FeedRefreshUpdater { private void updateEntry(final Feed feed, final FeedEntry entry, final List subscriptions) { - lockManager.executeLocked(entry.getGuid(), new LockCallback() { - @Override - public void doInLock() throws Exception { - feedUpdateService.updateEntry(feed, entry, subscriptions); - } - }); + Lock lock = locks.get(entry.getGuid()); + lock.lock(); + try { + feedUpdateService.updateEntry(feed, entry, subscriptions); + } finally { + lock.unlock(); + } } private void handlePubSub(final Feed feed) { From 1ce2d854cbf9fe9b051e15a330ed173a14a09dd9 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 27 May 2013 11:20:19 +0200 Subject: [PATCH 2/7] queue size in metrics --- .../commafeed/backend/feeds/FeedRefreshUpdater.java | 10 ++++++++-- .../frontend/rest/resources/AbstractREST.java | 4 ++++ .../commafeed/frontend/rest/resources/AdminREST.java | 3 ++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java index b7da4e45..c90e3afd 100644 --- a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java +++ b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java @@ -3,6 +3,7 @@ package com.commafeed.backend.feeds; import java.util.Collection; import java.util.List; import java.util.concurrent.ArrayBlockingQueue; +import java.util.concurrent.BlockingQueue; import java.util.concurrent.RejectedExecutionHandler; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; @@ -57,6 +58,7 @@ public class FeedRefreshUpdater { FeedSubscriptionDAO feedSubscriptionDAO; private ThreadPoolExecutor pool; + private BlockingQueue queue; private Striped locks; @PostConstruct @@ -66,8 +68,8 @@ public class FeedRefreshUpdater { log.info("Creating database pool with {} threads", threads); locks = Striped.lock(threads); pool = new ThreadPoolExecutor(threads, threads, 0, - TimeUnit.MILLISECONDS, new ArrayBlockingQueue( - 100 * threads)); + TimeUnit.MILLISECONDS, + queue = new ArrayBlockingQueue(100 * threads)); pool.setRejectedExecutionHandler(new RejectedExecutionHandler() { @Override public void rejectedExecution(Runnable r, ThreadPoolExecutor e) { @@ -149,4 +151,8 @@ public class FeedRefreshUpdater { } } + public int getQueueSize() { + return queue.size(); + } + } diff --git a/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java b/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java index b2599379..7d975cda 100644 --- a/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java +++ b/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java @@ -36,6 +36,7 @@ import com.commafeed.backend.dao.UserRoleDAO; import com.commafeed.backend.dao.UserSettingsDAO; import com.commafeed.backend.feeds.FeedFetcher; import com.commafeed.backend.feeds.FeedRefreshTaskGiver; +import com.commafeed.backend.feeds.FeedRefreshUpdater; import com.commafeed.backend.feeds.OPMLExporter; import com.commafeed.backend.feeds.OPMLImporter; import com.commafeed.backend.model.User; @@ -116,6 +117,9 @@ public abstract class AbstractREST { @Inject FeedRefreshTaskGiver taskGiver; + @Inject + FeedRefreshUpdater feedRefreshUpdater; + @PostConstruct public void init() { CommaFeedApplication app = CommaFeedApplication.get(); diff --git a/src/main/java/com/commafeed/frontend/rest/resources/AdminREST.java b/src/main/java/com/commafeed/frontend/rest/resources/AdminREST.java index 4f6f402f..1673ebe3 100644 --- a/src/main/java/com/commafeed/frontend/rest/resources/AdminREST.java +++ b/src/main/java/com/commafeed/frontend/rest/resources/AdminREST.java @@ -179,7 +179,8 @@ public class AdminREST extends AbstractResourceREST { Map map = ImmutableMap.of("lastMinute", metricsBean.getLastMinute(), "lastHour", metricsBean.getLastHour(), "backlog", - feedDAO.getUpdatableCount()); + feedDAO.getUpdatableCount(), "queue", + feedRefreshUpdater.getQueueSize()); return Response.ok(map).build(); } } From ab88a2f8926b8588040b4789b2efc5b0c536093b Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 27 May 2013 11:35:34 +0200 Subject: [PATCH 3/7] allow proxy configuration out of the box (fix #196) --- src/main/java/com/commafeed/backend/HttpGetter.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/commafeed/backend/HttpGetter.java b/src/main/java/com/commafeed/backend/HttpGetter.java index 4a264adc..51515e66 100644 --- a/src/main/java/com/commafeed/backend/HttpGetter.java +++ b/src/main/java/com/commafeed/backend/HttpGetter.java @@ -33,6 +33,7 @@ import org.apache.http.conn.ssl.X509HostnameVerifier; import org.apache.http.impl.client.DecompressingHttpClient; import org.apache.http.impl.client.DefaultHttpClient; import org.apache.http.impl.client.DefaultHttpRequestRetryHandler; +import org.apache.http.impl.client.SystemDefaultHttpClient; import org.apache.http.params.HttpConnectionParams; import org.apache.http.params.HttpParams; import org.apache.http.params.HttpProtocolParams; @@ -182,7 +183,7 @@ public class HttpGetter { } public static HttpClient newClient() { - DefaultHttpClient client = new DefaultHttpClient(); + DefaultHttpClient client = new SystemDefaultHttpClient(); SSLSocketFactory ssf = new SSLSocketFactory(SSL_CONTEXT, VERIFIER); ClientConnectionManager ccm = client.getConnectionManager(); From 475391c6a7edec506621d95f6ed0ea052084377c Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 27 May 2013 11:37:38 +0200 Subject: [PATCH 4/7] use weak references for lock keys --- .../java/com/commafeed/backend/feeds/FeedRefreshUpdater.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java index c90e3afd..3e203fdb 100644 --- a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java +++ b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java @@ -66,7 +66,7 @@ public class FeedRefreshUpdater { ApplicationSettings settings = applicationSettingsService.get(); int threads = Math.max(settings.getDatabaseUpdateThreads(), 1); log.info("Creating database pool with {} threads", threads); - locks = Striped.lock(threads); + locks = Striped.lazyWeakLock(threads); pool = new ThreadPoolExecutor(threads, threads, 0, TimeUnit.MILLISECONDS, queue = new ArrayBlockingQueue(100 * threads)); From c0df050184a2ae21370cc7a2b79f9f7051195542 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 27 May 2013 11:42:01 +0200 Subject: [PATCH 5/7] wait for lock for one minute, then timeout --- .../java/com/commafeed/backend/feeds/FeedRefreshUpdater.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java index 3e203fdb..9090609e 100644 --- a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java +++ b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java @@ -131,9 +131,12 @@ public class FeedRefreshUpdater { private void updateEntry(final Feed feed, final FeedEntry entry, final List subscriptions) { Lock lock = locks.get(entry.getGuid()); - lock.lock(); try { + lock.tryLock(1, TimeUnit.MINUTES); feedUpdateService.updateEntry(feed, entry, subscriptions); + } catch (InterruptedException e) { + log.error("interrupted while waiting for lock: " + e.getMessage(), + e); } finally { lock.unlock(); } From 5b0c9e940b722a6ece508fe76f1611a0e8b228d0 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 27 May 2013 11:44:14 +0200 Subject: [PATCH 6/7] first resolve absolute url then truncate --- src/main/java/com/commafeed/backend/feeds/FeedParser.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/commafeed/backend/feeds/FeedParser.java b/src/main/java/com/commafeed/backend/feeds/FeedParser.java index 6cce1fa8..0f2a413d 100644 --- a/src/main/java/com/commafeed/backend/feeds/FeedParser.java +++ b/src/main/java/com/commafeed/backend/feeds/FeedParser.java @@ -76,9 +76,9 @@ public class FeedParser { entry.setGuid(FeedUtils.truncate(item.getUri(), 2048)); entry.setGuidHash(DigestUtils.sha1Hex(item.getUri())); - entry.setUrl(FeedUtils.toAbsoluteUrl( - FeedUtils.truncate(item.getLink(), 2048), - feed.getLink())); + entry.setUrl(FeedUtils.truncate( + FeedUtils.toAbsoluteUrl(item.getLink(), feed.getLink()), + 2048)); entry.setUpdated(validateDate(getUpdateDate(item))); entry.setAuthor(FeedUtils.truncate(item.getAuthor(), 128)); From 648923afb857a5467f5d374827fa5c07452f8c74 Mon Sep 17 00:00:00 2001 From: Athou Date: Mon, 27 May 2013 11:46:23 +0200 Subject: [PATCH 7/7] better log message --- .../java/com/commafeed/backend/feeds/FeedRefreshUpdater.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java index 9090609e..28a075fd 100644 --- a/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java +++ b/src/main/java/com/commafeed/backend/feeds/FeedRefreshUpdater.java @@ -135,8 +135,8 @@ public class FeedRefreshUpdater { lock.tryLock(1, TimeUnit.MINUTES); feedUpdateService.updateEntry(feed, entry, subscriptions); } catch (InterruptedException e) { - log.error("interrupted while waiting for lock: " + e.getMessage(), - e); + log.error("interrupted while waiting for lock for " + feed.getUrl() + + " : " + e.getMessage(), e); } finally { lock.unlock(); }