[freenet-cvs] r14597 - trunk/freenet/src/freenet/client

[toad at freenetproject.org]

Author: toad
Date: 2007-08-11 12:53:51 +0000 (Sat, 11 Aug 2007)
New Revision: 14597

Modified:
   trunk/freenet/src/freenet/client/ArchiveManager.java
   trunk/freenet/src/freenet/client/ArchiveStoreContext.java
   trunk/freenet/src/freenet/client/RealArchiveStoreItem.java
Log:
Remove incrementSpace/decrementSpace. Call close() directly, inside the ArchiveManager lock:
- much more accurate space usage limiting
- won't get nasty race conditions where we have space used but nothing to remove, or the same due to exceptions/throwables
- ArchiveManager lock may be held during expensive deletes
- simpler code

Modified: trunk/freenet/src/freenet/client/ArchiveManager.java
===================================================================
--- trunk/freenet/src/freenet/client/ArchiveManager.java	2007-08-11 12:43:04 UTC (rev 14596)
+++ trunk/freenet/src/freenet/client/ArchiveManager.java	2007-08-11 12:53:51 UTC (rev 14597)
@@ -147,7 +147,14 @@
 	 * @param item The ArchiveStoreItem to remove.
 	 */
 	synchronized void removeCachedItem(ArchiveStoreItem item) {
-		storedData.removeKey(item.key);	
+		long size = item.spaceUsed();
+		storedData.removeKey(item.key);
+		// Hard disk space limit = remove it here.
+		// Soft disk space limit would be to remove it outside the lock.
+		// Soft disk space limit = we go over the limit significantly when we
+		// are overloaded.
+		cachedData -= size;
+		item.close();
 	}
 	
 	/**
@@ -400,7 +407,7 @@
 	 * callback != null.
 	 */
 	private ArchiveStoreItem addStoreElement(ArchiveStoreContext ctx, FreenetURI key, String name, TempStoreElement temp, MutableBoolean gotElement, String callbackName, ArchiveExtractCallback callback) throws ArchiveFailureException {
-		RealArchiveStoreItem element = new RealArchiveStoreItem(this, ctx, key, name, temp);
+		RealArchiveStoreItem element = new RealArchiveStoreItem(ctx, key, name, temp);
 		if(logMINOR) Logger.minor(this, "Adding store element: "+element+" ( "+key+ ' ' +name+" size "+element.spaceUsed()+" )");
 		ArchiveStoreItem oldItem;
 		// Let it throw, if it does something is drastically wrong
@@ -411,13 +418,16 @@
 		synchronized (this) {
 			oldItem = (ArchiveStoreItem) storedData.get(element.key);
 			storedData.push(element.key, element);
+			cachedData += element.spaceUsed();
+			if(oldItem != null) {
+				cachedData -= oldItem.spaceUsed();
+				oldItem.close();
+			}
 		}
 		if(matchBucket != null) {
 			callback.gotBucket(matchBucket);
 			gotElement.value = true;
 		}
-		if(oldItem != null)
-			oldItem.close();
 		return element;
 	}
 
@@ -426,21 +436,25 @@
 	 * Call synchronized on storedData.
 	 */
 	private void trimStoredData() {
+		synchronized(this) {
 		while(true) {
 			ArchiveStoreItem item;
-			synchronized(this) {
 				if(cachedData <= maxCachedData && storedData.size() <= maxCachedElements) return;
 				if(storedData.isEmpty()) {
 					// Race condition? cachedData out of sync?
 					Logger.error(this, "storedData is empty but still over limit: cachedData="+cachedData+" / "+maxCachedData);
 					return;
 				}
-				item = (ArchiveStoreItem) storedData.popValue();	
-			}
+				item = (ArchiveStoreItem) storedData.popValue();
+				long space = item.spaceUsed();
+				cachedData -= space;
+				// Hard limits = delete file within lock, soft limits = delete outside of lock
+				// Here we use a hard limit
 			if(logMINOR)
 				Logger.minor(this, "Dropping "+item+" : cachedData="+cachedData+" of "+maxCachedData);
 			item.close();
 		}
+		}
 	}
 
 	/** 
@@ -476,12 +490,4 @@
 			return Metadata.ARCHIVE_ZIP;
 		else throw new IllegalArgumentException(); 
 	}
-
-	synchronized void decrementSpace(long spaceUsed) {
-		cachedData -= spaceUsed;
-	}
-
-	synchronized void incrementSpace(long spaceUsed) {
-		cachedData += spaceUsed;
-	}
 }

Modified: trunk/freenet/src/freenet/client/ArchiveStoreContext.java
===================================================================
--- trunk/freenet/src/freenet/client/ArchiveStoreContext.java	2007-08-11 12:43:04 UTC (rev 14596)
+++ trunk/freenet/src/freenet/client/ArchiveStoreContext.java	2007-08-11 12:53:51 UTC (rev 14597)
@@ -123,12 +123,10 @@
 
 	/** Notify that an archive store item with this key has been expelled from the cache. */
 	public void removeItem(ArchiveStoreItem item) {
-		long spaceUsed = item.spaceUsed();
 		synchronized(myItems) {
 			if(myItems.remove(item) == null) return; // only removed once
 		}
 		item.innerClose();
-		manager.decrementSpace(spaceUsed);
 	}
 
 	public short getArchiveType() {

Modified: trunk/freenet/src/freenet/client/RealArchiveStoreItem.java
===================================================================
--- trunk/freenet/src/freenet/client/RealArchiveStoreItem.java	2007-08-11 12:43:04 UTC (rev 14596)
+++ trunk/freenet/src/freenet/client/RealArchiveStoreItem.java	2007-08-11 12:53:51 UTC (rev 14597)
@@ -12,7 +12,6 @@
 
 class RealArchiveStoreItem extends ArchiveStoreItem {
 
-	private final ArchiveManager manager;
 	private final File myFilename;
 	private final MultiReaderBucket mb;
 	private final Bucket bucket;
@@ -25,15 +24,13 @@
 	 * @param temp The TempStoreElement currently storing the data.
 	 * @param manager The parent ArchiveManager within which this item is stored.
 	 */
-	RealArchiveStoreItem(ArchiveManager manager, ArchiveStoreContext ctx, FreenetURI key2, String realName, TempStoreElement temp) {
+	RealArchiveStoreItem(ArchiveStoreContext ctx, FreenetURI key2, String realName, TempStoreElement temp) {
 		super(new ArchiveKey(key2, realName), ctx);
-		this.manager = manager;
 		mb = new MultiReaderBucket(temp.bucket);
 		this.bucket = mb.getReaderBucket();
 		temp.underBucket.setReadOnly();
 		this.myFilename = temp.underBucket.getFile();
 		spaceUsed = FileUtil.estimateUsage(myFilename, temp.underBucket.size());
-		this.manager.incrementSpace(spaceUsed);
 	}
 
 	/**