jiayuasu opened a new pull request, #3187:
URL: https://github.com/apache/sedona/pull/3187

   ## Did you read the Contributor Guide?
   
   - Yes, I have read the [Contributor 
Rules](https://sedona.apache.org/latest/community/rule/) and [Contributor 
Development Guide](https://sedona.apache.org/latest/community/develop/)
   
   ## Is this PR related to a ticket?
   
   - Yes, and the PR name follows the format `[GH-XXX] my subject`. Closes #3186
   
   ## What changes were proposed in this PR?
   
   `AddressProcessingFunctionsTest` downloads ~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](https://github.com/apache/sedona/actions/runs/30229831581) on 
`master`:
   
   ```
   - should return expected normalized forms *** FAILED ***
     java.lang.RuntimeException: Failed to download or extract 
language_classifier.tar.gz
     at 
com.mapzen.jpostal.DataDownloadUtils.populateDataDir(DataDownloadUtils.java:63)
     Cause: java.net.SocketTimeoutException: connect timed out
   ```
   
   Three changes in `java.yml` and the test:
   
   1. **Cache `/tmp/libpostal`.** jpostal already skips the download when the 
directory is populated — `LibPostal.getInstance` only calls `populateDataDir` 
when `downloadDataIfNeeded && !isDataDirPopulated(dataDir)`. The cache key is a 
constant rather than a `hashFiles`, because the data version is pinned by the 
jpostal dependency and the `useSenzing` default rather than by anything in the 
tree; the key names both so it is bumped deliberately when they change.
   
   2. **Pre-fetch the archives in a workflow step using `curl --retry`.** 
`DataDownloadUtils.downloadFile` uses 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.
   
   3. **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`/`beforeEach` from the 
suite, which recursively deleted `spark.sedona.libpostal.dataDir` before 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 
`master` has 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 before `master` has the entry — during that time a PR 
that touches `java.yml` creates its own `refs/pull/N/merge`-scoped copy, which 
is the same behaviour the existing `~/.m2` cache 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 `AddressProcessingFunctionsTest` coverage is unchanged in the 
combination that runs it. Verified locally:
   
   - `mvn test-compile -pl spark/common -am -Dspark=3.5 -Dscala=2.12`
   - `mvn spotless:apply -pl spark/common` produces no changes
   - `pre-commit run --files .github/workflows/java.yml 
spark/common/src/test/scala/org/apache/sedona/sql/AddressProcessingFunctionsTest.scala`
 passes, including `actionlint` and `yamllint`
   - The five directory names the pre-fetch step checks are exactly those in 
`DataDownloadUtils.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?
   
   - No, this PR does not affect any public API so no need to change the 
documentation.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to