diff --git a/MergerLogic/Clients/S3Client.cs b/MergerLogic/Clients/S3Client.cs index f06c0e23..e47e5446 100644 --- a/MergerLogic/Clients/S3Client.cs +++ b/MergerLogic/Clients/S3Client.cs @@ -5,6 +5,7 @@ using MergerLogic.ImageProcessing; using MergerLogic.Utils; using Microsoft.Extensions.Logging; +using System.Net; using System.Reflection; namespace MergerLogic.Clients @@ -28,6 +29,10 @@ public S3Client(IAmazonS3 client, IPathUtils pathUtils, IGeoUtils geoUtils, ILog this._storageClass = new S3StorageClass(storageClass ?? S3StorageClass.Standard); } + // Read paths don't know a tile's extension ahead of time, so existence is probed against + // these candidates in order (matches the historical Jpeg-then-Png GetTile lookup). + private static readonly TileFormat[] _readFormats = { TileFormat.Jpeg, TileFormat.Png }; + private bool IsKeyError(Exception e) { if (e is AmazonS3Exception ex) @@ -39,10 +44,22 @@ private bool IsKeyError(Exception e) { return en.ErrorCode == "NoSuchKey"; } - + return false; } + // HEAD returns 404 (not the GET "NoSuchKey") for a missing object. + private bool IsKeyNotFound(Exception e) + { + AmazonS3Exception? ex = e as AmazonS3Exception ?? e.InnerException as AmazonS3Exception; + if (ex is null) + { + return false; + } + + return ex.StatusCode == HttpStatusCode.NotFound || ex.ErrorCode == "NoSuchKey"; + } + private byte[]? GetImageBytes(string key) { string? methodName = MethodBase.GetCurrentMethod()?.Name; @@ -149,24 +166,40 @@ public void UpdateTile(Tile tile) this._logger.LogDebug($"[{methodName}] end {tile.ToString()}"); } + // Resolves existence and the real extension with a HEAD per candidate format. HEAD is an + // O(1) key lookup; a prefix LIST would scan the bucket index, which does not scale on the + // billions-of-objects bucket (MAPCO-7954). private string? GetTileKey(int z, int x, int y) { - string methodName = MethodBase.GetCurrentMethod().Name; - string keyPrefix = this._pathUtils.GetTilePathWithoutExtension(this.path, z, x, y, true); + foreach (TileFormat format in _readFormats) + { + string key = this._pathUtils.GetTilePath(this.path, z, x, y, format, true); + if (this.KeyExists(key)) + { + return key; + } + } + + return null; + } + private bool KeyExists(string key) + { + string methodName = MethodBase.GetCurrentMethod().Name; try { - var getRequest = new GetObjectRequest { BucketName = this._bucket, Key = keyPrefix }; - var getObjectTask = this._client.GetObjectAsync(getRequest); - string result = getObjectTask.Result.Key; - return result; + var request = new GetObjectMetadataRequest { BucketName = this._bucket, Key = key }; + this._logger.LogDebug($"[{methodName}] GetObjectMetadataAsync BucketName: {this._bucket}, Key: {key}"); + var task = this._client.GetObjectMetadataAsync(request); + _ = task.Result; + return true; } catch (AggregateException e) { - if (IsKeyError(e)) + if (IsKeyNotFound(e)) { - this._logger.LogDebug($"[{methodName}] error getting key: {e.Message}"); - return null; + this._logger.LogDebug($"[{methodName}] key not found: {key}"); + return false; } // In case there are other errors such as connection to S3 throw e; diff --git a/MergerLogic/DataTypes/S3.cs b/MergerLogic/DataTypes/S3.cs index 84cdd79e..133cbc68 100644 --- a/MergerLogic/DataTypes/S3.cs +++ b/MergerLogic/DataTypes/S3.cs @@ -198,10 +198,9 @@ public override long TileCount() protected override void InternalUpdateTiles(IEnumerable targetTiles) { this._logger.LogDebug($"[{MethodBase.GetCurrentMethod()?.Name}] start"); - foreach (var tile in targetTiles) - { - this.Utils.UpdateTile(tile); - } + // ToList so Parallel.ForEach range-partitions instead of running the upstream grid/origin + // projection under the shared-enumerator lock a bare IEnumerable would impose. + Parallel.ForEach(targetTiles.ToList(), tile => this.Utils.UpdateTile(tile)); this._logger.LogDebug($"[{MethodBase.GetCurrentMethod()?.Name}] end"); } } diff --git a/MergerLogicUnitTests/DataTypes/S3Test.cs b/MergerLogicUnitTests/DataTypes/S3Test.cs index 50b286bf..20928669 100644 --- a/MergerLogicUnitTests/DataTypes/S3Test.cs +++ b/MergerLogicUnitTests/DataTypes/S3Test.cs @@ -473,8 +473,9 @@ public void UpdateTiles(bool isOneXOne, GridOrigin origin) if (!isOneXOne || tile.Z != 7) { + // Uploads run in parallel after the (ordered) grid/origin projection, so UpdateTile is + // intentionally outside the strict sequence; per-tile Times.Once is verified below. this._s3UtilsMock - .InSequence(seq) .Setup(utils => utils.UpdateTile(It.IsAny())); } } diff --git a/MergerLogicUnitTests/Utils/S3UtilsTest.cs b/MergerLogicUnitTests/Utils/S3UtilsTest.cs index b3e65ae1..9bae66df 100644 --- a/MergerLogicUnitTests/Utils/S3UtilsTest.cs +++ b/MergerLogicUnitTests/Utils/S3UtilsTest.cs @@ -1,4 +1,5 @@ -using Amazon.S3; +using Amazon.Runtime; +using Amazon.S3; using Amazon.S3.Model; using MergerLogic.Batching; using MergerLogic.Clients; @@ -12,6 +13,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Net; using System.Threading; using System.Threading.Tasks; @@ -324,29 +326,30 @@ public void GetTile(bool exist, GetTileParamType paramType, TileFormat tileForma [DataRow(false)] public void TileExists(bool exist) { - var seq = new MockSequence(); - this._pathUtilsMock - .InSequence(seq) - .Setup(utils => utils.GetTilePathWithoutExtension("test", 0, 0, 0, true)) - .Returns("key"); + var notFound = new AmazonS3Exception("Not Found", ErrorType.Sender, "NotFound", "req", HttpStatusCode.NotFound); - if (exist) - { - this._amazonS3ClientMock - .InSequence(seq) - .Setup(s3 => s3.GetObjectAsync(It.Is(req => - req.BucketName == "bucket" && req.Key == "key"), + // Jpeg is probed first via HEAD; only when it 404s is the Png candidate probed. + this._pathUtilsMock + .Setup(utils => utils.GetTilePath("test", 0, 0, 0, TileFormat.Jpeg, true)) + .Returns("keyJpeg"); + this._amazonS3ClientMock + .Setup(s3 => s3.GetObjectMetadataAsync( + It.Is(req => req.BucketName == "bucket" && req.Key == "keyJpeg"), It.IsAny())) - .ReturnsAsync(new GetObjectResponse() { Key = "key" }); - } - else + .Returns(exist + ? Task.FromResult(new GetObjectMetadataResponse()) + : Task.FromException(notFound)); + + if (!exist) { + this._pathUtilsMock + .Setup(utils => utils.GetTilePath("test", 0, 0, 0, TileFormat.Png, true)) + .Returns("keyPng"); this._amazonS3ClientMock - .InSequence(seq) - .Setup(s3 => s3.GetObjectAsync(It.Is(req => - req.BucketName == "bucket" && req.Key == "key"), - It.IsAny())) - .ThrowsAsync(new AmazonS3Exception("", Amazon.Runtime.ErrorType.Unknown, "NoSuchKey", "", System.Net.HttpStatusCode.NoContent)); + .Setup(s3 => s3.GetObjectMetadataAsync( + It.Is(req => req.BucketName == "bucket" && req.Key == "keyPng"), + It.IsAny())) + .Returns(Task.FromException(notFound)); } var s3Utils = new S3Client(this._amazonS3ClientMock.Object, this._pathUtilsMock.Object, @@ -354,9 +357,11 @@ public void TileExists(bool exist) Assert.AreEqual(exist, s3Utils.TileExists(0, 0, 0)); - this._pathUtilsMock.Verify(utils => utils.GetTilePathWithoutExtension("test", 0, 0, 0, true), Times.Once); - this._amazonS3ClientMock.Verify(s3 => s3.GetObjectAsync(It.Is(req => - req.BucketName == "bucket" && req.Key == "key"), It.IsAny()), Times.Once); + this._amazonS3ClientMock.Verify(s3 => s3.GetObjectMetadataAsync( + It.Is(req => req.Key == "keyJpeg"), It.IsAny()), Times.Once); + this._amazonS3ClientMock.Verify(s3 => s3.GetObjectMetadataAsync( + It.Is(req => req.Key == "keyPng"), It.IsAny()), + exist ? Times.Never() : Times.Once()); this.VerifyAll(); }