[BUGFIX] Download topography once in the tests, and fail fast when the GMT server is down

.github/workflows/CI.yml

@@ -20,6 +20,9 @@ jobs:
   run_tests:
     name: Julia ${{ matrix.version }} - ${{ matrix.os }} - ${{ matrix.arch }} - ${{ github.event_name }}
     runs-on: ${{ matrix.os }}
+    # A normal run takes 10-30 minutes. This is a backstop in case a download
+    # hangs after all, so that a job cannot occupy a runner for hours.
+    timeout-minutes: 90
     continue-on-error: ${{ matrix.allow_failure }}
     strategy:
       fail-fast: false
@@ -71,27 +74,44 @@ jobs:
           include-matrix: false
       - uses: julia-actions/julia-buildpkg@latest
       # Cache the topography tiles that the tests download (see
-      # test/topo_prefetch.jl), so PR runs do not depend on the GMT data
-      # server. The scheduled run skips the cache on purpose, so that the
-      # download itself is still tested regularly.
+      # test/topo_prefetch.jl), so push/PR runs do not depend on the GMT data
+      # server. The key changes with the prefetch list; `restore-keys` then
+      # still restores the previous tiles, so only the new ones are fetched
+      # rather than every job of the matrix fetching everything at once (the
+      # server throttles that, and a throttled job used to run for two hours).
       #
       # `import_topo` keeps the tiles in its own scratch space rather than in
       # ~/.gmt, which is where GMT used to put them; the UUID is the package's.
       # ~/.gmt is still cached for import_GeoTIFF, which does go through GMT.
+      # The decoded .raw files are left out: they are rebuilt from the tiles.
       - uses: actions/cache/restore@v4
         id: gmt-cache
-        if: github.event_name != 'schedule'
         with:
           path: |
             ~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles
+            !~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles/*.raw
+            !~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles/*.part
+            !~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles/server_unreachable_until
             ~/.gmt
           key: gmt-topo-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('test/topo_prefetch.jl') }}
+          restore-keys: |
+            gmt-topo-${{ runner.os }}-${{ runner.arch }}-
+      # The scheduled run tests the download itself: clear the tiles, whichever
+      # cache they came from (julia-actions/cache restores ~/.julia, scratch
+      # spaces included), so that test/topo_prefetch.jl has to fetch them.
+      - name: Clear topography cache (scheduled run)
+        if: github.event_name == 'schedule'
+        shell: bash
+        run: rm -rf ~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles
       - uses: julia-actions/julia-runtest@latest
       - uses: actions/cache/save@v4
-        if: always() && github.event_name != 'schedule' && steps.gmt-cache.outputs.cache-hit != 'true'
+        if: always() && steps.gmt-cache.outputs.cache-hit != 'true'
         with:
           path: |
             ~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles
+            !~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles/*.raw
+            !~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles/*.part
+            !~/.julia/scratchspaces/3700c31b-fa53-48a6-808a-ef22d5a84742/topo_tiles/server_unreachable_until
             ~/.gmt
           key: gmt-topo-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('test/topo_prefetch.jl') }}
       - uses: julia-actions/julia-processcoverage@v1

src/import_topo.jl

@@ -27,6 +27,20 @@ const GMT_SERVERS = [
     "https://sa.generic-mapping-tools.org/server/earth",
 ]
 
+"""
+How long to wait for the server's index file, which is a hundred kilobytes: if that does not
+arrive within a few seconds the mirror is not going to deliver the tiles either.
+"""
+const INDEX_TIMEOUT = 15.0
+
+"""
+Once every mirror has failed to deliver a file, how long (seconds) to leave the server alone
+before asking it again. Without this, every later call in the session -- and in every other
+test worker, hence the marker file rather than a variable -- waits out the full retry budget
+too, and a run that should fail in a minute takes hours.
+"""
+const SERVER_RETRY_AFTER = 600.0
+
 """
     tile_size_deg(res)
 
@@ -66,25 +80,73 @@ function server_entry(dataset::AbstractString, res::AbstractString, reg::Abstrac
     return nothing
 end
 
+"""
+The copy of the server's index that ships with the package, for when the server cannot be
+asked for the current one. It changes rarely (new sets are added now and then; the existing
+lines stay), so a stale copy is far better than none: without it a tiled resolution would be
+looked for as a single global file, which does not exist.
+"""
+const BUNDLED_SERVER_INDEX = joinpath(@__DIR__, "..", "assets", "gmt_data_server.txt")
+
+# whether this session has already tried to download the index
+const SERVER_INDEX_TRIED = Ref(false)
+
 """
     server_index_file()
 
-The server's own index of its datasets, downloaded once and cached.
+The server's own index of its datasets: downloaded once into the cache and read from there
+after that. If it cannot be downloaded, the copy bundled with the package is used instead.
+The download is attempted once per session, not once per call: the index is consulted
+several times per import, and with the server down each attempt would cost a full round of
+the mirrors.
 """
 function server_index_file()
     index = joinpath(topo_cache_dir(), "gmt_data_server.txt")
     (isfile(index) && filesize(index) > 0) && return index
-    for server in GMT_SERVERS
-        root = replace(server, "/server/earth" => "")
-        try
-            Downloads.download("$root/gmt_data_server.txt", index; timeout = TILE_TIMEOUT)
-            return index
-        catch
+    if !SERVER_INDEX_TRIED[] && server_down_for() == 0
+        SERVER_INDEX_TRIED[] = true
+        for server in GMT_SERVERS
+            root = replace(server, "/server/earth" => "")
+            try
+                Downloads.download("$root/gmt_data_server.txt", index; timeout = INDEX_TIMEOUT)
+                return index
+            catch
+                rm(index; force = true)               # no half-written index
+            end
         end
     end
-    return nothing
+    return BUNDLED_SERVER_INDEX
 end
 
+"""
+    server_down_for()
+
+How many seconds are left of leaving the server alone after every mirror failed to deliver a
+file, or `0.0` if it may be asked. The marker is a file in the cache rather than a variable
+so that separate processes -- the parallel test workers -- share it.
+"""
+function server_down_for()
+    marker = joinpath(topo_cache_dir(), "server_unreachable_until")
+    isfile(marker) || return 0.0
+    until = something(tryparse(Float64, read(marker, String)), 0.0)
+    remaining = until - time()
+    remaining > 0 && return remaining
+    rm(marker; force = true)
+    return 0.0
+end
+
+function mark_server_down()
+    return write(joinpath(topo_cache_dir(), "server_unreachable_until"), string(time() + SERVER_RETRY_AFTER))
+end
+
+"""
+    reset_topo_server()
+
+Forget that the GMT data server was unreachable, so that the next `import_topo` asks it
+again right away rather than after `SERVER_RETRY_AFTER` seconds.
+"""
+reset_topo_server() = rm(joinpath(topo_cache_dir(), "server_unreachable_until"); force = true)
+
 """
     tile_size_px(res, deg, reg)
 
@@ -172,8 +234,7 @@ asking for the other one would 404 on every tile.
 """
 function native_registration(dataset::AbstractString, res::AbstractString, reg::AbstractString)
     res in ("03s", "01s") && return "g"
-    index = joinpath(topo_cache_dir(), "gmt_data_server.txt")
-    isfile(index) || return reg
+    index = server_index_file()
     have = String[]
     for line in eachline(index)
         startswith(line, "#") && continue
@@ -242,6 +303,9 @@ function download_tile(relative_url::AbstractString, dest::AbstractString;
                       maxattempts::Integer = 5, timeout::Real = TILE_TIMEOUT)
     isfile(dest) && filesize(dest) > 0 && return true       # cached
 
+    wait = server_down_for()
+    wait > 0 && error("the GMT data server could not be reached a moment ago, so $(basename(dest)) was not requested; it will be tried again in $(round(Int, wait)) s. To retry now, call GeophysicalModelGenerator.reset_topo_server().")
+
     tmp = dest * ".part"                                    # never leave a half file behind
     for attempt in 1:max(1, maxattempts)
         for server in GMT_SERVERS
@@ -265,7 +329,11 @@ function download_tile(relative_url::AbstractString, dest::AbstractString;
         # is given a chance rather than hammered
         attempt < maxattempts && sleep(min(2.0^attempt, 10.0))
     end
-    return false
+    # Every mirror failed every time: the server is down, or throttling us. That is not the
+    # same as a tile that does not exist (which is filled with sea level), so it is an error
+    # -- and it is remembered, so the next call does not wait all of this out again.
+    mark_server_down()
+    return error("could not download $relative_url from any GMT data server mirror in $maxattempts attempts")
 end
 
 """

test/runtests.jl

@@ -2,8 +2,9 @@ using GeophysicalModelGenerator
 using ParallelTestRunner
 
 # Download the topography tiles the tests need *before* spawning the parallel
-# workers, so they are served from GMT's cache instead of being downloaded
-# concurrently (which times out regularly in CI). See topo_prefetch.jl.
+# workers, so they are served from the tile cache instead of being downloaded
+# concurrently (which the GMT data server throttles). This is the only place
+# that downloads topography; see topo_prefetch.jl.
 include("topo_prefetch.jl")
 
 include("test_import_topo.jl")

test/test_GMT.jl

@@ -1,11 +1,21 @@
 using Test
 using GeophysicalModelGenerator, GMT
 
-Topo = import_topo(lon = [8, 9], lat = [50, 51])
-@test sum(Topo.depth.val) ≈ 1076.7045 rtol = 1e-1
+# The topography comes from the tile cache that topo_prefetch.jl filled. If the prefetch could
+# not reach the GMT data server, asking again here is pointless: skip on a push/PR run, insist
+# on the scheduled run so a real breakage is caught.
+topo_server_ok = get(ENV, "GMG_TOPO_PREFETCH_OK", "true") == "true" ||
+    get(ENV, "GITHUB_EVENT_NAME", "") == "schedule"
 
-Topo = import_topo([8, 9, 50, 51]);
-@test sum(Topo.depth.val) ≈ 1076.7045 rtol = 1e-1
+if !topo_server_ok
+    @test_skip "GMT data server unreachable: topography tests skipped"
+else
+    Topo = import_topo(lon = [8, 9], lat = [50, 51])
+    @test sum(Topo.depth.val) ≈ 1076.7045 rtol = 1.0e-1
+
+    Topo = import_topo([8, 9, 50, 51])
+    @test sum(Topo.depth.val) ≈ 1076.7045 rtol = 1.0e-1
+end
 
 test_fwd = import_GeoTIFF("test_files/length_fwd.tif", fieldname = :forward)
 @test  maximum(test_fwd.fields.forward) ≈ 33.17775km

test/test_WaterFlow.jl

@@ -1,19 +1,28 @@
 using Test, GMT
 
+# The topography comes from the tile cache that topo_prefetch.jl filled. If the prefetch could
+# not reach the GMT data server, asking again here is pointless: skip on a push/PR run, insist
+# on the scheduled run so a real breakage is caught.
+topo_server_ok = get(ENV, "GMG_TOPO_PREFETCH_OK", "true") == "true" ||
+    get(ENV, "GITHUB_EVENT_NAME", "") == "schedule"
 
-# Download some topographic data
-Topo = import_topo([6.5, 7.3, 50.2, 50.6], file = "@earth_relief_03s");
+if !topo_server_ok
+    @test_skip "GMT data server unreachable: water flow tests skipped"
+else
+    # Some topographic data
+    Topo = import_topo([6.5, 7.3, 50.2, 50.6], file = "@earth_relief_03s")
 
-# Flow the water through the area:
-Topo_water, sinks, pits, bnds = waterflows(Topo)
+    # Flow the water through the area:
+    Topo_water, sinks, pits, bnds = waterflows(Topo)
 
-@test maximum(Topo_water.fields.area) ≈ 9.309204547276944e8
-@test sum(Topo_water.fields.c) == 834501044
-@test sum(Topo_water.fields.nin) == 459361
-@test sum(Topo_water.fields.dir) == 2412566
+    @test maximum(Topo_water.fields.area) ≈ 9.309204547276944e8
+    @test sum(Topo_water.fields.c) == 834501044
+    @test sum(Topo_water.fields.nin) == 459361
+    @test sum(Topo_water.fields.dir) == 2412566
 
-# With rain in m3/s per cell
-rainfall = ones(size(Topo.lon.val[:, :, 1])) * 1.0e-3 # 2D array with rainfall per cell area
-Topo_water1, sinks, pits, bnds = waterflows(Topo, rainfall = rainfall)
+    # With rain in m3/s per cell
+    rainfall = ones(size(Topo.lon.val[:, :, 1])) * 1.0e-3 # 2D array with rainfall per cell area
+    Topo_water1, sinks, pits, bnds = waterflows(Topo, rainfall = rainfall)
 
-@test maximum(Topo_water1.fields.area) ≈ 169.79800000000208
+    @test maximum(Topo_water1.fields.area) ≈ 169.79800000000208
+end