aglinxinyuan opened a new pull request, #7100:
URL: https://github.com/apache/texera/pull/7100
### What changes were proposed in this PR?
`HDFSRecordStorage` was the only member of the record-storage family without
a spec, and the `hdfs://` arm of `SequentialRecordStorage.getStorage` was
unexercised too:
| Class | Spec before | Spec now |
|---|---|---|
| `EmptyRecordStorage` | ✅ | ✅ |
| `SequentialRecordStorage` | ✅ (`None` / `file://` arms) | ✅ + `hdfs://`
arm |
| `VFSRecordStorage` | ✅ | ✅ |
| `HDFSRecordStorage` | ❌ **none** | ✅ **new** |
`SequentialRecordStorageSpec` had explicitly given up on it — "the `hdfs://`
branch is unit-test-hostile (`HDFSRecordStorage`'s constructor calls
`FileSystem.get`, which can block on DNS / network)". It turns out to be
testable with no NameNode and no native Hadoop.
**How.** The class builds its own `Configuration` internally, so a test
cannot inject one. But Hadoop's `FileSystem` cache is keyed on `(scheme,
authority, UGI)` **only** — the `Configuration`'s contents are not part of the
key. So the spec gets there first:
```
test: FileSystem.get("hdfs://<uuid>:9000/", conf) conf: fs.hdfs.impl =
WinutilsFreeLocalFileSystem
│ └─▶ populates
FileSystem.CACHE under (hdfs, <uuid>:9000, ugi)
▼
prod: new HDFSRecordStorage(uri) ─▶ FileSystem.get(uri, new Configuration())
└─▶ cache hit: same instance,
fs.hdfs.impl never re-resolved,
no NameNode contacted
```
That is the ordinary supported `FileSystem.get` path — **no reflection
anywhere**. A unique authority per test means a unique cache key per test, so
tests cannot see each other's instance or working directory, and each closes
its `FileSystem` in a `finally` (which evicts only its own key, leaving the
JVM-global cache as found).
`WinutilsFreeLocalFileSystem` is required, not incidental: plain
`LocalFileSystem` dies at the constructor's first `mkdirs` on a winutils-less
Windows host (`RawLocalFileSystem.setPermission` → `Shell.getWinUtilsPath` →
*"Hadoop bin directory does not exist"*). The shim skips permission ops only on
that host and otherwise delegates to Hadoop's default local filesystem, so this
spec runs unchanged on Linux/macOS CI.
Covered: the scheme guard; the auto-create-folder branch; writer/reader
round-tripping through the production serde; distinct files kept independent;
re-opened writer overwriting rather than appending; the `EmptyRecordStorage`
fallback for an absent file (asserted empty, not assumed); `deleteStorage` both
present and absent; `containsFolder` for a directory vs a plain file vs nothing
(the plain-file case is what an `exists`-only implementation gets wrong); and
factory dispatch for both `hdfs://` and `HDFS://` (which pins both
`.toLowerCase` call sites).
Also in this PR: the stale paragraph in `SequentialRecordStorageSpec` is
corrected to point at the new spec. Its second claim was never true — it said
an `hdfs` regression "would surface immediately in higher-level checkpoint /
fault-tolerance suites that use hdfs:// URIs", but no suite in the repo uses an
`hdfs://` URI.
Two branches are left uncovered on purpose, noted in-file: a scheme-less URI
NPEs on `getScheme.toLowerCase` before the assert evaluates (a latent
production nit, not behaviour to cement), and the
`dfs.client…replace-datanode-on-failure` config is unobservable without a real
DataNode write pipeline.
Unlike its siblings, this spec **restores** `AmberRuntime`'s `_actorSystem`
/ `_serde` in `afterAll` instead of nulling them, and builds its `ActorSystem`
lazily. sbt runs amber's suites concurrently in one JVM and several install
their own serde into those same globals; nulling makes a sibling's
`AmberRuntime.serde` take its lazy-bootstrap branch mid-run and spin up an
untracked cluster `ActorSystem`, and a strict `val` leaks one whenever a filter
selects no test from the suite (as `AMBER_TEST_FILTER` does in the integration
job).
### Any related issues, documentation, discussions?
Closes #7098
### How was this PR tested?
One new spec, 14 tests. Run together with all three existing sibling specs
to confirm the `AmberRuntime` save/restore change does not disturb them — 43
tests total, Java 17:
```
sbt "WorkflowExecutionService/testOnly
org.apache.texera.amber.engine.common.storage.HDFSRecordStorageSpec
org.apache.texera.amber.engine.common.storage.SequentialRecordStorageSpec
org.apache.texera.amber.engine.common.storage.VFSRecordStorageSpec
org.apache.texera.amber.engine.common.storage.EmptyRecordStorageSpec"
```
```
[info] Suites: completed 4, aborted 0
[info] Tests: succeeded 43, failed 0, canceled 0, ignored 0, pending 0
[info] All tests passed.
```
Verified locally on Windows with **no `winutils.exe` and no `hadoop.dll`** —
the run logs Hadoop's own `Did not find winutils.exe` probe at WARN, which is
exactly the failure plain `LocalFileSystem` would have *thrown*.
`Test/scalafmtCheck` and `Test/scalafix --check` both `[success]`. No
production file is touched.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
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]
