Mailing-List: contact oak-commits-help@jackrabbit.apache.org; run by ezmlm
Precedence: bulk
Reply-To: oak-dev@jackrabbit.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1409231 - in
 /jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs:
 AbstractBlobStore.java DbBlobStore.java FileBlobStore.java
Date: Wed, 14 Nov 2012 15:37:10 -0000
To: oak-commits@jackrabbit.apache.org
From: mduerig@apache.org
Message-Id: <20121114153711.401CB238897F@eris.apache.org>

Author: mduerig
Date: Wed Nov 14 15:37:10 2012
New Revision: 1409231

URL: http://svn.apache.org/viewvc?rev=1409231&view=rev
Log:
OAK-123 Data store improvements (documentation, encapsulation)
Reverting changes from revision 1409213 since they break the build

Modified:
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/AbstractBlobStore.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/DbBlobStore.java
    jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/FileBlobStore.java

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/AbstractBlobStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/AbstractBlobStore.java?rev=1409231&r1=1409230&r2=1409231&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/AbstractBlobStore.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/AbstractBlobStore.java Wed Nov 14 15:37:10 2012
@@ -66,43 +66,12 @@ public abstract class AbstractBlobStore 
 
     protected static final String HASH_ALGORITHM = "SHA-256";
 
-    /**
-     * The prefix for small blocks, where the data is encoded in the id.
-     */
     protected static final int TYPE_DATA = 0;
-    
-    /**
-     * The prefix for stored blocks, where the hash code is the id.
-     */
     protected static final int TYPE_HASH = 1;
+    protected static final int TYPE_HASH_COMPRESSED = 2;
     
-    /**
-     * The prefix for stored blocks, where the stored data contains the list of
-     * hashes (indirect hash).
-     */
-   protected static final int TYPE_HASH_COMPRESSED = 2;
-    
-    /**
-     * The minimum block size. Smaller blocks may not be stored, as the hash
-     * code would be larger than the stored data itself.
-     */
     protected static final int BLOCK_SIZE_LIMIT = 48;
 
-    /**
-     * The weak map of data store ids that are still in use. For ids that are
-     * still referenced in memory, the stored blocks will not be deleted when
-     * running garbage collection. This should prevent that binaries are removed
-     * before the reference was written to the MicroKernel (before there is a
-     * persistent reference).
-     * <p>
-     * Please note this will not prevent binaries to be deleted if they are only
-     * referenced from within another JVM (when the MicroKernel API is remoted).
-     * <p>
-     * Instead of this map, it might be better to use a fixed timeout. In this
-     * case, the creation / modification time should be persisted, so that it
-     * survives restarts. Or, as an alternative, the storage backends could be
-     * segmented (young generation / old generation).
-     */
     protected Map<String, WeakReference<String>> inUse =
         Collections.synchronizedMap(new WeakHashMap<String, WeakReference<String>>());
 
@@ -118,56 +87,22 @@ public abstract class AbstractBlobStore 
      */
     private int blockSize = 2 * 1024 * 1024;
 
-    /**
-     * A very small cache of the last used blocks.
-     */
     private Cache<AbstractBlobStore.BlockId, Data> cache = Cache.newInstance(this, 8 * 1024 * 1024);
 
-    /**
-     * Set the minimum block size (smaller blocks are inlined in the id, larger
-     * blocks are stored).
-     * 
-     * @param x the minimum block size
-     */
     public void setBlockSizeMin(int x) {
         validateBlockSize(x);
         this.blockSizeMin = x;
     }
 
-    /**
-     * Get the minimum block size.
-     * 
-     * @return the minimum block size
-     */
     public long getBlockSizeMin() {
         return blockSizeMin;
     }
 
-    /**
-     * Set the maximum block size (larger binaries are split into blocks of this
-     * size).
-     * 
-     * @param x the maximum block size
-     */
     public void setBlockSize(int x) {
         validateBlockSize(x);
         this.blockSize = x;
     }
     
-    /**
-     * Get the maximum block size.
-     * 
-     * @return the maximum block size
-     */
-    public int getBlockSize() {
-        return blockSize;
-    }
-    
-    /**
-     * Validate that the block size is larger than the lenght of a hash code.
-     * 
-     * @param x the size
-     */
     private static void validateBlockSize(int x) {
         if (x < BLOCK_SIZE_LIMIT) {
             throw new IllegalArgumentException(
@@ -176,6 +111,10 @@ public abstract class AbstractBlobStore 
         }
     }
 
+    public int getBlockSize() {
+        return blockSize;
+    }
+
     /**
      * Write a blob from a temporary file. The temporary file is removed
      * afterwards. A file based blob stores might simply rename the file, so
@@ -198,12 +137,6 @@ public abstract class AbstractBlobStore 
         }
     }
 
-    /**
-     * Write a binary, possibly splitting it into blocks and storing them.
-     * 
-     * @param in the input stream
-     * @return the data store id
-     */
     public String writeBlob(InputStream in) throws Exception {
         try {
             ByteArrayOutputStream idStream = new ByteArrayOutputStream();
@@ -222,40 +155,18 @@ public abstract class AbstractBlobStore 
         }
     }
 
-    /**
-     * Mark the data store id as 'current in use'.
-     * 
-     * @param blobId the id
-     */
     protected void usesBlobId(String blobId) {
         inUse.put(blobId, new WeakReference<String>(blobId));
     }
 
-    /**
-     * Clear the in-used map. This method is used for testing.
-     */
     public void clearInUse() {
         inUse.clear();
     }
 
-    /**
-     * Clear the cache to free up memory.
-     */
     public void clearCache() {
         cache.clear();
     }
 
-    /**
-     * Store a binary, possibly splitting it into blocks, and store the block
-     * ids in the id stream.
-     * 
-     * @param in the stream
-     * @param idStream the stream of block ids
-     * @param level the indirection level (0 if the binary is user data, 1 if
-     *            the data is a list of digests)
-     * @param totalLength the total length (if the data is a list of block ids)
-     * @throws Exception if storing failed
-     */
     private void convertBlobToId(InputStream in, ByteArrayOutputStream idStream, int level, long totalLength) throws Exception {
         byte[] block = new byte[blockSize];
         int count = 0;
@@ -320,35 +231,14 @@ public abstract class AbstractBlobStore 
      */
     protected abstract void storeBlock(byte[] digest, int level, byte[] data) throws Exception;
 
-    /**
-     * Start the mark phase of the data store garbage collection.
-     */
     public abstract void startMark() throws Exception;
 
-    /**
-     * Start the sweep phase of the data store garbage collection.
-     * 
-     * @return the number of removed blocks.
-     */
     public abstract int sweep() throws Exception;
 
-    /**
-     * Whether the mark phase has been started.
-     * 
-     * @return true if it was started
-     */
     protected abstract boolean isMarkEnabled();
 
-    /**
-     * Mark a block as 'in use'. This method is called in the mark phase.
-     * 
-     * @param id the block id
-     */
     protected abstract void mark(BlockId id) throws Exception;
 
-    /**
-     * Mark all blocks that are in the in-use map.
-     */
     protected void markInUse() throws Exception {
         for (String id : new ArrayList<String>(inUse.keySet())) {
             mark(id);
@@ -407,27 +297,11 @@ public abstract class AbstractBlobStore 
         }
     }
 
-    /**
-     * Read the block with the given digest. This method should not be
-     * overwritten by a subclass as it caches the data.
-     * 
-     * @param digest the digest
-     * @param pos the position within the block (usually 0)
-     * @return a byte array with the data (the byte array is not modified by the
-     *         caller, but might be cached)
-     */
     private byte[] readBlock(byte[] digest, long pos) throws Exception {
         BlockId id = new BlockId(digest, pos);
         return cache.get(id).data;
     }
 
-    /**
-     * Load a block from the backend. This method is called from the cache if
-     * the block is not in memory.
-     * 
-     * @param id the block id
-     * @return the data
-     */
     public Data load(BlockId id) {
         byte[] data;
         try {
@@ -482,11 +356,6 @@ public abstract class AbstractBlobStore 
         return totalLength;
     }
 
-    /**
-     * Mark a binary as 'in use'.
-     * 
-     * @param blobId the blob id
-     */
     protected void mark(String blobId) throws IOException {
         try {
             byte[] id = StringUtils.convertHexToBytes(blobId);
@@ -535,8 +404,8 @@ public abstract class AbstractBlobStore 
      */
     public static class BlockId {
 
-        private final byte[] digest;
-        private final long pos;
+        final byte[] digest;
+        final long pos;
 
         BlockId(byte[] digest, long pos) {
             this.digest = digest;
@@ -564,20 +433,10 @@ public abstract class AbstractBlobStore 
             return StringUtils.convertBytesToHex(digest) + "@" + pos;
         }
         
-        /**
-         * Get the digest (hash code).
-         * 
-         * @return the digest
-         */
         public byte[] getDigest() {
             return digest;
         }
         
-        /**
-         * Get the starting position within the block (usually 0).
-         * 
-         * @return the position
-         */
         public long getPos() {
             return pos;
         }
@@ -585,8 +444,7 @@ public abstract class AbstractBlobStore 
     }
 
     /**
-     * The data for a block. This class is only used within this class and the
-     * cache.
+     * The data for a block.
      */
     public static class Data implements Cache.Value {
 

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/DbBlobStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/DbBlobStore.java?rev=1409231&r1=1409230&r2=1409231&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/DbBlobStore.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/DbBlobStore.java Wed Nov 14 15:37:10 2012
@@ -104,7 +104,7 @@ public class DbBlobStore extends Abstrac
             PreparedStatement prep = conn.prepareStatement(
                     "select data from datastore_data where id = ?");
             try {
-                String id = StringUtils.convertBytesToHex(blockId.getDigest());
+                String id = StringUtils.convertBytesToHex(blockId.digest);
                 prep.setString(1, id);
                 ResultSet rs = prep.executeQuery();
                 if (!rs.next()) {
@@ -112,15 +112,15 @@ public class DbBlobStore extends Abstrac
                 }
                 byte[] data = rs.getBytes(1);
                 // System.out.println("    read block " + id + " blockLen: " + data.length + " [0]: " + data[0]);
-                if (blockId.getPos() == 0) {
+                if (blockId.pos == 0) {
                     return data;
                 }
-                int len = (int) (data.length - blockId.getPos());
+                int len = (int) (data.length - blockId.pos);
                 if (len < 0) {
                     return new byte[0];
                 }
                 byte[] d2 = new byte[len];
-                System.arraycopy(data, (int) blockId.getPos(), d2, 0, len);
+                System.arraycopy(data, (int) blockId.pos, d2, 0, len);
                 return d2;
             } finally {
                 prep.close();
@@ -148,7 +148,7 @@ public class DbBlobStore extends Abstrac
         }
         Connection conn = cp.getConnection();
         try {
-            String id = StringUtils.convertBytesToHex(blockId.getDigest());
+            String id = StringUtils.convertBytesToHex(blockId.digest);
             PreparedStatement prep = conn.prepareStatement(
                     "update datastore_meta set lastMod = ? where id = ? and lastMod < ?");
             prep.setLong(1, System.currentTimeMillis());

Modified: jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/FileBlobStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/FileBlobStore.java?rev=1409231&r1=1409230&r2=1409231&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/FileBlobStore.java (original)
+++ jackrabbit/oak/trunk/oak-mk/src/main/java/org/apache/jackrabbit/mk/blobs/FileBlobStore.java Wed Nov 14 15:37:10 2012
@@ -42,7 +42,7 @@ public class FileBlobStore extends Abstr
 
     // TODO file operations are not secure (return values not checked, no retry,...)
 
-    public FileBlobStore(String dir) {
+    public FileBlobStore(String dir) throws IOException {
         baseDir = new File(dir);
         baseDir.mkdirs();
     }
@@ -116,18 +116,18 @@ public class FileBlobStore extends Abstr
 
     @Override
     protected byte[] readBlockFromBackend(BlockId id) throws IOException {
-        File f = getFile(id.getDigest(), false);
+        File f = getFile(id.digest, false);
         if (!f.exists()) {
-            File old = getFile(id.getDigest(), true);
+            File old = getFile(id.digest, true);
             f.getParentFile().mkdir();
             old.renameTo(f);
-            f = getFile(id.getDigest(), false);
+            f = getFile(id.digest, false);
         }
         int length = (int) Math.min(f.length(), getBlockSize());
         byte[] data = new byte[length];
         InputStream in = new FileInputStream(f);
         try {
-            IOUtils.skipFully(in, id.getPos());
+            IOUtils.skipFully(in, id.pos);
             IOUtils.readFully(in, data, 0, length);
         } finally {
             in.close();
@@ -168,12 +168,12 @@ public class FileBlobStore extends Abstr
 
     @Override
     protected void mark(BlockId id) throws IOException {
-        File f = getFile(id.getDigest(), false);
+        File f = getFile(id.digest, false);
         if (!f.exists()) {
-            File old = getFile(id.getDigest(), true);
+            File old = getFile(id.digest, true);
             f.getParentFile().mkdir();
             old.renameTo(f);
-            f = getFile(id.getDigest(), false);
+            f = getFile(id.digest, false);
         }
     }