Author: etnu
Date: Fri Jun 20 02:32:48 2008
New Revision: 669839
URL: http://svn.apache.org/viewvc?rev=669839&view=rev
Log:
Added upper bounds for manifest file caching to avoid situations where an item
has been removed from the HttpCache, but is stuck in the parsed object caches.
This can be a common issue for systems backed by a shared cache.
If you want the old behavior, just set the maxTTL values to something
arbitrarily large.
Also cleaned up Basic*Factory to make the code more compatible. These really do
need to be merged.
Modified:
incubator/shindig/trunk/java/gadgets/conf/gadgets.properties
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java
Modified: incubator/shindig/trunk/java/gadgets/conf/gadgets.properties
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/conf/gadgets.properties?rev=669839&r1=669838&r2=669839&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/conf/gadgets.properties (original)
+++ incubator/shindig/trunk/java/gadgets/conf/gadgets.properties Fri Jun 20
02:32:48 2008
@@ -14,4 +14,6 @@
gadget-spec.cache.capacity=0
message-bundle.cache.capacity=0
gadget-spec.cache.minTTL=300000
+gadget-spec.cache.maxTTL=600000
message-bundle.cache.minTTL=300000
+message-bundle.cache.maxTTL=600000
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java?rev=669839&r1=669838&r2=669839&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicGadgetSpecFactory.java
Fri Jun 20 02:32:48 2008
@@ -39,20 +39,24 @@
import java.util.logging.Logger;
/**
- * Basic implementation of a gadget spec factory
+ * Basic implementation of a gadget spec factory.
+ *
+ * TODO: This needs to be unified with message bundle fetching. We've
basically copied the class in
+ * two places, which is horrible.
*/
@Singleton
public class BasicGadgetSpecFactory implements GadgetSpecFactory {
private static final Logger logger =
Logger.getLogger(BasicGadgetSpecFactory.class.getName());
- private final HttpFetcher specFetcher;
+ private final HttpFetcher fetcher;
private final ContentRewriter rewriter;
private final Executor executor;
- private final long specMinTTL;
+ private final long minTtl;
+ private final long maxTtl;
// A cache of GadgetSpecs with expirations
- private final Cache<URI, SpecTimeoutPair> inMemorySpecCache;
+ private final Cache<URI, TimeoutPair> cache;
public GadgetSpec getGadgetSpec(GadgetContext context) throws
GadgetException {
return getGadgetSpec(context.getUrl(), context.getIgnoreCache());
@@ -61,32 +65,38 @@
/**
* Retrieves a gadget specification from the cache or from the Internet.
*/
- public GadgetSpec getGadgetSpec(URI gadgetUri, boolean ignoreCache) throws
GadgetException {
+ public GadgetSpec getGadgetSpec(URI url, boolean ignoreCache) throws
GadgetException {
if (ignoreCache) {
- return fetchGadgetSpecFromWeb(gadgetUri, true);
+ return fetchFromWeb(url, true);
}
GadgetSpec spec = null;
long expiration = -1;
// Attempt to retrieve the gadget spec from the cache.
- synchronized (inMemorySpecCache) {
- SpecTimeoutPair gadgetSpecEntry =
inMemorySpecCache.getElement(gadgetUri);
+ synchronized (cache) {
+ TimeoutPair gadgetSpecEntry = cache.getElement(url);
if (gadgetSpecEntry != null) {
spec = gadgetSpecEntry.spec;
expiration = gadgetSpecEntry.timeout;
}
}
+ long now = System.currentTimeMillis();
+
// If the gadget spec is not in the cache or has expired, fetch it from
its URI.
- if (spec == null || expiration < System.currentTimeMillis()) {
+ if (spec == null || expiration < now) {
try {
- return fetchGadgetSpecFromWeb(gadgetUri, false);
+ return fetchFromWeb(url, false);
} catch (GadgetException e) {
if (spec == null) {
throw e;
} else {
- logger.info("Gadget spec fetch failed for " + gadgetUri + " - using
cached ");
+ logger.info("Gadget spec fetch failed for " + url + " - using
cached ");
+ // Try again later...
+ synchronized (cache) {
+ cache.addElement(url, new TimeoutPair(spec, now + minTtl));
+ }
}
}
}
@@ -98,16 +108,15 @@
* Retrieves a gadget specification from the Internet, processes its views
and
* adds it to the cache.
*/
- private GadgetSpec fetchGadgetSpecFromWeb(URI gadgetUri, boolean ignoreCache)
- throws GadgetException {
- HttpRequest request = HttpRequest.getRequest(gadgetUri, ignoreCache);
- HttpResponse response = specFetcher.fetch(request);
+ private GadgetSpec fetchFromWeb(URI url, boolean ignoreCache) throws
GadgetException {
+ HttpRequest request = HttpRequest.getRequest(url, ignoreCache);
+ HttpResponse response = fetcher.fetch(request);
if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
throw new
GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
"Unable to retrieve gadget xml. HTTP error " +
response.getHttpStatusCode());
}
- GadgetSpec spec = new GadgetSpec(gadgetUri,
response.getResponseAsString());
+ GadgetSpec spec = new GadgetSpec(url, response.getResponseAsString());
// Find the type=HTML views that link to their content externally.
List<View> hrefViewList = new ArrayList<View>();
@@ -120,7 +129,7 @@
// Retrieve all external view contents simultaneously.
CountDownLatch latch = new CountDownLatch(hrefViewList.size());
for (View v : hrefViewList) {
- executor.execute(new ViewContentFetcher(v, latch, specFetcher,
ignoreCache));
+ executor.execute(new ViewContentFetcher(v, latch, fetcher, ignoreCache));
}
try {
latch.await();
@@ -142,34 +151,39 @@
}
}
- // Add the updated spec back to the cache and force the min TTL
- long expiration = Math.max(
- response.getCacheExpiration(), System.currentTimeMillis() +
specMinTTL);
- synchronized (inMemorySpecCache) {
- inMemorySpecCache.addElement(gadgetUri, new SpecTimeoutPair(spec,
expiration));
+ // We enforce the lower bound limit here for situations where a remote
server temporarily serves
+ // the wrong cache control headers. This allows any distributed caches to
be updated and for the
+ // updates to eventually cascade back into the factory.
+ long now = System.currentTimeMillis();
+ long expiration = response.getCacheExpiration();
+ expiration = Math.min(now + minTtl, Math.max(now + maxTtl, expiration));
+ synchronized (cache) {
+ cache.addElement(url, new TimeoutPair(spec, expiration));
}
return spec;
}
@Inject
- public BasicGadgetSpecFactory(HttpFetcher specFetcher,
+ public BasicGadgetSpecFactory(HttpFetcher fetcher,
ContentRewriter rewriter,
Executor executor,
@Named("gadget-spec.cache.capacity")int
gadgetSpecCacheCapacity,
- @Named("gadget-spec.cache.minTTL")long minTTL)
{
- this.specFetcher = specFetcher;
+ @Named("gadget-spec.cache.minTTL")long minTtl,
+ @Named("gadget-spec.cache.maxTTL")long maxTtl)
{
+ this.fetcher = fetcher;
this.rewriter = rewriter;
this.executor = executor;
- this.inMemorySpecCache = new LruCache<URI,
SpecTimeoutPair>(gadgetSpecCacheCapacity);
- this.specMinTTL = minTTL;
+ this.cache = new LruCache<URI, TimeoutPair>(gadgetSpecCacheCapacity);
+ this.minTtl = minTtl;
+ this.maxTtl = maxTtl;
}
- private static class SpecTimeoutPair {
+ private static class TimeoutPair {
private final GadgetSpec spec;
private final long timeout;
- private SpecTimeoutPair(GadgetSpec spec, long timeout) {
+ private TimeoutPair(GadgetSpec spec, long timeout) {
this.spec = spec;
this.timeout = timeout;
}
Modified:
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java?rev=669839&r1=669838&r2=669839&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/BasicMessageBundleFactory.java
Fri Jun 20 02:32:48 2008
@@ -31,7 +31,6 @@
import com.google.inject.name.Named;
import java.net.URI;
-import java.util.logging.Level;
import java.util.logging.Logger;
/**
@@ -43,11 +42,12 @@
private static final Logger logger
= Logger.getLogger(BasicMessageBundleFactory.class.getName());
- private final HttpFetcher bundleFetcher;
+ private final HttpFetcher fetcher;
- private final Cache<URI, BundleTimeoutPair> inMemoryBundleCache;
+ private final Cache<URI, TimeoutPair> cache;
- private final long bundleMinTTL;
+ private final long minTtl;
+ private final long maxTtl;
public MessageBundle getBundle(LocaleSpec localeSpec, GadgetContext context)
throws GadgetException {
@@ -61,72 +61,79 @@
return getBundle(messages, context.getIgnoreCache());
}
- public MessageBundle getBundle(URI bundleUrl, boolean ignoreCache)
- throws GadgetException {
+ public MessageBundle getBundle(URI url, boolean ignoreCache) throws
GadgetException {
if (ignoreCache) {
- return fetchBundleFromWeb(bundleUrl, true);
+ return fetchFromWeb(url, true);
}
MessageBundle bundle = null;
long expiration = -1;
- synchronized (inMemoryBundleCache) {
- BundleTimeoutPair entry = inMemoryBundleCache.getElement(bundleUrl);
+ synchronized (cache) {
+ TimeoutPair entry = cache.getElement(url);
if (entry != null) {
bundle = entry.bundle;
expiration = entry.timeout;
}
}
- if (bundle == null || expiration < System.currentTimeMillis()) {
+
+ long now = System.currentTimeMillis();
+ if (bundle == null || expiration < now) {
try {
- return fetchBundleFromWeb(bundleUrl, false);
- } catch (GadgetException ge) {
+ return fetchFromWeb(url, false);
+ } catch (GadgetException e) {
if (bundle == null) {
- throw ge;
+ throw e;
} else {
- logger.log(Level.WARNING,
- "Msg bundle fetch failed for " + bundleUrl + " - using cached
", ge);
+ logger.info("Message bundle fetch failed for " + url + " - using
cached ");
+ // Try again later...
+ synchronized (cache) {
+ cache.addElement(url, new TimeoutPair(bundle, now + minTtl));
+ }
}
}
}
return bundle;
}
- private MessageBundle fetchBundleFromWeb(URI bundleUrl, boolean ignoreCache)
- throws GadgetException {
- HttpRequest request = HttpRequest.getRequest(bundleUrl, ignoreCache);
- HttpResponse response = bundleFetcher.fetch(request);
+ private MessageBundle fetchFromWeb(URI url, boolean ignoreCache) throws
GadgetException {
+ HttpRequest request = HttpRequest.getRequest(url, ignoreCache);
+ HttpResponse response = fetcher.fetch(request);
if (response.getHttpStatusCode() != HttpResponse.SC_OK) {
throw new
GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT,
"Unable to retrieve message bundle xml. HTTP error " +
response.getHttpStatusCode());
}
- MessageBundle bundle
- = new MessageBundle(bundleUrl, response.getResponseAsString());
- synchronized (inMemoryBundleCache) {
- long expiration = Math.max(response.getCacheExpiration(),
- System.currentTimeMillis() + bundleMinTTL);
- inMemoryBundleCache.addElement(bundleUrl,
- new BundleTimeoutPair(bundle, expiration));
+ MessageBundle bundle = new MessageBundle(url,
response.getResponseAsString());
+
+ // We enforce the lower bound limit here for situations where a remote
server temporarily serves
+ // the wrong cache control headers. This allows any distributed caches to
be updated and for the
+ // updates to eventually cascade back into the factory.
+ long now = System.currentTimeMillis();
+ long expiration = response.getCacheExpiration();
+ expiration = Math.min(now + minTtl, Math.max(now + maxTtl, expiration));
+ synchronized (cache) {
+ cache.addElement(url, new TimeoutPair(bundle, expiration));
}
return bundle;
}
@Inject
- public BasicMessageBundleFactory(HttpFetcher bundleFetcher,
- @Named("message-bundle.cache.capacity")int messageBundleCacheCapacity,
- @Named("message-bundle.cache.minTTL")long minTTL) {
- this.bundleFetcher = bundleFetcher;
- this.inMemoryBundleCache =
- new LruCache<URI, BundleTimeoutPair>(messageBundleCacheCapacity);
- this.bundleMinTTL = minTTL;
+ public BasicMessageBundleFactory(HttpFetcher fetcher,
+ @Named("message-bundle.cache.capacity")int
capacity,
+ @Named("message-bundle.cache.minTTL")long
minTtl,
+ @Named("message-bundle.cache.maxTTL")long
maxTtl) {
+ this.fetcher = fetcher;
+ this.cache = new LruCache<URI, TimeoutPair>(capacity);
+ this.minTtl = minTtl;
+ this.maxTtl = maxTtl;
}
- private static class BundleTimeoutPair {
+ private static class TimeoutPair {
private MessageBundle bundle;
private long timeout;
- private BundleTimeoutPair(MessageBundle bundle, long timeout) {
+ private TimeoutPair(MessageBundle bundle, long timeout) {
this.bundle = bundle;
this.timeout = timeout;
}
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java?rev=669839&r1=669838&r2=669839&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicGadgetSpecFactoryTest.java
Fri Jun 20 02:32:48 2008
@@ -90,7 +90,7 @@
private final CaptureRewriter rewriter = new CaptureRewriter();
private final BasicGadgetSpecFactory specFactory
- = new BasicGadgetSpecFactory(fetcher, rewriter, FAKE_EXECUTOR, 5, -1000);
+ = new BasicGadgetSpecFactory(fetcher, rewriter, FAKE_EXECUTOR, 5, -1000,
1000);
@Test
public void specFetched() throws Exception {
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java?rev=669839&r1=669838&r2=669839&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/BasicMessageBundleFactoryTest.java
Fri Jun 20 02:32:48 2008
@@ -70,7 +70,7 @@
private final MessageBundleFactory bundleFactory;
public BasicMessageBundleFactoryTest() {
- bundleFactory = new BasicMessageBundleFactory(fetcher, 5, -1000);
+ bundleFactory = new BasicMessageBundleFactory(fetcher, 5, -1000, 1000);
}
@Test
Modified:
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java
URL:
http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java?rev=669839&r1=669838&r2=669839&view=diff
==============================================================================
---
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java
(original)
+++
incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/GadgetTestFixture.java
Fri Jun 20 02:32:48 2008
@@ -40,7 +40,7 @@
public final HttpFetcher fetcher = mock(HttpFetcher.class);
public final GadgetBlacklist blacklist = mock(GadgetBlacklist.class);
public final MessageBundleFactory bundleFactory =
- new BasicMessageBundleFactory(fetcher, 0, 0L);
+ new BasicMessageBundleFactory(fetcher, 0, 0L, 0L);
public GadgetFeatureRegistry registry;
public ContainerConfig containerConfig = mock(ContainerConfig.class);
public final Executor executor = new Executor() {
@@ -49,7 +49,7 @@
}
};
public final GadgetSpecFactory specFactory = new BasicGadgetSpecFactory(
- fetcher, new NoOpContentRewriter(), executor, 0, 0L);
+ fetcher, new NoOpContentRewriter(), executor, 0, 0L, 0L);
public GadgetTestFixture() {
