[GH-3186] Cache libpostal model data in CI - #3187
Merged
Merged
Conversation
AddressProcessingFunctionsTest downloads ~1.3 GB of libpostal model data on every job, in all six matrix combinations, with no retry. A transient stall in DataDownloadUtils (15 s connect timeout, no retry) fails the whole build. - Cache /tmp/libpostal. jpostal skips the download when the directory is populated, so a warm cache means no network access at all. - Pre-fetch the archives in a workflow step using curl --retry, so a cold cache is slow rather than fatal. - Run the suite in one matrix combination and gate the cache and pre-fetch steps on the same key, so only that job pays the data cost. The SEDONA_SKIP_LIBPOSTAL_TESTS variable is opt-out: unset, the tests run as before. Also drops clearLibpostalDataDir/beforeEach, which recursively deleted the data directory before the first test. Nothing asserts on it, and it would have deleted the restored cache.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Did you read the Contributor Guide?
Is this PR related to a ticket?
[GH-XXX] my subject. Closes Flaky CI: libpostal model data download times out in the Scala and Java build #3186What changes were proposed in this PR?
AddressProcessingFunctionsTestdownloads ~1.3 GB of libpostal model data on every job, in all six matrix combinations, with no retry. When the object store stalls, the whole build fails — for example run 30229831581 onmaster:Three changes in
java.ymland the test:Cache
/tmp/libpostal. jpostal already skips the download when the directory is populated —LibPostal.getInstanceonly callspopulateDataDirwhendownloadDataIfNeeded && !isDataDirPopulated(dataDir). The cache key is a constant rather than ahashFiles, because the data version is pinned by the jpostal dependency and theuseSenzingdefault rather than by anything in the tree; the key names both so it is bumped deliberately when they change.Pre-fetch the archives in a workflow step using
curl --retry.DataDownloadUtils.downloadFileuses a 15 s connect timeout and no retry, so today a transient stall inside the test JVM fails the build. Moving the fetch into a step with retries makes a cold cache slow rather than fatal.Run the suite in one matrix combination (Spark 3.5.8 / Scala 2.12.15) instead of all six, gated on
SEDONA_SKIP_LIBPOSTAL_TESTS. The cache and pre-fetch steps are gated on the same matrix key, so only that job pays the data cost. The variable is opt-out: unset — as it is for local runs and for anyone building outside this workflow — the tests run exactly as before.This also required removing
clearLibpostalDataDir/beforeEachfrom the suite, which recursively deletedspark.sedona.libpostal.dataDirbefore the first test. Nothing asserts on it; it only forced the download path to be exercised, and it would have deleted the restored cache and re-triggered the download that fails here.Net effect: libpostal traffic goes from roughly 8 GB per CI run to zero on a warm cache, and a cold cache no longer fails the build on a single timeout.
Cache cost
Worth a reviewer's attention: the repo currently holds ~14.5 GB of active caches against GitHub's 10 GB limit, so it is already evicting. This adds one entry of roughly 1.3 GB.
In steady state it stays at one entry. The key is a constant, so once
masterhas it, PR runs restore it from the default branch as an exact hit and do not re-save; only one of the six matrix jobs has the cache step at all. The exception is the window beforemasterhas the entry — during that time a PR that touchesjava.ymlcreates its ownrefs/pull/N/merge-scoped copy, which is the same behaviour the existing~/.m2cache already has here.If that cost is judged not worth it, the pre-fetch step alone fixes the reported failure at no cache cost, and the cache step can be dropped from this PR.
How was this patch tested?
Existing
AddressProcessingFunctionsTestcoverage is unchanged in the combination that runs it. Verified locally:mvn test-compile -pl spark/common -am -Dspark=3.5 -Dscala=2.12mvn spotless:apply -pl spark/commonproduces no changespre-commit run --files .github/workflows/java.yml spark/common/src/test/scala/org/apache/sedona/sql/AddressProcessingFunctionsTest.scalapasses, includingactionlintandyamllintDataDownloadUtils.isDataDirPopulated, and match the contents of the three archives.The cache path itself is exercised by this PR's own CI run: the first run populates the cache, and any subsequent run on the branch should skip the download.
Did this PR include necessary documentation updates?