Skip to content

Commit

Permalink
Ability to switch temporary storage to use either memory or disk (#166)
Browse files Browse the repository at this point in the history
* Ability to switch temporary storage to use either memory or disk

Signed-off-by: Victor Chang <vicchang@nvidia.com>

* Fix configuration name for temp data storage.

Signed-off-by: Victor Chang <vicchang@nvidia.com>

* Validate storage configurations based on temp storage location

Signed-off-by: Victor Chang <vicchang@nvidia.com>

* Log time for a payload to complete end-to-end within the service.

Signed-off-by: Victor Chang <vicchang@nvidia.com>

Signed-off-by: Victor Chang <vicchang@nvidia.com>
  • Loading branch information
mocsharp authored Sep 23, 2022
1 parent 5b05935 commit 09887b1
Show file tree
Hide file tree
Showing 26 changed files with 353 additions and 80 deletions.
2 changes: 2 additions & 0 deletions src/Api/Storage/Payload.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public enum PayloadState

public bool HasTimedOut { get => ElapsedTime().TotalSeconds >= Timeout; }

public TimeSpan Elapsed { get { return DateTime.UtcNow.Subtract(DateTimeCreated); } }

public string CallingAeTitle { get => Files.OfType<DicomFileStorageMetadata>().Select(p => p.CallingAeTitle).FirstOrDefault(); }

public string CalledAeTitle { get => Files.OfType<DicomFileStorageMetadata>().Select(p => p.CalledAeTitle).FirstOrDefault(); }
Expand Down
25 changes: 22 additions & 3 deletions src/Api/Storage/StorageObjectMetadata.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

using System;
using System.IO;
using System.Runtime;
using System.Text.Json.Serialization;
using Ardalis.GuardClauses;

Expand Down Expand Up @@ -123,9 +124,27 @@ public void SetUploaded(string bucketName)

if (Data is not null && Data.CanSeek)
{
Data.Close();
Data.Dispose();
Data = null;
if (Data is FileStream fileStream)
{
var filename = fileStream.Name;
Data.Close();
Data.Dispose();
Data = null;
System.IO.File.Delete(filename);
}
else // MemoryStream
{
Data.Close();
Data.Dispose();
Data = null;

// When IG stores all received/downloaded data in-memory using MemoryStream, LOH grows tremendously and thus impacts the performance and
// memory usage. The following makes sure LOH is compacted after the data is uploaded.
GCSettings.LargeObjectHeapCompactionMode = GCLargeObjectHeapCompactionMode.CompactOnce;
#pragma warning disable S1215 // "GC.Collect" should not be called
GC.Collect();
#pragma warning restore S1215 // "GC.Collect" should not be called
}
}
}

Expand Down
38 changes: 37 additions & 1 deletion src/Configuration/ConfigurationValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

using System;
using System.Collections.Generic;
using System.IO;
using System.IO.Abstractions;
using System.Text.RegularExpressions;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
Expand All @@ -29,16 +31,18 @@ namespace Monai.Deploy.InformaticsGateway.Configuration
public class ConfigurationValidator : IValidateOptions<InformaticsGatewayConfiguration>
{
private readonly ILogger<ConfigurationValidator> _logger;
private readonly IFileSystem _fileSystem;
private readonly List<string> _validationErrors;

/// <summary>
/// Initializes an instance of the <see cref="ConfigurationValidator"/> class.
/// </summary>
/// <param name="configuration">InformaticsGatewayConfiguration to be validated</param>
/// <param name="logger">Logger to be used by ConfigurationValidator</param>
public ConfigurationValidator(ILogger<ConfigurationValidator> logger)
public ConfigurationValidator(ILogger<ConfigurationValidator> logger, IFileSystem fileSystem)
{
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
_fileSystem = fileSystem ?? throw new ArgumentNullException(nameof(fileSystem));
_validationErrors = new List<string>();
}

Expand Down Expand Up @@ -82,6 +86,38 @@ private bool IsStorageValid(StorageConfiguration storage)
valid &= IsValueInRange("InformaticsGateway>storage>reserveSpaceGB", 1, 999, storage.ReserveSpaceGB);
valid &= IsValueInRange("InformaticsGateway>storage>payloadProcessThreads", 1, 128, storage.PayloadProcessThreads);
valid &= IsValueInRange("InformaticsGateway>storage>concurrentUploads", 1, 128, storage.ConcurrentUploads);
valid &= IsValueInRange("InformaticsGateway>storage>memoryThreshold", 1, int.MaxValue, storage.MemoryThreshold);

if (storage.TemporaryDataStorage == TemporaryDataStorageLocation.Disk)
{
valid &= IsNotNullOrWhiteSpace("InformaticsGateway>storage>bufferRootPath", storage.BufferStorageRootPath);
valid &= IsValidDirectory("InformaticsGateway>storage>bufferRootPath", storage.BufferStorageRootPath);
valid &= IsValueInRange("InformaticsGateway>storage>bufferSize", 1, int.MaxValue, storage.BufferSize);
}

return valid;
}

private bool IsValidDirectory(string source, string directory)
{
var valid = true;
try
{
if (!_fileSystem.Directory.Exists(directory))
{
valid = false;
_validationErrors.Add($"Directory `{directory}` specified in `{source}` does not exist.");
}
else
{
using var _ = _fileSystem.File.Create(Path.Combine(directory, Path.GetRandomFileName()), 1, FileOptions.DeleteOnClose);
}
}
catch (Exception ex)
{
valid = false;
_validationErrors.Add($"Directory `{directory}` specified in `{source}` is not accessible: {ex.Message}.");
}
return valid;
}

Expand Down
15 changes: 15 additions & 0 deletions src/Configuration/StorageConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,28 @@ namespace Monai.Deploy.InformaticsGateway.Configuration
{
public class StorageConfiguration : StorageServiceConfiguration
{
/// <summary>
/// Gets or sets whether to store temporary data in <c>Memory</c> or on <c>Disk</c>.
/// Defaults to <c>Memory</c>.
/// </summary>
[ConfigurationKeyName("tempStorageLocation")]
public TemporaryDataStorageLocation TemporaryDataStorage { get; set; } = TemporaryDataStorageLocation.Memory;

/// <summary>
/// Gets or sets the path used for buffering incoming data.
/// Defaults to <c>./temp</c>.
/// </summary>
[ConfigurationKeyName("bufferRootPath")]
public string BufferStorageRootPath { get; set; } = "./temp";

/// <summary>
/// Gets or sets the number of bytes buffered for reads and writes to the temporary file.
/// Defaults to <c>128000</c>.
/// </summary>
[ConfigurationKeyName("bufferSize")]
public int BufferSize { get; set; } = 128000;


/// <summary>
/// Gets or set the maximum memory buffer size in bytes with default to 30MiB.
/// </summary>
Expand Down
24 changes: 24 additions & 0 deletions src/Configuration/TemporaryDataStorageLocation.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright 2022 MONAI Consortium
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

namespace Monai.Deploy.InformaticsGateway.Configuration
{
public enum TemporaryDataStorageLocation
{
Memory,
Disk
}
}
38 changes: 31 additions & 7 deletions src/Configuration/Test/ConfigurationValidatorTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/

using System;
using System.IO;
using System.IO.Abstractions;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Monai.Deploy.InformaticsGateway.SharedTest;
Expand All @@ -26,17 +28,19 @@ namespace Monai.Deploy.InformaticsGateway.Configuration.Test
public class ConfigurationValidatorTest
{
private readonly Mock<ILogger<ConfigurationValidator>> _logger;
private readonly Mock<IFileSystem> _fileSystem;

public ConfigurationValidatorTest()
{
_logger = new Mock<ILogger<ConfigurationValidator>>();
_fileSystem = new Mock<IFileSystem>();
}

[Fact(DisplayName = "ConfigurationValidator test with all valid settings")]
public void AllValid()
{
var config = MockValidConfiguration();
var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);
Assert.True(valid == ValidateOptionsResult.Success);
}

Expand All @@ -46,7 +50,7 @@ public void InvalidScpPort()
var config = MockValidConfiguration();
config.Dicom.Scp.Port = Int32.MaxValue;

var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessage = $"Invalid port number '{Int32.MaxValue}' specified for InformaticsGateway>dicom>scp>port.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -59,7 +63,7 @@ public void InvalidScpMaxAssociations()
var config = MockValidConfiguration();
config.Dicom.Scp.MaximumNumberOfAssociations = 0;

var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessage = $"Value of InformaticsGateway>dicom>scp>max-associations must be between {1} and {1000}.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -72,7 +76,7 @@ public void StorageWithInvalidWatermark()
var config = MockValidConfiguration();
config.Storage.Watermark = 1000;

var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessage = "Value of InformaticsGateway>storage>watermark must be between 1 and 100.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -85,7 +89,7 @@ public void StorageWithInvalidReservedSpace()
var config = MockValidConfiguration();
config.Storage.ReserveSpaceGB = 9999;

var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessage = "Value of InformaticsGateway>storage>reserveSpaceGB must be between 1 and 999.";
Assert.Equal(validationMessage, valid.FailureMessage);
Expand All @@ -98,7 +102,7 @@ public void StorageWithInvalidTemporaryBucketName()
var config = MockValidConfiguration();
config.Storage.TemporaryStorageBucket = " ";

var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessages = new[] { "Value for InformaticsGateway>storage>temporaryBucketName is required.", "Value for InformaticsGateway>storage>temporaryBucketName does not conform to Amazon S3 bucket naming requirements." };
Assert.Equal(string.Join(Environment.NewLine, validationMessages), valid.FailureMessage);
Expand All @@ -114,7 +118,7 @@ public void StorageWithInvalidBucketName()
var config = MockValidConfiguration();
config.Storage.StorageServiceBucketName = "";

var valid = new ConfigurationValidator(_logger.Object).Validate("", config);
var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessages = new[] { "Value for InformaticsGateway>storage>bucketName is required.", "Value for InformaticsGateway>storage>bucketName does not conform to Amazon S3 bucket naming requirements." };
Assert.Equal(string.Join(Environment.NewLine, validationMessages), valid.FailureMessage);
Expand All @@ -124,6 +128,26 @@ public void StorageWithInvalidBucketName()
}
}

[Fact(DisplayName = "ConfigurationValidator test with inaccessible directory")]
public void StorageWithInaccessbleDirectory()
{
_fileSystem.Setup(p => p.Directory.Exists(It.IsAny<string>())).Returns(true);
_fileSystem.Setup(p => p.File.Create(It.IsAny<string>(), It.IsAny<int>(), It.IsAny<FileOptions>())).Throws(new UnauthorizedAccessException("error"));

var config = MockValidConfiguration();
config.Storage.TemporaryDataStorage = TemporaryDataStorageLocation.Disk;
config.Storage.BufferStorageRootPath = "/blabla";

var valid = new ConfigurationValidator(_logger.Object, _fileSystem.Object).Validate("", config);

var validationMessages = new[] { $"Directory `/blabla` specified in `InformaticsGateway>storage>bufferRootPath` is not accessible: error." };
Assert.Equal(string.Join(Environment.NewLine, validationMessages), valid.FailureMessage);
foreach (var message in validationMessages)
{
_logger.VerifyLogging(message, LogLevel.Error, Times.Once());
}
}

private static InformaticsGatewayConfiguration MockValidConfiguration()
{
var config = new InformaticsGatewayConfiguration();
Expand Down
1 change: 1 addition & 0 deletions src/Database/PayloadConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public void Configure(EntityTypeBuilder<Payload> builder)
builder.Ignore(j => j.CalledAeTitle);
builder.Ignore(j => j.CallingAeTitle);
builder.Ignore(j => j.HasTimedOut);
builder.Ignore(j => j.Elapsed);
builder.Ignore(j => j.Count);
}
}
Expand Down
75 changes: 63 additions & 12 deletions src/InformaticsGateway/Common/FileStorageMetadataExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,41 +14,92 @@
* limitations under the License.
*/

using System.IO;
using System.IO.Abstractions;
using System.Text;
using System.Threading.Tasks;
using Ardalis.GuardClauses;
using FellowOakDicom;
using Monai.Deploy.InformaticsGateway.Api.Storage;
using Monai.Deploy.InformaticsGateway.Configuration;

namespace Monai.Deploy.InformaticsGateway.Common
{
internal static class FileStorageMetadataExtensions
{
public static async Task SetDataStreams(this DicomFileStorageMetadata dicomFileStorageMetadata, DicomFile dicomFile, string dicomJson)
public static async Task SetDataStreams(
this DicomFileStorageMetadata dicomFileStorageMetadata,
DicomFile dicomFile,
string dicomJson,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
{
Guard.Against.Null(dicomFile, nameof(dicomFile));
Guard.Against.Null(dicomJson, nameof(dicomJson)); // allow empty here

dicomFileStorageMetadata.File.Data = new MemoryStream();
switch (storageLocation)
{
case TemporaryDataStorageLocation.Disk:
Guard.Against.Null(fileSystem, nameof(fileSystem));
Guard.Against.NullOrWhiteSpace(temporaryStoragePath, nameof(temporaryStoragePath));

var tempFile = fileSystem.Path.Combine(temporaryStoragePath, $@"{System.DateTime.UtcNow.Ticks}.tmp");
dicomFileStorageMetadata.File.Data = fileSystem.File.Create(tempFile);
break;
default:
dicomFileStorageMetadata.File.Data = new System.IO.MemoryStream();
break;
}

await dicomFile.SaveAsync(dicomFileStorageMetadata.File.Data).ConfigureAwait(false);
dicomFileStorageMetadata.File.Data.Seek(0, SeekOrigin.Begin);
dicomFileStorageMetadata.File.Data.Seek(0, System.IO.SeekOrigin.Begin);

SetTextStream(dicomFileStorageMetadata.JsonFile, dicomJson);
await SetTextStream(dicomFileStorageMetadata.JsonFile, dicomJson, storageLocation, fileSystem, temporaryStoragePath);
}

public static void SetDataStream(this FhirFileStorageMetadata fhirFileStorageMetadata, string json)
=> SetTextStream(fhirFileStorageMetadata.File, json);
public static async Task SetDataStream(
this FhirFileStorageMetadata fhirFileStorageMetadata,
string json,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
=> await SetTextStream(fhirFileStorageMetadata.File, json, storageLocation, fileSystem, temporaryStoragePath);

public static void SetDataStream(this Hl7FileStorageMetadata hl7FileStorageMetadata, string message)
=> SetTextStream(hl7FileStorageMetadata.File, message);
public static async Task SetDataStream(
this Hl7FileStorageMetadata hl7FileStorageMetadata,
string message,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
=> await SetTextStream(hl7FileStorageMetadata.File, message, storageLocation, fileSystem, temporaryStoragePath);

private static void SetTextStream(StorageObjectMetadata storageObjectMetadata, string message)
private static async Task SetTextStream(
StorageObjectMetadata storageObjectMetadata,
string message,
TemporaryDataStorageLocation storageLocation,
IFileSystem fileSystem = null,
string temporaryStoragePath = "")
{
Guard.Against.Null(message, nameof(message)); // allow empty here

storageObjectMetadata.Data = new MemoryStream(Encoding.UTF8.GetBytes(message));
storageObjectMetadata.Data.Seek(0, SeekOrigin.Begin);
switch (storageLocation)
{
case TemporaryDataStorageLocation.Disk:
Guard.Against.Null(fileSystem, nameof(fileSystem));
Guard.Against.NullOrWhiteSpace(temporaryStoragePath, nameof(temporaryStoragePath));

var tempFile = fileSystem.Path.Combine(temporaryStoragePath, $@"{System.DateTime.UtcNow.Ticks}.tmp");
var stream = fileSystem.File.Create(tempFile);
var data = Encoding.UTF8.GetBytes(message);
await stream.WriteAsync(data, 0, data.Length);
storageObjectMetadata.Data = stream;
break;
default:
storageObjectMetadata.Data = new System.IO.MemoryStream(Encoding.UTF8.GetBytes(message));
break;
}

storageObjectMetadata.Data.Seek(0, System.IO.SeekOrigin.Begin);
}
}
}
Loading

0 comments on commit 09887b1

Please sign in to comment.