Author: amitj Date: Tue Aug 2 04:46:39 2016 New Revision: 1754815 URL: http://svn.apache.org/viewvc?rev=1754815&view=rev Log: OAK-4573: S3 fetching record leads to multiple calls and background download Merged r1753332 from trunk
Modified: jackrabbit/oak/branches/1.4/ (props changed) jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java Propchange: jackrabbit/oak/branches/1.4/ ------------------------------------------------------------------------------ --- svn:mergeinfo (original) +++ svn:mergeinfo Tue Aug 2 04:46:39 2016 @@ -1,3 +1,3 @@ /jackrabbit/oak/branches/1.0:1665962 -/jackrabbit/oak/trunk:1733615,1733875,1733913,1733929,1734230,1734254,1734279,1734941,1735052,1735405,1735484,1735549,1735564,1735588,1735622,1735638,1735919,1735983,1736176,1737309-1737310,1737334,1737349,1737998,1738004,1738775,1738795,1738833,1738950,1738957,1738963,1739894,1740116,1740625-1740626,1740971,1741032,1741339,1741343,1742520,1742888,1742916,1743097,1743172,1743343,1744265,1744959,1745038,1745197,1745368,1746086,1746117,1746342,1746345,1746696,1746981,1747341-1747342,1747492,1747512,1748505,1748553,1748722,1748870,1749275,1749350,1749464,1749475,1749645,1749662,1749815,1749872,1749875,1749899,1750052,1750076-1750077,1750287,1750457,1750462,1750465,1750495,1750626,1750809,1750886,1751410,1751478,1751755,1751871,1752273-1752274,1752438,1752447,1752508,1752616,1752659,1752672,1753262,1753331,1753444,1754117,1754239 +/jackrabbit/oak/trunk:1733615,1733875,1733913,1733929,1734230,1734254,1734279,1734941,1735052,1735405,1735484,1735549,1735564,1735588,1735622,1735638,1735919,1735983,1736176,1737309-1737310,1737334,1737349,1737998,1738004,1738775,1738795,1738833,1738950,1738957,1738963,1739894,1740116,1740625-1740626,1740971,1741032,1741339,1741343,1742520,1742888,1742916,1743097,1743172,1743343,1744265,1744959,1745038,1745197,1745368,1746086,1746117,1746342,1746345,1746696,1746981,1747341-1747342,1747492,1747512,1748505,1748553,1748722,1748870,1749275,1749350,1749464,1749475,1749645,1749662,1749815,1749872,1749875,1749899,1750052,1750076-1750077,1750287,1750457,1750462,1750465,1750495,1750626,1750809,1750886,1751410,1751478,1751755,1751871,1752273-1752274,1752438,1752447,1752508,1752616,1752659,1752672,1753262,1753331-1753332,1753444,1754117,1754239 /jackrabbit/trunk:1345480 Modified: jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java?rev=1754815&r1=1754814&r2=1754815&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java (original) +++ jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/S3Backend.java Tue Aug 2 04:46:39 2016 @@ -642,7 +642,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); @@ -661,7 +661,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) { @@ -722,6 +722,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> @@ -817,13 +846,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 @@ -843,13 +880,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/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java?rev=1754815&r1=1754814&r2=1754815&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java (original) +++ jackrabbit/oak/branches/1.4/oak-blob-cloud/src/main/java/org/apache/jackrabbit/oak/blob/cloud/aws/s3/SharedS3Backend.java Tue Aug 2 04:46:39 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/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java?rev=1754815&r1=1754814&r2=1754815&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStore.java Tue Aug 2 04:46:39 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; @@ -94,6 +95,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/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java?rev=1754815&r1=1754814&r2=1754815&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/DataStoreBlobStore.java Tue Aug 2 04:46:39 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/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java?rev=1754815&r1=1754814&r2=1754815&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/OakFileDataStore.java Tue Aug 2 04:46:39 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/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java?rev=1754815&r1=1754814&r2=1754815&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/blob/datastore/SharedS3DataStore.java Tue Aug 2 04:46:39 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/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java?rev=1754815&r1=1754814&r2=1754815&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/blob/SharedDataStoreUtilsTest.java Tue Aug 2 04:46:39 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];