Skip to content

Commit

Permalink
Improve code and reduce warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
lilith committed Jan 29, 2024
1 parent 08130d7 commit 52468d6
Show file tree
Hide file tree
Showing 18 changed files with 102 additions and 60 deletions.
4 changes: 2 additions & 2 deletions examples/Imageflow.Server.Example/CustomBlobService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ public async Task<CodeResult<IBlobWrapper>> Fetch(string virtualPath)
try
{
var blobClient = client.GetBlobContainerClient(container).GetBlobClient(blobKey);

var latencyZone = new LatencyTrackingZone($"azure::blob/{container}", 100);
var s = await blobClient.DownloadAsync();
return CodeResult<IBlobWrapper>.Ok(new BlobWrapper(new CustomAzureBlob(s)));
return CodeResult<IBlobWrapper>.Ok(new BlobWrapper(latencyZone,new CustomAzureBlob(s)));

}
catch (RequestFailedException e)
Expand Down
1 change: 0 additions & 1 deletion examples/Imageflow.Server.Example/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ public void ConfigureServices(IServiceCollection services)
// How long after a file is created before it can be deleted
MinAgeToDelete = TimeSpan.FromSeconds(10),
// How much RAM to use for the write queue before switching to synchronous writes
WriteQueueMemoryMb = 100,
// The maximum size of the cache (1GB)
CacheSizeMb = 1000,
});
Expand Down
7 changes: 4 additions & 3 deletions src/Imageflow.Server.Configuration/Executor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ public ImageflowMiddlewareOptions GetImageflowMiddlewareOptions(){
throw new ArgumentNullException(
$"route_defaults.apply_default_commands.value is missing. Defined in file '{sourcePath}' for key route_defaults.apply_default_commands.key '{key}'");
}

options.AddCommandDefault(key, value);
options.AddCommandDefault(key, value!);
}
}
if (config.RouteDefaults?.ApplyDefaultCommandsToQuerylessUrls ?? false){
Expand Down Expand Up @@ -243,8 +242,10 @@ public HybridCacheOptions GetHybridCacheOptions(){
}
var writeQueueRamMb = config.DiskCache.WriteQueueRamMb ?? 0;
if (writeQueueRamMb > 0){
options.WriteQueueMemoryMb = writeQueueRamMb;
//TODO: warn ignored

}

var EvictionSweepSizeMb = config.DiskCache.EvictionSweepSizeMb ?? 0;
if (EvictionSweepSizeMb > 0){
options.EvictionSweepSizeMb = EvictionSweepSizeMb;
Expand Down
2 changes: 1 addition & 1 deletion src/Imageflow.Server.Storage.AzureBlob/AzureBlobService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public AzureBlobService(AzureBlobServiceOptions options, IReLoggerFactory logger

options.NamedCaches.ForEach(c =>
{
var cache = new AzureBlobCache(c, clientName => clientName == null ? client : clientFactory.CreateClient(clientName), null); //TODO! get logging working
var cache = new AzureBlobCache(c, clientName => clientName == null ? client : clientFactory.CreateClient(clientName), loggerFactory); //TODO! get logging working
caches.Add(cache);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ internal record struct ExtensionlessPath(string StringToCompare, StringCompariso

public bool UsePresetsExclusively { get; set; }

public string DefaultCacheControlString { get; set; } = "";
public string? DefaultCacheControlString { get; set; } = "";

public RequestSignatureOptions RequestSignatureOptions { get; set; } = RequestSignatureOptions.Empty;
public SecurityOptions JobSecurityOptions { get; set; } = new();
Expand Down Expand Up @@ -241,7 +241,7 @@ public ImageflowMiddlewareOptions SetAllowDiskCaching(bool value)
/// <param name="cacheControlString"></param>
/// <returns></returns>
/// <exception cref="NotImplementedException"></exception>
public ImageflowMiddlewareOptions SetDefaultCacheControlString(string cacheControlString)
public ImageflowMiddlewareOptions SetDefaultCacheControlString(string? cacheControlString)
{
DefaultCacheControlString = cacheControlString;
return this;
Expand Down
3 changes: 2 additions & 1 deletion src/Imazen.Abstractions/Blobs/BlobWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ public BlobWrapper(LatencyTrackingZone? latencyZone, IReusableBlob reusable)
CreatedAtUtc = DateTime.UtcNow;
LatencyZone = latencyZone;
}
[Obsolete("Use the constructor that takes a first parameter of LatencyTrackingZone")]
[Obsolete("Use the constructor that takes a first parameter of LatencyTrackingZone, so that you " +
"can allow Imageflow Server to apply intelligent caching logic to this blob.")]
public BlobWrapper(IConsumableBlob consumable)
{
this.consumable = consumable;
Expand Down
6 changes: 3 additions & 3 deletions src/Imazen.Common/Instrumentation/WindowsCpuInfo.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Runtime.InteropServices;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;

namespace Imazen.Common.Instrumentation
{
Expand All @@ -22,8 +23,7 @@ public static byte[] Invoke(int level)

Marshal.Copy(codeBytes, 0, codePointer, codeBytes.Length);

var cpuIdDelg =
(CpuIdDelegate) Marshal.GetDelegateForFunctionPointer(codePointer, typeof(CpuIdDelegate));
var cpuIdDelg =Marshal.GetDelegateForFunctionPointer<CpuIdDelegate>(codePointer);

// invoke
var handle = default(GCHandle);
Expand Down
8 changes: 7 additions & 1 deletion src/Imazen.Common/Licensing/LicensingSupport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,14 @@ class RealClock : ILicenseClock
public DateTimeOffset? GetAssemblyWriteDate()
{
try {
// .Location can throw
// .Location can throw, or be empty string (on AOT)
#pragma warning disable IL3000
var path = GetType().Assembly.Location;
#pragma warning restore IL3000
if (string.IsNullOrEmpty(path))
{
return null;
}
return File.Exists(path)
? new DateTimeOffset?(File.GetLastWriteTimeUtc(path))
: null;
Expand Down
5 changes: 3 additions & 2 deletions src/Imazen.HybridCache/AsyncCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -524,13 +524,14 @@ public async Task<CodeResult> CachePut(ICacheEventDetails e, CancellationToken c
if (cancellationToken.IsCancellationRequested)
throw new OperationCanceledException(cancellationToken);
if (e.Result == null) throw new InvalidOperationException("Result is null");
var blob = await e.Result.Unwrap().CreateConsumable(e.BlobFactory);
var blob = await e.Result.Unwrap().CreateConsumable(e.BlobFactory, cancellationToken);
if (blob == null) throw new InvalidOperationException("Blob is null");
var record = new CacheDatabaseRecord
{
AccessCountKey = CleanupManager.GetAccessCountKey(entry),
CreatedAt = DateTimeOffset.UtcNow.AddDays(1),
LastDeletionAttempt = DateTime.MinValue,
EstDiskSize = CleanupManager.EstimateFileSizeOnDisk(blob?.StreamLength ?? 0),
EstDiskSize = CleanupManager.EstimateFileSizeOnDisk(blob.StreamLength ?? 0),
RelativePath = entry.RelativePath,
ContentType = blob.Attributes.ContentType,
Flags = (e.BlobCategory == BlobGroup.Essential) ? CacheEntryFlags.DoNotEvict : CacheEntryFlags.Unknown,
Expand Down
3 changes: 2 additions & 1 deletion src/Imazen.HybridCache/AsyncCacheOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ public class AsyncCacheOptions
public bool WriteSynchronouslyWhenQueueFull { get; set; } = false;

/// <summary>
/// If this is used from .NET Core, set to File.Move(from, to, true)
/// If you're not on .NET 6/8, but you have File.Move(from, to, true) available,
/// you can set this to true to use that instead of the default File.Move(from,to) for better performance
/// Used by deletion code even if MoveFilesIntoPlace is false
/// </summary>
public Action<string,string>? MoveFileOverwriteFunc { get; set; }
Expand Down
24 changes: 19 additions & 5 deletions src/Imazen.HybridCache/CacheFileWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ internal enum FileWriteStatus
LockTimeout,
}

private readonly Action<string, string> moveFileOverwriteFunc;
private readonly Action<string, string>? moveFileOverwriteFunc;
private AsyncLockProvider WriteLocks { get; }

private bool moveIntoPlace;
public CacheFileWriter(AsyncLockProvider writeLocks, Action<string,string> moveFileOverwriteFunc, bool moveIntoPlace)
private readonly bool moveIntoPlace;
public CacheFileWriter(AsyncLockProvider writeLocks, Action<string,string>? moveFileOverwriteFunc, bool moveIntoPlace)
{
this.moveIntoPlace = moveIntoPlace;
WriteLocks = writeLocks;
this.moveFileOverwriteFunc = moveFileOverwriteFunc ?? File.Move;
this.moveFileOverwriteFunc = moveFileOverwriteFunc;
}


Expand Down Expand Up @@ -99,7 +99,7 @@ internal async Task<FileWriteStatus> TryWriteFile(CacheEntry entry,
{
if (moveIntoPlace)
{
moveFileOverwriteFunc(writeToFile, entry.PhysicalPath);
MoveOverwrite(writeToFile, entry.PhysicalPath);
}
resultStatus = FileWriteStatus.FileCreated;
Expand Down Expand Up @@ -135,5 +135,19 @@ internal async Task<FileWriteStatus> TryWriteFile(CacheEntry entry,
}
return resultStatus;
}

private void MoveOverwrite(string from, string to)
{
if (moveFileOverwriteFunc != null)
moveFileOverwriteFunc(from, to);
else
{
#if NET5_0_OR_GREATER
File.Move(from, to, true);
#else
File.Move(from, to);
#endif
}
}
}
}
23 changes: 22 additions & 1 deletion src/Imazen.HybridCache/CleanupManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ public async Task<CodeResult> CacheDelete(string relativePath, AsyncLockProvider
var record = await Database.GetRecord(shard, relativePath);
if (cancellationToken.IsCancellationRequested)
throw new OperationCanceledException(cancellationToken);

if (record == null)
return CodeResult.Ok((200, $"File doesn't exist (in metabase) to delete: {relativePath}"));

var result = await TryDelete(shard, record, writeLocks);
// Retry once if the record reference is stale.
if (result.RecordDeleteResult == DeleteRecordResult.RecordStaleReQueryRetry)
Expand All @@ -112,6 +116,9 @@ public async Task<CodeResult> CacheDelete(string relativePath, AsyncLockProvider
record = await Database.GetRecord(shard, relativePath);
if (cancellationToken.IsCancellationRequested)
throw new OperationCanceledException(cancellationToken);
if (record == null)
return CodeResult.Ok((200, $"File doesn't exist (in metabase) to delete: {relativePath}"));

result = await TryDelete(shard, record, writeLocks);
}

Expand Down Expand Up @@ -361,7 +368,7 @@ private async Task<TryDeleteResult> TryDelete(int shard, ICacheDatabaseRecord re
//Move it so it usage will decrease and it can be deleted later
//TODO: This is not transactional, as the db record is written *after* the file is moved
//This should be split up into create and delete
(Options.MoveFileOverwriteFunc ?? File.Move)(physicalPath, movedPath);
MoveOverwrite(physicalPath, movedPath);
await Database.ReplaceRelativePathAndUpdateLastDeletion(shard, record, movedRelativePath,
DateTime.UtcNow);
result.FileMovedForFutureDeletion = true;
Expand All @@ -380,5 +387,19 @@ await Database.ReplaceRelativePathAndUpdateLastDeletion(shard, record, movedRela
return result;
}

private void MoveOverwrite(string from, string to)
{

if (Options.MoveFileOverwriteFunc != null)
Options.MoveFileOverwriteFunc(from, to);
else
{
#if NET5_0_OR_GREATER
File.Move(from, to, true);
#else
File.Move(from, to);
#endif
}
}
}
}
19 changes: 4 additions & 15 deletions src/Imazen.HybridCache/CleanupManagerOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,11 @@ public class CleanupManagerOptions
public int CleanupSelectBatchSize { get; set; } = 1000;

/// <summary>
/// If this is used from .NET Core, set to File.Move(from, to, true). If you're referencing binaries compiled for .NET Core, it can be left null;
/// If you're not on .NET 6/8, but you have File.Move(from, to, true) available,
/// you can set this to true to use that instead of the default File.Move(from,to) for better performance
/// Used by deletion code even if MoveFilesIntoPlace is false
/// </summary>
public Action<string,string>? MoveFileOverwriteFunc { get; set; }

internal static void MoveFileOverwrite(CleanupManagerOptions options, string from, string to)
{
if (options.MoveFileOverwriteFunc != null)
{
options.MoveFileOverwriteFunc(from, to);
return;
}
#if (NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER)
File.Move(from, to, true);
#else
File.Move(from, to);
#endif
}

}
}
2 changes: 1 addition & 1 deletion src/Imazen.HybridCache/MetaStore/Shard.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private async Task<ConcurrentDictionary<string, CacheDatabaseRecord>> GetLoadedD
{
dict = await writeLog.Startup();
}
catch (Exception ex)
catch (Exception)
{
FailedToStart = true;
throw;
Expand Down
27 changes: 13 additions & 14 deletions src/Imazen.HybridCache/MetaStore/WriteLog.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ internal class WriteLog
private readonly ILogger logger;
private long startedAt;
private bool startupComplete;
private FileStream writeLogStream;
private BinaryWriter binaryLogWriter;
private FileStream? writeLogStream;
private BinaryWriter? binaryLogWriter;
private readonly object writeLock = new object();
private readonly int shardId;
private long previousLogsBytes;
Expand Down Expand Up @@ -69,16 +69,10 @@ private void CreateWriteLog(long startedAtTick)
}

// create a record containing a version int and a FileStream instance
private class FileStreamWithVersion
private class FileStreamWithVersion(int version, FileStream stream)
{
public FileStreamWithVersion(int version, FileStream stream)
{
Version = version;
Stream = stream;
}

public int Version { get; }
public FileStream Stream { get; }
public int Version { get; } = version;
public FileStream Stream { get; } = stream;
}

// Returns a dictionary of the database
Expand All @@ -101,7 +95,7 @@ public async Task<ConcurrentDictionary<string, CacheDatabaseRecord>> Startup()
? new Tuple<string, long>(path, result)
: null;
})
.Where(t => t != null)
.Where(t => t != null).Cast<Tuple<string, long>>()
.OrderBy(t => t.Item2)
.ToArray();

Expand Down Expand Up @@ -209,7 +203,11 @@ public async Task<ConcurrentDictionary<string, CacheDatabaseRecord>> Startup()
{
await WriteLogEntry(new LogEntry(LogEntryType.Create, record), false);
}

if (writeLogStream == null)
{
throw new InvalidOperationException("writeLogStream is null");
}

await writeLogStream.FlushAsync();

// Delete old logs
Expand Down Expand Up @@ -309,8 +307,9 @@ private Task WriteLogEntry(LogEntry entry, bool flush)
{
if (startedAt == 0)
throw new InvalidOperationException("WriteLog cannot be used before calling Startup()");
if (writeLogStream == null)
if (writeLogStream == null || binaryLogWriter == null)
throw new InvalidOperationException("WriteLog cannot be after StopAsync is called");

var startPos = writeLogStream.Position;
binaryLogWriter.Write((byte)entry.EntryType);
binaryLogWriter.Write(entry.RelativePath);
Expand Down
5 changes: 5 additions & 0 deletions src/Imazen.Routing/Health/BehaviorTask.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,9 @@ public enum BehaviorTask: byte
SearchByTag,
PurgeByTag,
HealthCheck
}

internal static class BehaviorTaskHelpers
{
internal const int BehaviorTaskCount = 8;
}
4 changes: 2 additions & 2 deletions src/Imazen.Routing/Health/CacheHealthMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ internal class CacheHealthMetrics
/// </summary>
private bool SupportsHealthCheck { get; init; }
// collectedMetrics[BehaviorTask.HealthCheck] refers to whether the health check completed or threw an exception, not if it returned a degraded result (it was still a success)
private readonly BehaviorMetrics[] collectedMetrics = new BehaviorMetrics[Enum.GetValues(typeof(BehaviorTask)).Length];
private readonly BehaviorMetrics[] collectedMetrics = new BehaviorMetrics[BehaviorTaskHelpers.BehaviorTaskCount];
// This just tracks reported capabilities over time.
private readonly BehaviorMetrics[] healthCheckMetrics = new BehaviorMetrics[Enum.GetValues(typeof(BehaviorTask)).Length];
private readonly BehaviorMetrics[] healthCheckMetrics = new BehaviorMetrics[BehaviorTaskHelpers.BehaviorTaskCount];
/// <summary>
/// This tracks whether the health checks have been returning at full health or not (ignoring crashes, those are counted in collectedMetrics)
/// </summary>
Expand Down
Loading

0 comments on commit 52468d6

Please sign in to comment.