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 2017/05/26 11:26:05 UTC
svn commit: r1796274 - in /jackrabbit/oak/trunk:
oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/
oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/
oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/
Author: amitj
Date: Fri May 26 11:26:05 2017
New Revision: 1796274
URL: http://svn.apache.org/viewvc?rev=1796274&view=rev
Log:
OAK-5935: AbstractSharedCachingDataStore#getRecordIfStored should use the underlying cache.get
- The call now returns a FileCachedDataRecord for cases where the cache did not have any entry but the backend has and lazily loads the input stream forcing load through the cache so that the cache is loaded
- Added test lazyLoadStream
- Fixed an issue with the test which would keep a stale reference to a file
Modified:
jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/AbstractSharedCachingDataStore.java
jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java
jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/CachingDataStoreTest.java
jackrabbit/oak/trunk/oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/AbstractDataRecord.java
Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/AbstractSharedCachingDataStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/AbstractSharedCachingDataStore.java?rev=1796274&r1=1796273&r2=1796274&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/AbstractSharedCachingDataStore.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/main/java/org/apache/jackrabbit/oak/plugins/blob/AbstractSharedCachingDataStore.java Fri May 26 11:26:05 2017
@@ -192,18 +192,20 @@ public abstract class AbstractSharedCach
public DataRecord getRecordIfStored(DataIdentifier dataIdentifier)
throws DataStoreException {
// Return file attributes from cache only if corresponding file is cached
- // This avoids downloading the file for just accessing the meta data
+ // This avoids downloading the file for just accessing the meta data.
File cached = cache.getIfPresent(dataIdentifier.toString());
if (cached != null && cached.exists()) {
return new FileCacheDataRecord(this, backend, dataIdentifier, cached.length(),
cached.lastModified());
- }
-
- // File not in cache so, retrieve the meta data from the backend explicitly
- try {
- return backend.getRecord(dataIdentifier);
- } catch (Exception e) {
- LOG.error("Error retrieving record [{}] from backend", dataIdentifier, e);
+ } else {
+ // Return the metadata from backend and lazily load the stream
+ try {
+ DataRecord rec = backend.getRecord(dataIdentifier);
+ return new FileCacheDataRecord(this, backend, dataIdentifier, rec.getLength(),
+ rec.getLastModified());
+ } catch (Exception e) {
+ LOG.error("Error retrieving record [{}]", dataIdentifier, e);
+ }
}
return null;
}
@@ -306,8 +308,21 @@ public abstract class AbstractSharedCach
@Override
public InputStream getStream() throws DataStoreException {
+ File cached = null;
+ // Need a catch as there's a possibility of eviction of this from cache
try {
- return new FileInputStream(store.cache.get(getIdentifier().toString()));
+ cached = store.cache.get(getIdentifier().toString());
+ } catch (final Exception e) {
+ LOG.debug("Error retrieving from cache " + getIdentifier(), e);
+ }
+
+ try {
+ // If cache configured to 0 will return null
+ if (cached == null || !cached.exists()) {
+ return backend.getRecord(getIdentifier()).getStream();
+ } else {
+ return new FileInputStream(cached);
+ }
} catch (final Exception e) {
throw new DataStoreException(
"Error opening input stream for identifier " + getIdentifier(), e);
Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java?rev=1796274&r1=1796273&r2=1796274&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/AbstractDataStoreCacheTest.java Fri May 26 11:26:05 2017
@@ -252,13 +252,14 @@ public class AbstractDataStoreCacheTest
@Override public void write(DataIdentifier identifier, File file)
throws DataStoreException {
+ File backendFile = getFile(identifier.toString(), root);
if (file != null && file.exists()) {
try {
- FileUtils.copyFile(file, getFile(identifier.toString(), root));
+ FileUtils.copyFile(file, backendFile);
} catch (IOException e) {
throw new DataStoreException(e);
}
- _backend.put(identifier, file);
+ _backend.put(identifier, backendFile);
} else {
throw new DataStoreException(
String.format("file %s of id %s", new Object[] {file, identifier.toString()}));
Modified: jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/CachingDataStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/CachingDataStoreTest.java?rev=1796274&r1=1796273&r2=1796274&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/CachingDataStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-blob-plugins/src/test/java/org/apache/jackrabbit/oak/plugins/blob/CachingDataStoreTest.java Fri May 26 11:26:05 2017
@@ -255,7 +255,7 @@ public class CachingDataStoreTest extend
}
/**
- * {@link CompositeDataStoreCache#getIfPresent(String)} when no cache.
+ * {@link CompositeDataStoreCache#getIfPresent(String)} when no record.
*/
@Test
public void getRecordNotAvailable() throws DataStoreException {
@@ -268,6 +268,53 @@ public class CachingDataStoreTest extend
}
/**
+ * Add in datastore, invalidate from cache and lazy load record stream.
+ */
+ @Test
+ public void lazyLoadStream() throws Exception {
+ LOG.info("Starting lazyLoadStream");
+
+ File f = copyToFile(randomStream(0, 4 * 1024), folder.newFile());
+ String id = getIdForInputStream(f);
+ FileInputStream fin = new FileInputStream(f);
+ closer.register(fin);
+
+ DataRecord rec = dataStore.addRecord(fin);
+ assertEquals(id, rec.getIdentifier().toString());
+
+ //start & finish
+ taskLatch.countDown();
+ callbackLatch.countDown();
+ waitFinish();
+
+ // Invalidate from the local cache
+ dataStore.getCache().invalidate(id);
+
+ // retrieve record from the datastore
+ rec = dataStore.getRecordIfStored(new DataIdentifier(id));
+ assertNotNull(rec);
+ assertEquals(id, rec.getIdentifier().toString());
+
+ // the file should not be in cache
+ File cached = dataStore.getCache().getIfPresent(id);
+ assertNull(cached);
+
+ // assert stream
+ assertFile(rec.getStream(), f, folder);
+
+ // Now should be available in the cache
+ cached = dataStore.getCache().getIfPresent(id);
+ assertNotNull(cached);
+ assertTrue(Files.equal(f, cached));
+
+ dataStore.deleteRecord(new DataIdentifier(id));
+ rec = dataStore.getRecordIfStored(new DataIdentifier(id));
+ assertNull(rec);
+
+ LOG.info("Finished lazyLoadStream");
+ }
+
+ /**
* {@link CompositeDataStoreCache#get(String)} when no cache.
* @throws IOException
*/
Modified: jackrabbit/oak/trunk/oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/AbstractDataRecord.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/AbstractDataRecord.java?rev=1796274&r1=1796273&r2=1796274&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/AbstractDataRecord.java (original)
+++ jackrabbit/oak/trunk/oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/AbstractDataRecord.java Fri May 26 11:26:05 2017
@@ -29,7 +29,7 @@ public abstract class AbstractDataRecord
/**
* The data store that contains this record.
*/
- private final AbstractSharedBackend backend;
+ protected final AbstractSharedBackend backend;
/**
* The binary identifier;