You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by am...@apache.org on 2016/07/19 03:48:39 UTC

svn commit: r1753332 - in /jackrabbit/oak/trunk: oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/ oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/data...

Author: amitj
Date: Tue Jul 19 03:48:38 2016
New Revision: 1753332

URL: http://svn.apache.org/viewvc?rev=1753332&view=rev
Log:
OAK-4573: S3 fetching record leads to multiple calls and background download

Introduced a new method to bypass optimizations done on getRecord for S3 as these are not needed during GC

Modified:
    jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java
    jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java

Modified: jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java?rev=1753332&r1=1753331&r2=1753332&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java (original)
+++ jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java Tue Jul 19 03:48:38 2016
@@ -655,7 +655,7 @@ public class S3Backend implements Shared
                 getClass().getClassLoader());
             ObjectMetadata meta = s3service.getObjectMetadata(bucket, addMetaKeyPrefix(name));
             return new S3DataRecord(s3service, bucket, name,
-                meta.getLastModified().getTime(), meta.getContentLength());
+                meta.getLastModified().getTime(), meta.getContentLength(), true);
         } finally {
             if (contextClassLoader != null) {
                 Thread.currentThread().setContextClassLoader(contextClassLoader);
@@ -674,7 +674,7 @@ public class S3Backend implements Shared
             ObjectListing prevObjectListing = s3service.listObjects(listObjectsRequest);
             for (final S3ObjectSummary s3ObjSumm : prevObjectListing.getObjectSummaries()) {
                 metadataList.add(new S3DataRecord(s3service, bucket, stripMetaKeyPrefix(s3ObjSumm.getKey()),
-                    s3ObjSumm.getLastModified().getTime(), s3ObjSumm.getSize()));
+                    s3ObjSumm.getLastModified().getTime(), s3ObjSumm.getSize(), true));
             }
         } finally {
             if (contextClassLoader != null) {
@@ -735,6 +735,35 @@ public class S3Backend implements Shared
             });
     }
 
+    @Override
+    public DataRecord getRecord(DataIdentifier identifier) throws DataStoreException {
+        long start = System.currentTimeMillis();
+        String key = getKeyName(identifier);
+        ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader();
+        try {
+            Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
+
+            ObjectMetadata object = s3service.getObjectMetadata(bucket, key);
+            S3DataRecord record = new S3DataRecord(s3service, bucket, identifier.toString(),
+                object.getLastModified().getTime(), object.getContentLength());
+            LOG.debug("Identifier [{}]'s getRecord = [{}] took [{}]ms.",
+                new Object[] {identifier, record, (System.currentTimeMillis() - start)});
+
+            return record;
+        } catch (AmazonServiceException e) {
+            if (e.getStatusCode() == 404 || e.getStatusCode() == 403) {
+                LOG.info(
+                    "getRecord:Identifier [{}] not found. Took [{}] ms.",
+                    identifier, (System.currentTimeMillis() - start));
+            }
+            throw new DataStoreException(e);
+        } finally {
+            if (contextClassLoader != null) {
+                Thread.currentThread().setContextClassLoader(contextClassLoader);
+            }
+        }
+    }
+
     /**
      * Returns an iterator over the S3 objects
      * @param <T>
@@ -830,13 +859,21 @@ public class S3Backend implements Shared
         private long length;
         private long lastModified;
         private String bucket;
+        private boolean isMeta;
+
+        public S3DataRecord(AmazonS3Client s3service, String bucket, String key, long lastModified,
+            long length) {
+            this(s3service, bucket, key, lastModified, length, false);
+        }
 
-        public S3DataRecord(AmazonS3Client s3service, String bucket, String key, long lastModified, long length) {
+        public S3DataRecord(AmazonS3Client s3service, String bucket, String key, long lastModified,
+            long length, boolean isMeta) {
             this.s3service = s3service;
             this.identifier = new DataIdentifier(key);
             this.lastModified = lastModified;
             this.length = length;
             this.bucket = bucket;
+            this.isMeta = isMeta;
         }
 
         @Override
@@ -856,13 +893,27 @@ public class S3Backend implements Shared
 
         @Override
         public InputStream getStream() throws DataStoreException {
-            return s3service.getObject(bucket, addMetaKeyPrefix(identifier.toString())).getObjectContent();
+            String id = getKeyName(identifier);
+            if (isMeta) {
+                id = addMetaKeyPrefix(identifier.toString());
+            }
+            return s3service.getObject(bucket, id).getObjectContent();
         }
 
         @Override
         public long getLastModified() {
             return lastModified;
         }
+
+        @Override
+        public String toString() {
+            return "S3DataRecord{" +
+                "identifier=" + identifier +
+                ", length=" + length +
+                ", lastModified=" + lastModified +
+                ", bucket='" + bucket + '\'' +
+                '}';
+        }
     }
 
     private void write(DataIdentifier identifier, File file,

Modified: jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java?rev=1753332&r1=1753331&r2=1753332&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java (original)
+++ jackrabbit/oak/trunk/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java Tue Jul 19 03:48:38 2016
@@ -20,6 +20,7 @@
 package org.apache.jackrabbit.oak.blob.cloud.aws.s3;
 
 import org.apache.jackrabbit.core.data.Backend;
+import org.apache.jackrabbit.core.data.DataIdentifier;
 import org.apache.jackrabbit.core.data.DataRecord;
 import org.apache.jackrabbit.core.data.DataStoreException;
 
@@ -90,4 +91,12 @@ public interface SharedS3Backend extends
      */
     Iterator<DataRecord> getAllRecords()
         throws DataStoreException;
+
+    /**
+     * Gets the record with the specified identifier
+     *
+     * @param id the record identifier
+     * @return the metadata DataRecord
+     */
+    DataRecord getRecord(DataIdentifier id) throws DataStoreException;
 }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java?rev=1753332&r1=1753331&r2=1753332&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java Tue Jul 19 03:48:38 2016
@@ -21,6 +21,7 @@ import java.io.InputStream;
 import java.util.Iterator;
 import java.util.List;
 
+import org.apache.jackrabbit.core.data.DataIdentifier;
 import org.apache.jackrabbit.core.data.DataRecord;
 import org.apache.jackrabbit.core.data.DataStoreException;
 
@@ -93,6 +94,14 @@ public interface SharedDataStore {
     Iterator<DataRecord> getAllRecords() throws DataStoreException;
 
     /**
+     * Retrieves the record for the given identifier
+     *
+     * @param id the if of the record
+     * @return data record
+     */
+    DataRecord getRecordForId(DataIdentifier id) throws DataStoreException;
+
+    /**
      * Gets the type.
      * 
      * @return the type

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java?rev=1753332&r1=1753331&r2=1753332&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java Tue Jul 19 03:48:38 2016
@@ -396,7 +396,7 @@ public class DataStoreBlobStore implemen
             for (String chunkId : chunkIds) {
                 String blobId = extractBlobId(chunkId);
                 DataIdentifier identifier = new DataIdentifier(blobId);
-                DataRecord dataRecord = delegate.getRecord(identifier);
+                DataRecord dataRecord = getRecordForId(identifier);
                 boolean success = (maxLastModifiedTime <= 0)
                         || dataRecord.getLastModified() <= maxLastModifiedTime;
                 log.trace("Deleting blob [{}] with last modified date [{}] : [{}]", blobId,
@@ -481,6 +481,14 @@ public class DataStoreBlobStore implemen
     }
 
     @Override
+    public DataRecord getRecordForId(DataIdentifier identifier) throws DataStoreException {
+        if (delegate instanceof SharedDataStore) {
+            return ((SharedDataStore) delegate).getRecordForId(identifier);
+        }
+        return delegate.getRecord(identifier);
+    }
+
+    @Override
     public Type getType() {
         if (delegate instanceof SharedDataStore) {
             return Type.SHARED;

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java?rev=1753332&r1=1753331&r2=1753332&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java Tue Jul 19 03:48:38 2016
@@ -251,6 +251,11 @@ public class OakFileDataStore extends Fi
     }
 
     @Override
+    public DataRecord getRecordForId(DataIdentifier id) throws DataStoreException {
+        return getRecord(id);
+    }
+
+    @Override
     public Type getType() {
         return Type.SHARED;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java?rev=1753332&r1=1753331&r2=1753332&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java Tue Jul 19 03:48:38 2016
@@ -19,6 +19,7 @@
 package org.apache.jackrabbit.oak.plugins.blob.datastore;
 
 import org.apache.jackrabbit.core.data.Backend;
+import org.apache.jackrabbit.core.data.DataIdentifier;
 import org.apache.jackrabbit.core.data.DataRecord;
 import org.apache.jackrabbit.core.data.DataStoreException;
 import org.apache.jackrabbit.oak.blob.cloud.aws.s3.S3Backend;
@@ -81,6 +82,11 @@ public class SharedS3DataStore extends S
     }
 
     @Override
+    public DataRecord getRecordForId(DataIdentifier identifier) throws DataStoreException {
+        return backend.getRecord(identifier);
+    }
+
+    @Override
     public Type getType() {
         return Type.SHARED;
     }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java?rev=1753332&r1=1753331&r2=1753332&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java Tue Jul 19 03:48:38 2016
@@ -20,6 +20,7 @@ package org.apache.jackrabbit.oak.plugin
 
 import static com.google.common.collect.Sets.newHashSet;
 import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertTrue;
 import static org.apache.jackrabbit.oak.plugins.blob.datastore.DataStoreUtils.cleanup;
 import static org.apache.jackrabbit.oak.plugins.blob.datastore.DataStoreUtils.getBlobStore;
 import static org.hamcrest.CoreMatchers.instanceOf;
@@ -30,6 +31,7 @@ import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Date;
+import java.util.Map;
 import java.util.Random;
 import java.util.Set;
 import java.util.UUID;
@@ -39,11 +41,15 @@ import javax.annotation.Nullable;
 
 import com.google.common.base.Function;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import junit.framework.Assert;
 
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
+import org.apache.jackrabbit.core.data.DataIdentifier;
 import org.apache.jackrabbit.core.data.DataRecord;
+import org.apache.jackrabbit.core.data.DataStoreException;
 import org.apache.jackrabbit.oak.commons.FileIOUtils;
 import org.apache.jackrabbit.oak.plugins.blob.datastore.DataStoreBlobStore;
 import org.apache.jackrabbit.oak.plugins.blob.datastore.SharedDataStoreUtils;
@@ -217,6 +223,55 @@ public class SharedDataStoreUtilsTest {
         assertEquals(added, retrieved);
     }
 
+    @Test
+    public void testStreamFromGetAllRecords() throws Exception {
+        dataStore = getBlobStore();
+        int number = 10;
+        Set<DataRecord> added = newHashSet();
+        for (int i = 0; i < number; i++) {
+            added.add(dataStore.addRecord(randomStream(i, 16516)));
+        }
+
+        Set<DataRecord> retrieved = newHashSet((dataStore.getAllRecords()));
+        assertRecords(added, retrieved);
+    }
+
+    @Test
+    public void testGetRecordForId() throws Exception {
+        dataStore = getBlobStore();
+        int number = 10;
+        Set<DataRecord> added = newHashSet();
+        for (int i = 0; i < number; i++) {
+            added.add(dataStore.addRecord(randomStream(i, 16516)));
+        }
+
+        Set<DataRecord> retrieved = newHashSet();
+        for (DataRecord rec : added) {
+            retrieved.add(dataStore.getRecordForId(rec.getIdentifier()));
+        }
+        assertRecords(added, retrieved);
+    }
+
+    private static void assertRecords(Set<DataRecord> expected, Set<DataRecord> retrieved)
+        throws DataStoreException, IOException {
+        //assert streams
+        Map<DataIdentifier, DataRecord> retMap = Maps.newHashMap();
+        for (DataRecord ret : retrieved) {
+            retMap.put(ret.getIdentifier(), ret);
+        }
+
+        for (DataRecord rec : expected) {
+            assertEquals("Record id different for " + rec.getIdentifier(),
+                rec.getIdentifier(), retMap.get(rec.getIdentifier()).getIdentifier());
+            assertEquals("Record length different for " + rec.getIdentifier(),
+                rec.getLength(), retMap.get(rec.getIdentifier()).getLength());
+            assertEquals("Record lastModified different for " + rec.getIdentifier(),
+                rec.getLastModified(), retMap.get(rec.getIdentifier()).getLastModified());
+            assertTrue("Record steam different for " + rec.getIdentifier(),
+                IOUtils.contentEquals(rec.getStream(), retMap.get(rec.getIdentifier()).getStream()));
+        }
+    }
+
     static InputStream randomStream(int seed, int size) {
         Random r = new Random(seed);
         byte[] data = new byte[size];