From bbe9a3b2d0d44e2022a96429aca053c736bed062 Mon Sep 17 00:00:00 2001 From: Gui <51219838+gui-ace@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:25:51 +0900 Subject: [PATCH 1/3] =?UTF-8?q?S3=E9=80=9A=E4=BF=A1=E9=9A=9C=E5=AE=B3?= =?UTF-8?q?=E6=99=82=E3=81=AE=E6=9B=B4=E6=96=B0=E4=BF=9D=E6=8C=81=E3=81=A8?= =?UTF-8?q?=E5=86=8D=E8=A9=A6=E8=A1=8C=E3=81=8A=E3=82=88=E3=81=B3=E5=B7=AE?= =?UTF-8?q?=E5=88=86=E5=85=AC=E9=96=8B=E3=82=92=E6=94=B9=E5=96=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/main/java/org/dynmap/DynmapCore.java | 13 +-- .../src/main/java/org/dynmap/DynmapWorld.java | 21 +++-- .../dynmap/JsonFileClientUpdateComponent.java | 77 +++-------------- .../org/dynmap/hdmap/IsoHDPerspective.java | 32 ++++--- .../java/org/dynmap/storage/MapStorage.java | 2 +- .../dynmap/storage/StorageReadException.java | 6 ++ .../storage/aws_s3/AWSS3MapStorage.java | 83 ++++++++++++------- .../storage/aws_s3/RetryingS3Client.java | 42 ++++++++++ .../org/dynmap/utils/RetryingFileQueue.java | 68 +++++++++++++++ .../org/dynmap/ZoomStorageFailureTest.java | 54 ++++++++++++ .../storage/aws_s3/AWSS3ReliabilityTest.java | 78 +++++++++++++++++ .../storage/aws_s3/RetryingS3ClientTest.java | 54 ++++++++++++ .../dynmap/utils/RetryingFileQueueTest.java | 68 +++++++++++++++ docs/r2-reliability.md | 19 +++++ 14 files changed, 493 insertions(+), 124 deletions(-) create mode 100644 DynmapCore/src/main/java/org/dynmap/storage/StorageReadException.java create mode 100644 DynmapCore/src/main/java/org/dynmap/storage/aws_s3/RetryingS3Client.java create mode 100644 DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java create mode 100644 DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java create mode 100644 DynmapCore/src/test/java/org/dynmap/storage/aws_s3/AWSS3ReliabilityTest.java create mode 100644 DynmapCore/src/test/java/org/dynmap/storage/aws_s3/RetryingS3ClientTest.java create mode 100644 DynmapCore/src/test/java/org/dynmap/utils/RetryingFileQueueTest.java create mode 100644 docs/r2-reliability.md diff --git a/DynmapCore/src/main/java/org/dynmap/DynmapCore.java b/DynmapCore/src/main/java/org/dynmap/DynmapCore.java index 46dcd1bec..627c0c3c2 100644 --- a/DynmapCore/src/main/java/org/dynmap/DynmapCore.java +++ b/DynmapCore/src/main/java/org/dynmap/DynmapCore.java @@ -507,10 +507,6 @@ public boolean enableCore(EnableCoreCallbacks cb) { authmgr = new WebAuthManager(this); defaultStorage.setLoginEnabled(this); } - // If storage serves web files, extract and publsh them - if (defaultStorage.needsStaticWebFiles()) { - updateStaticWebToStorage(); - } /* Load control for leaf transparency (spout lighting bug workaround) */ transparentLeaves = configuration.getBoolean("transparent-leaves", true); @@ -632,6 +628,10 @@ public boolean enableCore(EnableCoreCallbacks cb) { mapManager = new MapManager(this, configuration); mapManager.startRendering(); + // Network publication must not block the server startup thread. + if (defaultStorage.needsStaticWebFiles()) { + updateStaticWebToStorage(); + } if (markerapi != null) { MarkerAPIImpl.completeInitializeMarkerAPI(markerapi); @@ -2856,6 +2856,9 @@ private void updateStaticWebToStorage() { return; } Log.info("Publishing web files to storage"); + org.dynmap.utils.RetryingFileQueue publication = new org.dynmap.utils.RetryingFileQueue( + defaultStorage::setStaticWebFile, MapManager::scheduleDelayedJob, + name -> Log.severe("Web asset publication failed; retry queued: " + name)); /* Open JAR as ZIP */ ZipFile zf = null; InputStream ins = null; @@ -2882,7 +2885,7 @@ private void updateStaticWebToStorage() { while ((len = ins.read(buf)) >= 0) { buffer.write(buf, 0, len); } - defaultStorage.setStaticWebFile(n, buffer); + publication.enqueue(n, buffer); } catch(IOException io) { Log.severe("Error updating file in storage - " + n, io); } finally { diff --git a/DynmapCore/src/main/java/org/dynmap/DynmapWorld.java b/DynmapCore/src/main/java/org/dynmap/DynmapWorld.java index 22c76940e..bf7f87b7e 100644 --- a/DynmapCore/src/main/java/org/dynmap/DynmapWorld.java +++ b/DynmapCore/src/main/java/org/dynmap/DynmapWorld.java @@ -10,6 +10,7 @@ import org.dynmap.hdmap.TexturePack; import org.dynmap.storage.MapStorage; import org.dynmap.storage.MapStorageTile; +import org.dynmap.storage.StorageReadException; import org.dynmap.utils.DynmapBufferedImage; import org.dynmap.utils.ImageIOManager; import org.dynmap.utils.MapChunkCache; @@ -116,7 +117,10 @@ public void freshenZoomOutFiles() { if(cancelled) return; for (int varIdx = 0; varIdx < var.length; varIdx++) { MapStorageTile tile = storage.getTile(this, mt, c.x, c.y, c.zoomlevel, var[varIdx]); - processZoomFile(mts, tile, varIdx == 0); + if (!processZoomFile(mts, tile, varIdx == 0)) { + // Accumulator is separate from this pass: no tight retry loop. + mts.setZoomOutInv(tile.x, tile.y, tile.zoom); + } } } } @@ -132,7 +136,7 @@ public void activateZoomOutFreshen() { private static final int[] stepseq = { 3, 1, 2, 0 }; - private void processZoomFile(MapTypeState mts, MapStorageTile tile, boolean firstVariant) { + private boolean processZoomFile(MapTypeState mts, MapStorageTile tile, boolean firstVariant) { long mostRecentTimestamp = 0; int step = 1 << tile.zoom; MapStorageTile ztile = tile.getZoomOutTile(); @@ -148,6 +152,7 @@ private void processZoomFile(MapTypeState mts, MapStorageTile tile, boolean firs /* create image buffer */ kzIm = DynmapBufferedImage.allocateBufferedImage(width, height); zIm = kzIm.buf_img; + try { for(int i = 0; i < 4; i++) { boolean doblit = true; int tx1 = tx + step * (1 & stepseq[i]); @@ -236,22 +241,28 @@ private void processZoomFile(MapTypeState mts, MapStorageTile tile, boolean firs try { MapManager mm = MapManager.mapman; if(mm == null) - return; + return false; long crc = MapStorage.calculateImageHashCode(kzIm.argb_buf, 0, kzIm.argb_buf.length); /* Get hash of tile */ if(blank) { if (ztile.exists()) { - ztile.delete(); + if (!ztile.delete()) return false; MapManager.mapman.pushUpdate(this, new Client.Tile(ztile.getURI())); enqueueZoomOutUpdate(ztile); } } else /* if (!ztile.matchesHashCode(crc)) */ { - ztile.write(crc, zIm, (mostRecentTimestamp == 0)? System.currentTimeMillis() : mostRecentTimestamp); + if (!ztile.write(crc, zIm, (mostRecentTimestamp == 0)? System.currentTimeMillis() : mostRecentTimestamp)) return false; MapManager.mapman.pushUpdate(this, new Client.Tile(ztile.getURI())); enqueueZoomOutUpdate(ztile); } } finally { ztile.releaseWriteLock(); + } + return true; + } catch (StorageReadException ex) { + Log.warning("Storage read failed; retaining zoom update for " + tile.getURI()); + return false; + } finally { DynmapBufferedImage.freeBufferedImage(kzIm); } } diff --git a/DynmapCore/src/main/java/org/dynmap/JsonFileClientUpdateComponent.java b/DynmapCore/src/main/java/org/dynmap/JsonFileClientUpdateComponent.java index ceeac3a56..1d1f6a6fd 100644 --- a/DynmapCore/src/main/java/org/dynmap/JsonFileClientUpdateComponent.java +++ b/DynmapCore/src/main/java/org/dynmap/JsonFileClientUpdateComponent.java @@ -12,12 +12,12 @@ import java.util.Arrays; import java.util.HashMap; import java.util.Iterator; -import java.util.LinkedList; import java.util.List; import org.dynmap.storage.MapStorage; import org.dynmap.utils.BufferInputStream; import org.dynmap.utils.BufferOutputStream; +import org.dynmap.utils.RetryingFileQueue; import org.dynmap.web.Json; import org.json.simple.JSONArray; import org.json.simple.JSONObject; @@ -50,69 +50,20 @@ public class JsonFileClientUpdateComponent extends ClientUpdateComponent { private MapStorage storage; private File baseStandaloneDir; - private static class FileToWrite { - String filename; - byte[] content; - boolean phpwrapper; - @Override - public boolean equals(Object o) { - if(o instanceof FileToWrite) { - return ((FileToWrite)o).filename.equals(this.filename); - } - return false; - } - } - private class FileProcessor implements Runnable { - public void run() { - while(true) { - FileToWrite f = null; - synchronized(lock) { - if(files_to_write.isEmpty() == false) { - f = files_to_write.removeFirst(); - } - else { - pending = null; - return; - } - } - BufferOutputStream buf = null; - if (f.content != null) { - buf = new BufferOutputStream(); - if(f.phpwrapper) { - buf.write("\n".getBytes(cs_utf8)); - } - } - if (!storage.setStandaloneFile(f.filename, buf)) { - Log.severe("Exception while writing JSON-file - " + f.filename); - } - } - } - } - private Object lock = new Object(); - private FileProcessor pending; - private LinkedList files_to_write = new LinkedList(); + private final RetryingFileQueue files = new RetryingFileQueue( + (name, content) -> storage.setStandaloneFile(name, content), + MapManager::scheduleDelayedJob, + name -> Log.severe("Exception while writing JSON-file - " + name + "; retry queued")); private void enqueueFileWrite(String filename, byte[] content, boolean phpwrap) { - FileToWrite ftw = new FileToWrite(); - ftw.filename = filename; - ftw.content = content; - ftw.phpwrapper = phpwrap; - synchronized(lock) { - boolean didadd = false; - if(pending == null) { - didadd = true; - pending = new FileProcessor(); - } - files_to_write.remove(ftw); - files_to_write.add(ftw); - if(didadd) { - MapManager.scheduleDelayedJob(new FileProcessor(), 0); - } + BufferOutputStream buf = null; + if (content != null) { + buf = new BufferOutputStream(); + if (phpwrap) buf.write("\n".getBytes(cs_utf8)); } + files.enqueue(filename, buf); } private static Charset cs_utf8 = Charset.forName("UTF-8"); @@ -274,9 +225,7 @@ private void generateConfigJS(DynmapCore core) { MapManager.scheduleDelayedJob(new Runnable() { public void run() { if (core.getDefaultMapStorage().needsStaticWebFiles()) { - BufferOutputStream os = new BufferOutputStream(); - os.write(outputBytes); - core.getDefaultMapStorage().setStaticWebFile("standalone/config.js", os); + enqueueFileWrite("config.js", outputBytes, false); } else { File f = new File(baseStandaloneDir, "config.js"); diff --git a/DynmapCore/src/main/java/org/dynmap/hdmap/IsoHDPerspective.java b/DynmapCore/src/main/java/org/dynmap/hdmap/IsoHDPerspective.java index 4ed045661..cb38ad922 100644 --- a/DynmapCore/src/main/java/org/dynmap/hdmap/IsoHDPerspective.java +++ b/DynmapCore/src/main/java/org/dynmap/hdmap/IsoHDPerspective.java @@ -1402,19 +1402,17 @@ public boolean render(MapChunkCache cache, HDMapTile tile, String mapname) { try { if(mtile.matchesHashCode(crc) == false) { /* Wrap buffer as buffered image */ - if(rendered[i]) { - mtile.write(crc, im[i].buf_img, startTimestamp); - } - else { - mtile.delete(); + tile_update = rendered[i] ? mtile.write(crc, im[i].buf_img, startTimestamp) : mtile.delete(); + if (tile_update) { + MapManager.mapman.pushUpdate(tile.getDynmapWorld(), new Client.Tile(mtile.getURI())); + renderone = true; + } else { + world.getMapState(shaderstate[i].getMap()).invalidateTile(tile.tx, tile.ty); } - MapManager.mapman.pushUpdate(tile.getDynmapWorld(), new Client.Tile(mtile.getURI())); - tile_update = true; - renderone = true; } else { if(!rendered[i]) { - mtile.delete(); + if (!mtile.delete()) world.getMapState(shaderstate[i].getMap()).invalidateTile(tile.tx, tile.ty); } } } finally { @@ -1433,19 +1431,17 @@ public boolean render(MapChunkCache cache, HDMapTile tile, String mapname) { try { if(mtile.matchesHashCode(crc) == false) { /* Wrap buffer as buffered image */ - if(rendered[i]) { - mtile.write(crc, dayim[i].buf_img, startTimestamp); + tile_update = rendered[i] ? mtile.write(crc, dayim[i].buf_img, startTimestamp) : mtile.delete(); + if (tile_update) { + MapManager.mapman.pushUpdate(tile.getDynmapWorld(), new Client.Tile(mtile.getURI())); + renderone = true; + } else { + world.getMapState(shaderstate[i].getMap()).invalidateTile(tile.tx, tile.ty); } - else { - mtile.delete(); - } - MapManager.mapman.pushUpdate(tile.getDynmapWorld(), new Client.Tile(mtile.getURI())); - tile_update = true; - renderone = true; } else { if(!rendered[i]) { - mtile.delete(); + if (!mtile.delete()) world.getMapState(shaderstate[i].getMap()).invalidateTile(tile.tx, tile.ty); } } } finally { diff --git a/DynmapCore/src/main/java/org/dynmap/storage/MapStorage.java b/DynmapCore/src/main/java/org/dynmap/storage/MapStorage.java index fde504ed4..e2050884d 100644 --- a/DynmapCore/src/main/java/org/dynmap/storage/MapStorage.java +++ b/DynmapCore/src/main/java/org/dynmap/storage/MapStorage.java @@ -26,7 +26,7 @@ public abstract class MapStorage { private static HashMap filelocks = new HashMap(); private static final Integer WRITELOCK = (-1); protected File baseStandaloneDir; - protected boolean isShutdown; + protected volatile boolean isShutdown; protected long serverID; diff --git a/DynmapCore/src/main/java/org/dynmap/storage/StorageReadException.java b/DynmapCore/src/main/java/org/dynmap/storage/StorageReadException.java new file mode 100644 index 000000000..404dcf746 --- /dev/null +++ b/DynmapCore/src/main/java/org/dynmap/storage/StorageReadException.java @@ -0,0 +1,6 @@ +package org.dynmap.storage; + +/** A failed read is not an absent tile: callers must preserve and retry the work. */ +public class StorageReadException extends RuntimeException { + public StorageReadException(Throwable cause) { super(cause); } +} diff --git a/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java index d4250eacd..80c5ba92b 100644 --- a/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java +++ b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java @@ -1,6 +1,7 @@ package org.dynmap.storage.aws_s3; import java.io.IOException; +import java.io.UncheckedIOException; import java.net.URI; import java.nio.charset.StandardCharsets; import java.security.MessageDigest; @@ -21,6 +22,7 @@ import org.dynmap.PlayerFaces.FaceType; import org.dynmap.WebAuthManager; import org.dynmap.storage.MapStorage; +import org.dynmap.storage.StorageReadException; import org.dynmap.storage.MapStorageTile; import org.dynmap.storage.MapStorageTileEnumCB; import org.dynmap.storage.MapStorageBaseTileEnumCB; @@ -72,14 +74,13 @@ public boolean exists() { s3 = getConnection(); ListObjectsV2Request req = ListObjectsV2Request.builder().bucketName(bucketname).prefix(baseKey).maxKeys(1).build(); ListObjectsV2Response rslt = s3.listObjectsV2(req); - if ((rslt != null) && (rslt.getKeyCount() > 0)) - exists = true; - } catch (S3Exception x) { - if (!x.getCode().equals("SignatureDoesNotMatch")) { // S3 behavior when no object match.... - Log.severe("AWS Exception", x); - } - } catch (StorageShutdownException x) { - + if (rslt != null) { + for (S3Object object : rslt.getContents()) { + if (baseKey.equals(object.getKey())) exists = true; + } + } + } catch (S3Exception | UncheckedIOException | StorageShutdownException x) { + throw new StorageReadException(x); } finally { releaseConnection(s3); } @@ -118,9 +119,8 @@ public TileRead read() { } } catch (NoSuchKeyException nskx) { return null; // Nominal case if it doesn't exist - } catch (S3Exception x) { - Log.severe("AWS Exception", x); - } catch (StorageShutdownException x) { + } catch (S3Exception | UncheckedIOException | StorageShutdownException x) { + throw new StorageReadException(x); } finally { releaseConnection(s3); } @@ -143,14 +143,14 @@ public boolean write(long hash, BufferOutputStream encImage, long timestamp) { s3.putObject(req, RequestBody.fromBytes(Arrays.copyOf(encImage.buf, encImage.len))); } done = true; - } catch (S3Exception x) { + } catch (S3Exception | UncheckedIOException x) { Log.severe("AWS Exception", x); } catch (StorageShutdownException x) { } finally { releaseConnection(s3); } // Signal update for zoom out - if (zoom == 0) { + if (done && zoom == 0) { world.enqueueZoomOutUpdate(this); } return done; @@ -287,7 +287,7 @@ public boolean init(DynmapCore core) { return false; } rslt.getContents(); - } catch (S3Exception s3x) { + } catch (S3Exception | UncheckedIOException s3x) { Log.severe("AWS Exception", s3x); return false; } catch (StorageShutdownException x) { @@ -413,8 +413,8 @@ private void processEnumMapTiles(DynmapWorld world, MapType map, ImageVariant va done = true; } } - } catch (S3Exception x) { - if (!x.getCode().equals("SignatureDoesNotMatch")) { // S3 behavior when no object match.... + } catch (S3Exception | UncheckedIOException x) { + if (!(x instanceof S3Exception) || !"SignatureDoesNotMatch".equals(((S3Exception)x).getCode())) { Log.severe("AWS Exception", x); Log.severe("req=" + req); } @@ -486,8 +486,8 @@ private void processPurgeMapTiles(DynmapWorld world, MapType map, ImageVariant v done = true; } } - } catch (S3Exception x) { - if (!x.getCode().equals("SignatureDoesNotMatch")) { // S3 behavior when no object match.... + } catch (S3Exception | UncheckedIOException x) { + if (!(x instanceof S3Exception) || !"SignatureDoesNotMatch".equals(((S3Exception)x).getCode())) { Log.severe("AWS Exception", x); Log.severe("req=" + req); } @@ -532,7 +532,7 @@ public boolean setPlayerFaceImage(String playername, FaceType facetype, s3.putObject(req, RequestBody.fromBytes(Arrays.copyOf(encImage.buf, encImage.len))); } done = true; - } catch (S3Exception x) { + } catch (S3Exception | UncheckedIOException x) { Log.severe("AWS Exception", x); } catch (StorageShutdownException x) { } finally { @@ -558,8 +558,8 @@ public boolean hasPlayerFaceImage(String playername, FaceType facetype) { ListObjectsV2Response rslt = s3.listObjectsV2(req); if ((rslt != null) && (rslt.getKeyCount() > 0)) exists = true; - } catch (S3Exception x) { - if (!x.getCode().equals("SignatureDoesNotMatch")) { // S3 behavior when no object match.... + } catch (S3Exception | UncheckedIOException x) { + if (!(x instanceof S3Exception) || !"SignatureDoesNotMatch".equals(((S3Exception)x).getCode())) { Log.severe("AWS Exception", x); } } catch (StorageShutdownException x) { @@ -585,7 +585,7 @@ public boolean setMarkerImage(String markerid, BufferOutputStream encImage) { s3.putObject(req, RequestBody.fromBytes(Arrays.copyOf(encImage.buf, encImage.len))); } done = true; - } catch (S3Exception x) { + } catch (S3Exception | UncheckedIOException x) { Log.severe("AWS Exception", x); } catch (StorageShutdownException x) { } finally { @@ -615,7 +615,7 @@ public boolean setMarkerFile(String world, String content) { s3.putObject(req, RequestBody.fromString(content)); } done = true; - } catch (S3Exception x) { + } catch (S3Exception | UncheckedIOException x) { Log.severe("AWS Exception", x); } catch (StorageShutdownException x) { } finally { @@ -686,7 +686,7 @@ public BufferInputStream getStandaloneFile(String fileid) { @Override public boolean setStandaloneFile(String fileid, BufferOutputStream content) { - return setStaticWebFile("standalone/" + fileid, content); + return setWebFile("standalone/" + fileid, content, false); } // Test if storage needs static web files public boolean needsStaticWebFiles() { @@ -699,6 +699,10 @@ public boolean needsStaticWebFiles() { * @return true if successful */ public boolean setStaticWebFile(String fileid, BufferOutputStream content) { + return setWebFile(fileid, content, true); + } + + private synchronized boolean setWebFile(String fileid, BufferOutputStream content, boolean verifyExisting) { boolean done = false; String baseKey = prefix + fileid; @@ -719,7 +723,7 @@ public boolean setStaticWebFile(String fileid, BufferOutputStream content) { byte[] digest = content.buf; try { MessageDigest md = MessageDigest.getInstance("MD5"); - md.update(content.buf); + md.update(content.buf, 0, content.len); digest = md.digest(); } catch (NoSuchAlgorithmException nsax) { @@ -728,6 +732,18 @@ public boolean setStaticWebFile(String fileid, BufferOutputStream content) { if (Arrays.equals(digest, cacheval)) { return true; } + // Cold-start verification only for bundled assets, never live update JSON. + // GET also repairs deleted assets on the next startup; no persisted blind cache. + if (cacheval == null && verifyExisting) { + try { + ResponseBytes existing = s3.getObjectAsBytes( + GetObjectRequest.builder().bucketName(bucketname).key(baseKey).build()); + if (existing != null && Arrays.equals(existing.getBytes(), Arrays.copyOf(content.buf, content.len))) { + standalone_cache.put(fileid, digest); + return true; + } + } catch (NoSuchKeyException absent) { /* Initial publish or missing asset. */ } + } String ct = "text/plain"; if (fileid.endsWith(".json")) { ct = "application/json"; @@ -749,7 +765,7 @@ else if (fileid.endsWith(".js")) { standalone_cache.put(fileid, digest); } done = true; - } catch (S3Exception x) { + } catch (S3Exception | UncheckedIOException x) { Log.severe("AWS Exception", x); } catch (StorageShutdownException x) { } finally { @@ -763,6 +779,7 @@ private S3Client getConnection() throws S3Exception, StorageShutdownException { if (isShutdown) throw new StorageShutdownException(); synchronized (cpool) { while (c == null) { + if (isShutdown) throw new StorageShutdownException(); for (int i = 0; i < cpool.length; i++) { // See if available connection if (cpool[i] != null) { // Found one c = cpool[i]; @@ -772,11 +789,14 @@ private S3Client getConnection() throws S3Exception, StorageShutdownException { } if (c == null) { if (cpoolCount < POOLSIZE) { // Still more we can have - c = new DefaultS3ClientBuilder() + c = RetryingS3Client.wrap(new DefaultS3ClientBuilder() .credentialsProvider(() -> AwsBasicCredentials.create(access_key_id, secret_access_key)) .region(region) - .httpClient(URLConnectionSdkHttpClient.create()) - .build(); + .httpClient(URLConnectionSdkHttpClient.withCustomizer(connection -> { + connection.setConnectTimeout(5000); + connection.setReadTimeout(10000); + })) + .build()); if (c == null) { Log.severe("Error creating S3 access client"); return null; @@ -785,9 +805,10 @@ private S3Client getConnection() throws S3Exception, StorageShutdownException { } else { try { - cpool.wait(); + cpool.wait(1000); } catch (InterruptedException e) { - return null; + Thread.currentThread().interrupt(); + throw new StorageShutdownException(); } } } diff --git a/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/RetryingS3Client.java b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/RetryingS3Client.java new file mode 100644 index 000000000..ad1a7817c --- /dev/null +++ b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/RetryingS3Client.java @@ -0,0 +1,42 @@ +package org.dynmap.storage.aws_s3; + +import java.io.UncheckedIOException; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Proxy; +import io.github.linktosriram.s3lite.api.client.S3Client; +import io.github.linktosriram.s3lite.api.exception.S3Exception; + +/** Bounded retries for the idempotent operations used by this storage backend. */ +final class RetryingS3Client { + interface Sleeper { void sleep(long millis) throws InterruptedException; } + + static S3Client wrap(S3Client delegate) { + return wrap(delegate, Thread::sleep); + } + + static S3Client wrap(S3Client delegate, Sleeper sleeper) { + return (S3Client) Proxy.newProxyInstance(S3Client.class.getClassLoader(), + new Class[] { S3Client.class }, (proxy, method, args) -> { + for (int attempt = 0; ; attempt++) { + try { + return method.invoke(delegate, args); + } catch (InvocationTargetException ex) { + Throwable cause = ex.getCause(); + boolean retry = cause instanceof UncheckedIOException; + if (cause instanceof S3Exception) { + String code = ((S3Exception) cause).getCode(); + retry = "InternalError".equals(code) || "ServiceUnavailable".equals(code) + || "SlowDown".equals(code) || "RequestTimeout".equals(code); + } + if (!retry || attempt >= 2 || Thread.currentThread().isInterrupted()) throw cause; + try { + sleeper.sleep(attempt == 0 ? 250 : 1000); + } catch (InterruptedException interrupted) { + Thread.currentThread().interrupt(); + throw cause; + } + } + } + }); + } +} diff --git a/DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java b/DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java new file mode 100644 index 000000000..f7ee53768 --- /dev/null +++ b/DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java @@ -0,0 +1,68 @@ +package org.dynmap.utils; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.function.BiPredicate; +import java.util.function.Consumer; + +/** Single writer, latest value per key, finite batches and capped retry frequency. */ +public final class RetryingFileQueue { + private final Map files = new LinkedHashMap<>(); + private final BiPredicate writer; + private final BiPredicate scheduler; + private final Consumer failure; + private boolean scheduled; + private int failures; + + public RetryingFileQueue(BiPredicate writer, + BiPredicate scheduler, Consumer failure) { + this.writer = writer; + this.scheduler = scheduler; + this.failure = failure; + } + + public synchronized void enqueue(String key, BufferOutputStream value) { + files.put(key, value); + schedule(0); + } + + private void schedule(long delay) { + if (scheduled || files.isEmpty()) return; + scheduled = true; + try { + if (!scheduler.test(this::runBatch, delay)) scheduled = false; + } catch (RuntimeException ex) { + scheduled = false; + throw ex; + } + } + + private void runBatch() { + String[] keys; + synchronized (this) { keys = files.keySet().toArray(new String[0]); } + boolean failed = false; + try { + for (String key : keys) { + BufferOutputStream value; + synchronized (this) { value = files.remove(key); } + boolean success = false; + try { success = writer.test(key, value); } + catch (RuntimeException ex) { /* Retain the value, and service the other files. */ } + if (!success) { + failed = true; + synchronized (this) { + // containsKey matters: a newer deletion is represented by null. + if (!files.containsKey(key)) files.put(key, value); + } + failure.accept(key); + } + } + } finally { + synchronized (this) { + scheduled = false; + failures = failed ? Math.min(failures + 1, 5) : 0; + schedule(failed ? Math.min(60000L, 5000L << (failures - 1)) : 0); + } + } + } +} diff --git a/DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java b/DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java new file mode 100644 index 000000000..f1aaecf06 --- /dev/null +++ b/DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java @@ -0,0 +1,54 @@ +package org.dynmap; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; +import java.io.IOException; +import java.lang.reflect.Field; +import java.util.ArrayList; +import org.dynmap.storage.*; +import org.junit.*; + +public class ZoomStorageFailureTest { + private MapManager previous; + private DynmapWorld world; + private MapTypeState state; + private MapStorageTile child, parent; + @Before public void setup() throws Exception { + previous = MapManager.mapman; MapManager.mapman = mock(MapManager.class); + world = mock(DynmapWorld.class, CALLS_REAL_METHODS); + doNothing().when(world).enqueueZoomOutUpdate(any()); + MapType map = mock(MapType.class); + when(map.getTileSize()).thenReturn(128); + when(map.getMapZoomOutLevels()).thenReturn(3); + when(map.getImageFormat()).thenReturn(MapType.ImageFormat.FORMAT_PNG); + when(map.getVariants()).thenReturn(new MapType.ImageVariant[] {MapType.ImageVariant.STANDARD}); + state = new MapTypeState(world,map); + world.mapstate = new ArrayList<>(); world.mapstate.add(state); + MapStorage storage = mock(MapStorage.class); + Field f = DynmapWorld.class.getDeclaredField("storage"); f.setAccessible(true); f.set(world,storage); + child = mock(MapStorageTile.class); parent = mock(MapStorageTile.class); + Field mapField = MapStorageTile.class.getDeclaredField("map"); mapField.setAccessible(true); mapField.set(child,map); + // Public final coordinate fields on the mocks default to zero (the origin). + when(child.getZoomOutTile()).thenReturn(parent); + when(storage.getTile(eq(world),any(),anyInt(),anyInt(),anyInt(),any())).thenReturn(child); + when(parent.exists()).thenReturn(true); + state.setZoomOutInv(0,0,0); + } + @After public void cleanup() { MapManager.mapman = previous; } + @Test public void failedReadPreservesParentAndRetriesNextPass() { + when(child.read()).thenThrow(new StorageReadException(new IOException())); + world.freshenZoomOutFiles(); + verify(parent,never()).delete(); verifyNoInteractions(MapManager.mapman); + assertNotNull(state.saveZoomOut()); + world.freshenZoomOutFiles(); + verify(child,times(2)).read(); + } + @Test public void failedDeleteDoesNotPublishAndRetryCanRecover() { + when(parent.delete()).thenReturn(false,true); + world.freshenZoomOutFiles(); + verifyNoInteractions(MapManager.mapman); assertNotNull(state.saveZoomOut()); + world.freshenZoomOutFiles(); + verify(parent,times(2)).delete(); + verify(MapManager.mapman).pushUpdate(eq(world),any(Client.Update.class)); + } +} diff --git a/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/AWSS3ReliabilityTest.java b/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/AWSS3ReliabilityTest.java new file mode 100644 index 000000000..acecb66c3 --- /dev/null +++ b/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/AWSS3ReliabilityTest.java @@ -0,0 +1,78 @@ +package org.dynmap.storage.aws_s3; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.lang.reflect.Field; +import org.dynmap.*; +import org.dynmap.storage.*; +import org.dynmap.utils.BufferOutputStream; +import org.junit.Before; +import org.junit.Test; +import io.github.linktosriram.s3lite.api.client.S3Client; +import io.github.linktosriram.s3lite.api.exception.NoSuchKeyException; +import io.github.linktosriram.s3lite.api.request.*; +import io.github.linktosriram.s3lite.api.response.*; +import io.github.linktosriram.s3lite.http.spi.request.RequestBody; + +public class AWSS3ReliabilityTest { + private AWSS3MapStorage storage; + private S3Client client; + private DynmapWorld world; + private MapStorageTile tile; + private void field(String name, Object value) throws Exception { + Field f = AWSS3MapStorage.class.getDeclaredField(name); f.setAccessible(true); f.set(storage,value); + } + @Before public void setup() throws Exception { + storage = new AWSS3MapStorage(); client = mock(S3Client.class); + field("cpool",new S3Client[] {client,null,null,null}); field("cpoolCount",1); + field("bucketname","test"); field("prefix",""); + world = mock(DynmapWorld.class); when(world.getName()).thenReturn("world"); + MapType map = mock(MapType.class); when(map.getPrefix()).thenReturn("flat"); + when(map.getImageFormat()).thenReturn(MapType.ImageFormat.FORMAT_PNG); + tile = storage.getTile(world,map,0,0,0,MapType.ImageVariant.STANDARD); + } + private BufferOutputStream bytes(int value) { BufferOutputStream b = new BufferOutputStream(); b.write(new byte[] {(byte)value}); return b; } + @Test public void failedWriteDoesNotQueueZoomOrCacheSuccess() { + when(client.putObject(any(PutObjectRequest.class),any(RequestBody.class))) + .thenThrow(new UncheckedIOException(new IOException())); + assertFalse(tile.write(1,bytes(1),1)); verify(world,never()).enqueueZoomOutUpdate(any()); + assertFalse(storage.setStandaloneFile("test.json",bytes(1))); + assertFalse(storage.setStandaloneFile("test.json",bytes(1))); + verify(client,times(3)).putObject(any(PutObjectRequest.class),any(RequestBody.class)); + } + @Test public void successfulWriteQueuesZoom() { + assertTrue(tile.write(1,bytes(1),1)); verify(world).enqueueZoomOutUpdate(tile); + } + @Test public void readFailureIsNotMissingTile() { + when(client.getObjectAsBytes(any(GetObjectRequest.class))).thenThrow(new UncheckedIOException(new IOException())); + try { tile.read(); fail(); } catch (StorageReadException expected) { } + } + @Test public void realNotFoundIsMissingTile() { + when(client.getObjectAsBytes(any(GetObjectRequest.class))).thenThrow(mock(NoSuchKeyException.class)); + assertNull(tile.read()); + } + @Test public void sameAssetSkipsPutButDifferentAssetIsPublished() { + ResponseBytes existing = mock(ResponseBytes.class); + when(existing.getBytes()).thenReturn(new byte[] {1}); + when(client.getObjectAsBytes(any(GetObjectRequest.class))).thenReturn(existing); + assertTrue(storage.setStaticWebFile("asset.js",bytes(1))); + verify(client,never()).putObject(any(PutObjectRequest.class),any(RequestBody.class)); + assertTrue(storage.setStaticWebFile("asset.js",bytes(2))); + verify(client).putObject(any(PutObjectRequest.class),any(RequestBody.class)); + } + @Test public void missingAssetIsPublishedAndLiveJsonIsNeverReadCached() { + when(client.getObjectAsBytes(any(GetObjectRequest.class))).thenThrow(mock(NoSuchKeyException.class)); + assertTrue(storage.setStaticWebFile("asset.js",bytes(1))); + assertTrue(storage.setStandaloneFile("update.json",bytes(1))); + verify(client).getObjectAsBytes(any(GetObjectRequest.class)); + verify(client,times(2)).putObject(any(PutObjectRequest.class),any(RequestBody.class)); + } + @Test public void digestIgnoresUnusedBufferCapacity() { + BufferOutputStream a = bytes(1), b = bytes(1); b.buf[b.len] = 42; + assertTrue(storage.setStandaloneFile("update.json",a)); + assertTrue(storage.setStandaloneFile("update.json",b)); + verify(client).putObject(any(PutObjectRequest.class),any(RequestBody.class)); + } +} diff --git a/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/RetryingS3ClientTest.java b/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/RetryingS3ClientTest.java new file mode 100644 index 000000000..5776268eb --- /dev/null +++ b/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/RetryingS3ClientTest.java @@ -0,0 +1,54 @@ +package org.dynmap.storage.aws_s3; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; +import java.io.UncheckedIOException; +import java.net.SocketTimeoutException; +import java.util.*; +import org.junit.Test; +import io.github.linktosriram.s3lite.api.client.S3Client; +import io.github.linktosriram.s3lite.api.exception.S3Exception; +import io.github.linktosriram.s3lite.api.request.GetObjectRequest; + +public class RetryingS3ClientTest { + private final GetObjectRequest request = GetObjectRequest.builder().bucketName("test").key("test").build(); + private S3Exception error(String code) { S3Exception e = mock(S3Exception.class); when(e.getCode()).thenReturn(code); return e; } + + @Test public void transportFailureRetriesThenSucceeds() { + S3Client client = mock(S3Client.class); + when(client.getObjectAsBytes(request)).thenThrow(new UncheckedIOException(new SocketTimeoutException())).thenReturn(null); + List sleeps = new ArrayList<>(); + RetryingS3Client.wrap(client, sleeps::add).getObjectAsBytes(request); + verify(client, times(2)).getObjectAsBytes(request); + assertEquals(Collections.singletonList(250L), sleeps); + } + @Test public void internalErrorHasThreeAttemptsNotInfiniteLoop() { + S3Client client = mock(S3Client.class); S3Exception e = error("InternalError"); + when(client.getObjectAsBytes(request)).thenThrow(e); + List sleeps = new ArrayList<>(); + try { RetryingS3Client.wrap(client, sleeps::add).getObjectAsBytes(request); fail(); } + catch (S3Exception actual) { assertSame(e, actual); } + verify(client, times(3)).getObjectAsBytes(request); + assertEquals(Arrays.asList(250L,1000L), sleeps); + } + @Test public void authenticationAndNotFoundAreNotRetried() { + for (String code : Arrays.asList("AccessDenied", "NoSuchKey", "SignatureDoesNotMatch")) { + S3Client client = mock(S3Client.class); S3Exception e = error(code); + when(client.getObjectAsBytes(request)).thenThrow(e); + try { RetryingS3Client.wrap(client, ms -> fail()).getObjectAsBytes(request); fail(); } + catch (S3Exception actual) { assertSame(e, actual); } + verify(client).getObjectAsBytes(request); + } + } + @Test public void interruptionIsPreservedAndStopsRetries() { + S3Client client = mock(S3Client.class); + S3Exception error = error("InternalError"); + when(client.getObjectAsBytes(request)).thenThrow(error); + try { + RetryingS3Client.wrap(client, ms -> { throw new InterruptedException(); }).getObjectAsBytes(request); + fail(); + } catch (S3Exception expected) { assertTrue(Thread.currentThread().isInterrupted()); } + finally { Thread.interrupted(); } + verify(client).getObjectAsBytes(request); + } +} diff --git a/DynmapCore/src/test/java/org/dynmap/utils/RetryingFileQueueTest.java b/DynmapCore/src/test/java/org/dynmap/utils/RetryingFileQueueTest.java new file mode 100644 index 000000000..632a80b15 --- /dev/null +++ b/DynmapCore/src/test/java/org/dynmap/utils/RetryingFileQueueTest.java @@ -0,0 +1,68 @@ +package org.dynmap.utils; + +import static org.junit.Assert.*; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.util.*; +import java.util.concurrent.atomic.AtomicInteger; +import org.junit.Test; + +public class RetryingFileQueueTest { + private final Deque jobs = new ArrayDeque<>(); + private final List delays = new ArrayList<>(); + private boolean schedule(Runnable job, Long delay) { jobs.add(job); delays.add(delay); return true; } + private BufferOutputStream value(int b) { BufferOutputStream v = new BufferOutputStream(); v.write(new byte[] {(byte)b}); return v; } + + @Test public void failureDoesNotStrandOtherFilesAndEventuallyRecovers() { + AtomicInteger calls = new AtomicInteger(); + List success = new ArrayList<>(); + RetryingFileQueue q = new RetryingFileQueue((k,v) -> { + if (k.equals("bad") && calls.getAndIncrement() == 0) throw new UncheckedIOException(new IOException()); + success.add(k); return true; + }, this::schedule, k -> {}); + q.enqueue("bad", value(1)); q.enqueue("good", value(2)); + jobs.remove().run(); + assertEquals(Collections.singletonList("good"), success); + assertEquals(Long.valueOf(5000), delays.get(1)); + jobs.remove().run(); + assertEquals(Arrays.asList("good", "bad"), success); + assertTrue(jobs.isEmpty()); + q.enqueue("later", value(3)); jobs.remove().run(); + assertEquals("later", success.get(2)); + } + + @Test public void failedOlderValueCannotReplaceNewValueOrDeletion() { + for (BufferOutputStream latest : Arrays.asList(value(2), null)) { + List attempts = new ArrayList<>(); + RetryingFileQueue[] queue = new RetryingFileQueue[1]; + queue[0] = new RetryingFileQueue((k,v) -> { + attempts.add(v); + if (attempts.size() == 1) { queue[0].enqueue(k, latest); return false; } + return true; + }, this::schedule, k -> {}); + queue[0].enqueue("same", value(1)); + jobs.remove().run(); jobs.remove().run(); + assertSame(latest, attempts.get(1)); + assertTrue(jobs.isEmpty()); + } + } + + @Test public void repeatedFailureUsesFiniteBatchesAndCappedBackoff() { + AtomicInteger calls = new AtomicInteger(); + RetryingFileQueue q = new RetryingFileQueue((k,v) -> { calls.incrementAndGet(); return false; }, this::schedule, k -> {}); + q.enqueue("file", value(1)); + for (int i=0; i<12; i++) { jobs.remove().run(); assertEquals(1, jobs.size()); } + assertEquals(12, calls.get()); + assertTrue(delays.get(delays.size()-1) >= 30000); + assertTrue(Collections.max(delays) <= 60000); + } + + @Test public void rejectedScheduleCanBeStartedByNextEnqueue() { + AtomicInteger schedules = new AtomicInteger(); + List writes = new ArrayList<>(); + RetryingFileQueue q = new RetryingFileQueue((k,v) -> { writes.add(k); return true; }, + (r,d) -> schedules.getAndIncrement() != 0 && schedule(r,d), k -> {}); + q.enqueue("first", value(1)); q.enqueue("second", value(2)); + jobs.remove().run(); assertEquals(Arrays.asList("first", "second"), writes); + } +} diff --git a/docs/r2-reliability.md b/docs/r2-reliability.md new file mode 100644 index 000000000..108d0d1cb --- /dev/null +++ b/docs/r2-reliability.md @@ -0,0 +1,19 @@ +# S3互換ストレージの障害耐性（Issue #8） + +- 通信は接続5秒・読取り10秒。ストレージ専用クライアントの設定で、TLS検証やJVM全体の設定は変えない。 +- `UncheckedIOException`、InternalError、ServiceUnavailable、SlowDown、RequestTimeoutは最大3試行（待機250ms/1000ms）。認証エラーや404は即返す。これは通信全体・書込み・プール待ちの厳密な総時間制限ではない。 +- JSONはファイル名ごとに最新内容へ集約し、失敗後も他ファイルを処理する。再試行は有限バッチで5/10/20/40/60秒と間隔を伸ばし、60秒を上限に保持する。古い失敗内容が新しい内容や削除指示を上書きしない。 +- 本描画の書込み失敗は通常更新の無効タイルとして残す。更新通知・updated統計は成功時だけ。全域描画の走査完了だけを保存完了とみなさず、通常更新キューも確認する。 +- ズームの読取り障害は欠落と区別し、既存画像を消さない。読取り/書込み/削除失敗は次のズーム周期へ残す。正常に存在しないタイルは従来どおり空白として扱う。 +- 起動時Web資材は描画executorで非同期公開する。プロセス初回はR2の実内容をGETして一致した資材のPUTを省くため、削除済み資材は再作成できる。版番号差分も内容比較の対象。GETと転送量は増えるが、同値PUTは減る。初回のWeb表示は資材公開完了まで待つ必要がある。 +- ライブJSONにGET確認や長期読取りキャッシュを追加しない。同値抑制用のローカルdigestは有効長だけを対象とし、確認成功後だけ確定する。 + +## 検証と展開 + +コアの模擬S3/キューテストで失敗・復帰・最新値・削除・認証エラー・割込み・ズーム維持を検査する。 +SpigotとMC26向けのビルドと隔離サーバー検証後、バックアップを取得して1台ずつ導入する。 +Enabledだけでは非同期Web公開完了を保証しない。公開index/version.js/config.js、設定JSON、ライブJSON、実タイルとログを別に確認する。 + +既に旧版で失敗したズームは自動的には列挙できない。対象のベースタイルから必要なズーム更新を限定的に再登録して回復させる。移行中の全域ジョブ・pendingデータ・旧prefixは保持し、全域再生成や既存オブジェクト削除を復旧の前提にしない。 + +プロバイダーの内部エラー発生原因とクライアントの障害耐性は別問題。継続する障害は成功扱いせず、移行公開を保留する。 From dc797a389a810c67e18185a0d547fdc02ba382d6 Mon Sep 17 00:00:00 2001 From: Gui <51219838+gui-ace@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:43:09 +0900 Subject: [PATCH 2/3] =?UTF-8?q?S3=E3=82=A8=E3=83=A9=E3=83=BC=E8=A7=A3?= =?UTF-8?q?=E6=9E=90=E3=82=92=E6=97=A7JAXB=E3=81=8B=E3=82=89=E5=88=86?= =?UTF-8?q?=E9=9B=A2=E3=81=97=E5=AE=89=E5=85=A8=E3=81=AAXML=E8=A7=A3?= =?UTF-8?q?=E6=9E=90=E3=81=AB=E7=BD=AE=E6=8F=9B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../storage/aws_s3/AWSS3MapStorage.java | 6 +- .../storage/aws_s3/SafeS3HttpClient.java | 82 +++++++++++++++++++ .../org/dynmap/utils/RetryingFileQueue.java | 4 +- .../storage/aws_s3/SafeS3HttpClientTest.java | 59 +++++++++++++ docs/r2-reliability.md | 1 + 5 files changed, 146 insertions(+), 6 deletions(-) create mode 100644 DynmapCore/src/main/java/org/dynmap/storage/aws_s3/SafeS3HttpClient.java create mode 100644 DynmapCore/src/test/java/org/dynmap/storage/aws_s3/SafeS3HttpClientTest.java diff --git a/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java index 80c5ba92b..03bee97cc 100644 --- a/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java +++ b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/AWSS3MapStorage.java @@ -45,7 +45,6 @@ import io.github.linktosriram.s3lite.core.auth.AwsBasicCredentials; import io.github.linktosriram.s3lite.core.client.DefaultS3ClientBuilder; import io.github.linktosriram.s3lite.http.spi.request.RequestBody; -import io.github.linktosriram.s3lite.http.urlconnection.URLConnectionSdkHttpClient; public class AWSS3MapStorage extends MapStorage { public class StorageTile extends MapStorageTile { @@ -792,10 +791,7 @@ private S3Client getConnection() throws S3Exception, StorageShutdownException { c = RetryingS3Client.wrap(new DefaultS3ClientBuilder() .credentialsProvider(() -> AwsBasicCredentials.create(access_key_id, secret_access_key)) .region(region) - .httpClient(URLConnectionSdkHttpClient.withCustomizer(connection -> { - connection.setConnectTimeout(5000); - connection.setReadTimeout(10000); - })) + .httpClient(SafeS3HttpClient.create()) .build()); if (c == null) { Log.severe("Error creating S3 access client"); diff --git a/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/SafeS3HttpClient.java b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/SafeS3HttpClient.java new file mode 100644 index 000000000..b6b783fa1 --- /dev/null +++ b/DynmapCore/src/main/java/org/dynmap/storage/aws_s3/SafeS3HttpClient.java @@ -0,0 +1,82 @@ +package org.dynmap.storage.aws_s3; + +import java.io.*; +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; +import org.w3c.dom.NodeList; +import org.xml.sax.ErrorHandler; +import org.xml.sax.SAXException; +import org.xml.sax.SAXParseException; +import io.github.linktosriram.s3lite.api.exception.ErrorResponse; +import io.github.linktosriram.s3lite.api.exception.NoSuchKeyException; +import io.github.linktosriram.s3lite.api.exception.S3Exception; +import io.github.linktosriram.s3lite.http.spi.SdkHttpClient; +import io.github.linktosriram.s3lite.http.spi.request.ImmutableRequest; +import io.github.linktosriram.s3lite.http.spi.response.ImmutableResponse; +import io.github.linktosriram.s3lite.http.urlconnection.URLConnectionSdkHttpClient; + +/** Keep SDK error handling independent of the old JAXB runtime and JVM-wide flags. */ +final class SafeS3HttpClient implements SdkHttpClient { + private final SdkHttpClient delegate; + SafeS3HttpClient(SdkHttpClient delegate) { this.delegate = delegate; } + + static SdkHttpClient create() { + return new SafeS3HttpClient(URLConnectionSdkHttpClient.withCustomizer(connection -> { + connection.setConnectTimeout(5000); + connection.setReadTimeout(10000); + })); + } + + @Override public ImmutableResponse apply(ImmutableRequest request) { + ImmutableResponse response = delegate.apply(request); + if (response.getStatus().is2xxSuccessful()) return response; + int status = response.getStatus().getStatusCode(); + String code = status >= 500 ? "ServiceUnavailable" : status == 429 ? "SlowDown" + : status == 408 ? "RequestTimeout" : "HTTP" + status; + try (InputStream stream = response.getResponseBody().orElse(null)) { + if (stream != null) { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + int length; + while (bytes.size() <= 65536 && (length = stream.read(buffer)) != -1) bytes.write(buffer, 0, length); + if (bytes.size() <= 65536) { + String parsed = errorCode(bytes.toByteArray()); + if (parsed != null) code = parsed; + } + } + } catch (IOException ex) { throw new UncheckedIOException(ex); } + ErrorResponse error = new ErrorResponse(); + error.setCode(code); + // Do not log arbitrary server response bodies, keys or potentially sensitive messages. + error.setMessage("S3 HTTP " + status + " (" + code + ")"); + if ("NoSuchKey".equals(code) && status == 404) throw new NoSuchKeyException(error); + throw new S3Exception(error); + } + + private static String errorCode(byte[] xml) { + try { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + factory.setNamespaceAware(true); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); + javax.xml.parsers.DocumentBuilder builder = factory.newDocumentBuilder(); + builder.setErrorHandler(new ErrorHandler() { + public void warning(SAXParseException ex) throws SAXException { throw ex; } + public void error(SAXParseException ex) throws SAXException { throw ex; } + public void fatalError(SAXParseException ex) throws SAXException { throw ex; } + }); + NodeList codes = builder.parse(new ByteArrayInputStream(xml)).getElementsByTagNameNS("*", "Code"); + if (codes.getLength() == 1) { + String value = codes.item(0).getTextContent(); + if (value.matches("[A-Za-z0-9]{1,64}")) return value; + } + } catch (Exception invalid) { /* Fail closed to HTTP status, never expand external entities. */ } + return null; + } + + @Override public void close() throws IOException { delegate.close(); } +} diff --git a/DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java b/DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java index f7ee53768..eb361b464 100644 --- a/DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java +++ b/DynmapCore/src/main/java/org/dynmap/utils/RetryingFileQueue.java @@ -47,7 +47,9 @@ private void runBatch() { synchronized (this) { value = files.remove(key); } boolean success = false; try { success = writer.test(key, value); } - catch (RuntimeException ex) { /* Retain the value, and service the other files. */ } + catch (RuntimeException ex) { + org.dynmap.Log.severe("Unexpected file publication failure; retaining " + key, ex); + } if (!success) { failed = true; synchronized (this) { diff --git a/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/SafeS3HttpClientTest.java b/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/SafeS3HttpClientTest.java new file mode 100644 index 000000000..d86c8c464 --- /dev/null +++ b/DynmapCore/src/test/java/org/dynmap/storage/aws_s3/SafeS3HttpClientTest.java @@ -0,0 +1,59 @@ +package org.dynmap.storage.aws_s3; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; +import java.io.*; +import java.lang.reflect.Field; +import java.nio.charset.StandardCharsets; +import java.util.Optional; +import java.util.function.Consumer; +import javax.net.ssl.HttpsURLConnection; +import org.junit.Test; +import io.github.linktosriram.s3lite.api.exception.*; +import io.github.linktosriram.s3lite.http.spi.*; +import io.github.linktosriram.s3lite.http.spi.response.ImmutableResponse; +import io.github.linktosriram.s3lite.http.urlconnection.URLConnectionSdkHttpClient; + +public class SafeS3HttpClientTest { + private S3Exception failure(int status, String body) { + SdkHttpClient delegate = mock(SdkHttpClient.class); + ImmutableResponse response = mock(ImmutableResponse.class); + when(response.getStatus()).thenReturn(HttpStatus.fromStatusCode(status)); + when(response.getResponseBody()).thenReturn(Optional.of(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)))); + when(delegate.apply(null)).thenReturn(response); + try { new SafeS3HttpClient(delegate).apply(null); throw new AssertionError(); } + catch (S3Exception error) { return error; } + } + @Test public void errorXmlDoesNotRequireJaxbAnd404IsTyped() { + assertTrue(failure(404,"NoSuchKey") instanceof NoSuchKeyException); + assertEquals("InternalError",failure(500,"InternalError").getCode()); + assertFalse(failure(500,"NoSuchKey") instanceof NoSuchKeyException); + } + @Test public void malformedOrOversizedOrEntityResponseFailsClosed() { + assertEquals("ServiceUnavailable",failure(503,"unavailable").getCode()); + assertEquals("HTTP404",failure(404,"]>&ex;").getCode()); + assertEquals("HTTP404",failure(404,new String(new char[70000]).replace('\0','x')).getCode()); + assertEquals("SlowDown",failure(429,"").getCode()); + } + @Test public void bodyIsNotIncludedInExceptionMessage() { + S3Exception error = failure(403,"AccessDeniedprivate-data"); + assertFalse(error.getMessage().contains("private-data")); + } + @Test public void successResponseIsUntouched() { + SdkHttpClient delegate = mock(SdkHttpClient.class); + ImmutableResponse response = mock(ImmutableResponse.class); + when(response.getStatus()).thenReturn(HttpStatus.OK); when(delegate.apply(null)).thenReturn(response); + assertSame(response,new SafeS3HttpClient(delegate).apply(null)); + verify(response,never()).getResponseBody(); + } + @SuppressWarnings("unchecked") + @Test public void factorySetsOnlyPerConnectionTimeouts() throws Exception { + SdkHttpClient client = SafeS3HttpClient.create(); + Field delegate = SafeS3HttpClient.class.getDeclaredField("delegate"); delegate.setAccessible(true); + Field customizer = URLConnectionSdkHttpClient.class.getDeclaredField("customizer"); customizer.setAccessible(true); + HttpsURLConnection connection = mock(HttpsURLConnection.class); + ((Consumer)customizer.get(delegate.get(client))).accept(connection); + verify(connection).setConnectTimeout(5000); verify(connection).setReadTimeout(10000); + verifyNoMoreInteractions(connection); + } +} diff --git a/docs/r2-reliability.md b/docs/r2-reliability.md index 108d0d1cb..4dcede6a1 100644 --- a/docs/r2-reliability.md +++ b/docs/r2-reliability.md @@ -1,6 +1,7 @@ # S3互換ストレージの障害耐性（Issue #8） - 通信は接続5秒・読取り10秒。ストレージ専用クライアントの設定で、TLS検証やJVM全体の設定は変えない。 +- エラー応答はJAXBの古いコード生成経路を使わず、Java標準XMLパーサーでコードだけ読む。DTD/外部エンティティは禁止、本文は64KiB上限、本文をログに転記しない。404のNoSuchKeyだけを正常な欠落とする。 - `UncheckedIOException`、InternalError、ServiceUnavailable、SlowDown、RequestTimeoutは最大3試行（待機250ms/1000ms）。認証エラーや404は即返す。これは通信全体・書込み・プール待ちの厳密な総時間制限ではない。 - JSONはファイル名ごとに最新内容へ集約し、失敗後も他ファイルを処理する。再試行は有限バッチで5/10/20/40/60秒と間隔を伸ばし、60秒を上限に保持する。古い失敗内容が新しい内容や削除指示を上書きしない。 - 本描画の書込み失敗は通常更新の無効タイルとして残す。更新通知・updated統計は成功時だけ。全域描画の走査完了だけを保存完了とみなさず、通常更新キューも確認する。 From 0659b4c6e2783e6b35c21fa6000b8477531cf8c2 Mon Sep 17 00:00:00 2001 From: Gui <51219838+gui-ace@users.noreply.github.com> Date: Wed, 9 Sep 2026 16:53:41 +0900 Subject: [PATCH 3/3] =?UTF-8?q?=E5=86=8D=E8=B5=B7=E5=8B=95=E6=99=82?= =?UTF-8?q?=E3=81=AB=E5=87=A6=E7=90=86=E4=B8=AD=E3=81=AE=E3=82=BA=E3=83=BC?= =?UTF-8?q?=E3=83=A0=E6=9B=B4=E6=96=B0=E3=82=82=E4=BF=9D=E6=8C=81=E3=81=99?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/main/java/org/dynmap/MapTypeState.java | 11 +++++++++++ .../java/org/dynmap/ZoomStorageFailureTest.java | 14 ++++++++++++++ docs/r2-reliability.md | 1 + 3 files changed, 26 insertions(+) diff --git a/DynmapCore/src/main/java/org/dynmap/MapTypeState.java b/DynmapCore/src/main/java/org/dynmap/MapTypeState.java index 1d6ee2200..823abc382 100644 --- a/DynmapCore/src/main/java/org/dynmap/MapTypeState.java +++ b/DynmapCore/src/main/java/org/dynmap/MapTypeState.java @@ -182,6 +182,17 @@ public void restoreZoomOut(List> dat) { } zoomOutInvAccum.set(i, tf); } + // A shutdown can interrupt the active pass. The next startZoomOutIter() + // replaces that pass with the accumulator, so retain both halves there. + for (int i = 0; i < zoomOutInv.size(); i++) { + TileFlags active = zoomOutInv.get(i); + if (active != null) { + TileFlags accumulated = zoomOutInvAccum.get(i); + if (accumulated == null) zoomOutInvAccum.set(i, active); + else accumulated.union(active); + zoomOutInv.set(i, null); + } + } } } diff --git a/DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java b/DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java index f1aaecf06..9d91c764f 100644 --- a/DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java +++ b/DynmapCore/src/test/java/org/dynmap/ZoomStorageFailureTest.java @@ -35,6 +35,20 @@ public class ZoomStorageFailureTest { state.setZoomOutInv(0,0,0); } @After public void cleanup() { MapManager.mapman = previous; } + @Test public void restartRetainsBothActiveAndAccumulatedZoomWork() { + state.startZoomOutIter(); + state.setZoomOutInv(2,2,0); + MapTypeState restored = new MapTypeState(world,state.type); + restored.restoreZoomOut(state.saveZoomOut()); + restored.startZoomOutIter(); + MapTypeState.ZoomOutCoord coordinate = new MapTypeState.ZoomOutCoord(); + int count = 0; + while (restored.nextZoomOutInv(coordinate)) { + restored.clearZoomOutInv(coordinate.x,coordinate.y,coordinate.zoomlevel); + assertTrue(++count <= 2); + } + assertEquals(2,count); + } @Test public void failedReadPreservesParentAndRetriesNextPass() { when(child.read()).thenThrow(new StorageReadException(new IOException())); world.freshenZoomOutFiles(); diff --git a/docs/r2-reliability.md b/docs/r2-reliability.md index 4dcede6a1..82958bdfd 100644 --- a/docs/r2-reliability.md +++ b/docs/r2-reliability.md @@ -6,6 +6,7 @@ - JSONはファイル名ごとに最新内容へ集約し、失敗後も他ファイルを処理する。再試行は有限バッチで5/10/20/40/60秒と間隔を伸ばし、60秒を上限に保持する。古い失敗内容が新しい内容や削除指示を上書きしない。 - 本描画の書込み失敗は通常更新の無効タイルとして残す。更新通知・updated統計は成功時だけ。全域描画の走査完了だけを保存完了とみなさず、通常更新キューも確認する。 - ズームの読取り障害は欠落と区別し、既存画像を消さない。読取り/書込み/削除失敗は次のズーム周期へ残す。正常に存在しないタイルは従来どおり空白として扱う。 +- 再起動時は保存済みの処理中・待機中の両方のズーム更新を統合して復元する。最初の周期切替で処理中だった更新を捨てない。 - 起動時Web資材は描画executorで非同期公開する。プロセス初回はR2の実内容をGETして一致した資材のPUTを省くため、削除済み資材は再作成できる。版番号差分も内容比較の対象。GETと転送量は増えるが、同値PUTは減る。初回のWeb表示は資材公開完了まで待つ必要がある。 - ライブJSONにGET確認や長期読取りキャッシュを追加しない。同値抑制用のローカルdigestは有効長だけを対象とし、確認成功後だけ確定する。