dhis2-devs team mailing list archive
-
dhis2-devs team
-
Mailing list archive
-
Message #40942
[Branch ~dhis2-devs-core/dhis2/trunk] Rev 20855: Improvements to file resource service layer. Added better logging as well as a temporary fail-saf...
------------------------------------------------------------
revno: 20855
committer: Halvdan Hoem Grelland <halvdanhg@xxxxxxxxx>
branch nick: dhis2
timestamp: Fri 2015-10-23 01:43:32 +0200
message:
Improvements to file resource service layer. Added better logging as well as a temporary fail-safe to correct issue with wrong storageStatus.
modified:
dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceContentStore.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/FileResourceUploadCallbackProvider.java
dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/JCloudsFileResourceContentStore.java
dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.java
--
lp:dhis2
https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk
Your team DHIS 2 developers is subscribed to branch lp:dhis2.
To unsubscribe from this branch go to https://code.launchpad.net/~dhis2-devs-core/dhis2/trunk/+edit-subscription
=== modified file 'dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceContentStore.java'
--- dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceContentStore.java 2015-10-12 18:52:30 +0000
+++ dhis-2/dhis-api/src/main/java/org/hisp/dhis/fileresource/FileResourceContentStore.java 2015-10-22 23:43:32 +0000
@@ -61,6 +61,13 @@
void deleteFileResourceContent( String key );
/**
+ * Check existence of a file.
+ * @param key key of the file.
+ * @return true if the file exists in the file store, false otherwise.
+ */
+ boolean fileResourceContentExists( String key );
+
+ /**
* Create a signed GET request which gives access to the content.
* @param key the key.
* @return a URI containing the signed GET request or null if signed requests are not supported.
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java 2015-10-13 22:07:31 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/DefaultFileResourceService.java 2015-10-22 23:43:32 +0000
@@ -29,11 +29,14 @@
*/
import com.google.common.io.ByteSource;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.hisp.dhis.common.GenericIdentifiableObjectStore;
import org.hisp.dhis.system.scheduling.Scheduler;
import org.joda.time.DateTime;
import org.joda.time.Duration;
import org.joda.time.Hours;
+import org.joda.time.Minutes;
import org.springframework.transaction.annotation.Transactional;
import org.springframework.util.concurrent.ListenableFuture;
@@ -50,13 +53,16 @@
public class DefaultFileResourceService
implements FileResourceService
{
+ private static final Log log = LogFactory.getLog( DefaultFileResourceService.class );
+
private static final String KEY_FILE_CLEANUP_TASK = "fileResourceCleanupTask";
+ private static final Duration IS_ORPHAN_TIME_DELTA = Hours.TWO.toStandardDuration();
+ private static final Duration LONG_STORAGE_DURATION_TIME_DELTA = Minutes.TWO.toStandardDuration();
+
private static final Predicate<FileResource> IS_ORPHAN_PREDICATE =
( fr -> !fr.isAssigned() || fr.getStorageStatus() != FileResourceStorageStatus.STORED );
- private static final Duration IS_ORPHAN_TIME_DELTA = Hours.TWO.toStandardDuration();
-
// -------------------------------------------------------------------------
// Dependencies
// -------------------------------------------------------------------------
@@ -114,10 +120,12 @@
// FileResourceService implementation
// -------------------------------------------------------------------------
+ @Transactional
@Override
public FileResource getFileResource( String uid )
{
- return fileResourceStore.getByUid( uid );
+ // TODO Consider need for ensureStorageStatus
+ return ensureStorageStatus( fileResourceStore.getByUid( uid ) );
}
@Override
@@ -201,4 +209,43 @@
return fileResourceContentStore.getSignedGetContentUri( fileResource.getStorageKey() );
}
+
+ // -------------------------------------------------------------------------
+ // Supportive methods
+ // -------------------------------------------------------------------------
+
+ /**
+ * Ensures that the storageStatus of the FileResource is correct.
+ * If it has been pending for more than two minutes existance of the content is
+ * 'double checked' in the file store.
+ * If the content is actually present the storageStatus is updated to reflect this.
+ *
+ * TODO Should not be necessary but needs to be in place as a fail-safe due to mysterious issues with saving.
+ */
+ private FileResource ensureStorageStatus( FileResource fileResource )
+ {
+ if ( FileResourceStorageStatus.PENDING == fileResource.getStorageStatus() )
+ {
+ Duration pendingDuration = new Duration( new DateTime( fileResource.getLastUpdated() ), DateTime.now() );
+
+ if ( pendingDuration.isLongerThan( LONG_STORAGE_DURATION_TIME_DELTA ) )
+ {
+ // Upload has been running for 2+ minutes and is still PENDING.
+ // Check if content has actually been stored and correct to STORED if this is the case.
+
+ boolean contentIsStored = fileResourceContentStore.fileResourceContentExists( fileResource.getStorageKey() );
+
+ if ( contentIsStored )
+ {
+ // Status is PENDING but content is actually stored. Fix it.
+ fileResource.setStorageStatus( FileResourceStorageStatus.STORED );
+ fileResourceStore.update( fileResource );
+ log.warn( "Corrected issue: File resource '" + fileResource.getUid() +
+ "' had storageStatus PENDING but content was fully stored." );
+ }
+ }
+ }
+
+ return fileResource;
+ }
}
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/FileResourceUploadCallbackProvider.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/FileResourceUploadCallbackProvider.java 2015-10-14 22:50:32 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/FileResourceUploadCallbackProvider.java 2015-10-22 23:43:32 +0000
@@ -31,6 +31,9 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.hisp.dhis.common.IdentifiableObjectManager;
+import org.joda.time.DateTime;
+import org.joda.time.Period;
+import org.joda.time.format.PeriodFormat;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.util.concurrent.ListenableFutureCallback;
@@ -48,38 +51,42 @@
{
return new ListenableFutureCallback<String>()
{
+ DateTime startTime = DateTime.now();
+
@Override
public void onFailure( Throwable ex )
{
- log.error( "Saving file content failed", ex );
-
- FileResource fetchedFileResource = idObjectManager.get( FileResource.class, fileResourceUid );
-
- if ( fetchedFileResource != null )
+ log.error( "Saving content for file resource '" + fileResourceUid + "' failed", ex );
+
+ FileResource fileResource = idObjectManager.get( FileResource.class, fileResourceUid );
+
+ if ( fileResource != null )
{
- fetchedFileResource.setStorageStatus( FileResourceStorageStatus.FAILED );
- idObjectManager.update( fetchedFileResource );
+ log.info( "File resource '" + fileResource.getUid() + "' storageStatus set to FAILED." );
+
+ fileResource.setStorageStatus( FileResourceStorageStatus.FAILED );
+ idObjectManager.update( fileResource );
}
}
@Override
public void onSuccess( String result )
{
- log.info( "File content stored: " + result );
-
- FileResource fetchedFileResource = idObjectManager.get( FileResource.class, fileResourceUid );
-
- if ( result != null && fetchedFileResource != null )
+ Period timeDiff = new Period( startTime, DateTime.now() );
+
+ log.info( "File stored with key: '" + result + "'. Upload finished in " + timeDiff.toString( PeriodFormat.getDefault() ) );
+
+ FileResource fileResource = idObjectManager.get( FileResource.class, fileResourceUid );
+
+ if ( result != null && fileResource != null )
{
- fetchedFileResource.setStorageStatus( FileResourceStorageStatus.STORED );
+ fileResource.setStorageStatus( FileResourceStorageStatus.STORED );
+ idObjectManager.update( fileResource );
}
else
{
- log.error( "Conflict: content was stored but FileResource with uid: " + fileResourceUid + " could not be found." );
- return;
+ log.error( "Conflict: content was stored but FileResource with uid '" + fileResourceUid + "' could not be found." );
}
-
- idObjectManager.update( fetchedFileResource );
}
};
}
=== modified file 'dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/JCloudsFileResourceContentStore.java'
--- dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/JCloudsFileResourceContentStore.java 2015-10-14 22:50:32 +0000
+++ dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/fileresource/JCloudsFileResourceContentStore.java 2015-10-22 23:43:32 +0000
@@ -285,6 +285,12 @@
}
@Override
+ public boolean fileResourceContentExists( String key )
+ {
+ return blobExists( key );
+ }
+
+ @Override
public URI getSignedGetContentUri( String key )
{
BlobRequestSigner signer = blobStoreContext.getSigner();
@@ -317,6 +323,11 @@
return blobStore.getBlob( container, key );
}
+ private boolean blobExists( String key )
+ {
+ return key != null && blobStore.blobExists( container, key );
+ }
+
private void deleteBlob( String key )
{
blobStore.removeBlob( container, key );
@@ -336,9 +347,10 @@
if ( cause != null && cause instanceof UserPrincipalNotFoundException )
{
- // Intentionally ignored exception which occurs with JClouds on non-English
- // Windows installations while trying to resolve the "Everyone" group.
- log.debug( "Ignored UserPrincipalNotFoundException as a workaround for bug with JClouds filesystem provider on Windows" );
+ // Intentionally ignored exception which occurs with JClouds on localized
+ // Windows systems while trying to resolve the "Everyone" group.
+ // See https://issues.apache.org/jira/browse/JCLOUDS-1015
+ log.debug( "Ignored UserPrincipalNotFoundException. Workaround for JClouds bug 'JCLOUDS-1015'." );
}
else
{
=== modified file 'dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.java'
--- dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.java 2015-10-12 18:52:30 +0000
+++ dhis-2/dhis-web/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/DataValueController.java 2015-10-22 23:43:32 +0000
@@ -454,12 +454,15 @@
throw new WebMessageException( WebMessageUtils.notFound( "A data value file resource with id " + uid + " does not exist." ) );
}
- if ( fileResource.getStorageStatus() != FileResourceStorageStatus.STORED )
+ FileResourceStorageStatus storageStatus = fileResource.getStorageStatus();
+
+ if ( storageStatus != FileResourceStorageStatus.STORED )
{
// Special case:
// The FileResource exists and has been tied to this DataValue, however, the underlying file
- // content is still in transit (PENDING) to the (most likely external) file store provider.
+ // content is still not stored to the (most likely external) file store provider.
+ // HTTP 409, for lack of a more suitable status code
WebMessage webMessage = WebMessageUtils.conflict( "The content is being processed and is not available yet. Try again later.",
"The content requested is in transit to the file store and will be available at a later time." );
webMessage.setResponse( new FileResourceWebMessageResponse( fileResource ) );
