aglinxinyuan opened a new pull request, #7092:
URL: https://github.com/apache/texera/pull/7092
### What changes were proposed in this PR?
**Root cause.** `AsterixDBConnUtil.asterixDBVersionMapping` is a
process-global `host -> version` cache backed by an unsynchronized
`scala.collection.mutable.Map` (a `HashMap`), but it is written from
`updateAsterixDBVersionMapping`, reached from `queryAsterixDB` on the first
query to each host. Several `AsterixDBSourceOpExec` workers can initialize
concurrently in one JVM, so two threads can `+=` at the same time. Concurrent
insertion into an unsynchronized `HashMap` can corrupt the table during a
resize — surfacing as a lost entry, a spin, or a spurious
`NoSuchElementException` from the read at `asterixDBVersionMapping(host)`.
**Fix.** Back it with `scala.collection.concurrent.TrieMap`, which is in the
Scala standard library — no new dependency.
```diff
+import scala.collection.concurrent.TrieMap
import scala.collection.mutable
-import scala.collection.mutable.Map
- var asterixDBVersionMapping: Map[String, String] = Map()
+ val asterixDBVersionMapping: mutable.Map[String, String] = TrieMap.empty
```
| | before | after |
| --- | --- | --- |
| concurrent `+=` | can corrupt the table | safe |
| `contains` then update | benign race — both threads fetch the same version
for the same host and write the same value | unchanged, still benign |
| iteration | never iterated | unchanged (`TrieMap`'s snapshot iterators are
not reachable here) |
| declaration | `var` | `val` — the map was only ever mutated in place,
never reassigned |
Two incidental tidies that fall out of the same edit:
- Dropping `import scala.collection.mutable.Map` un-shadows `Predef.Map` for
the whole file, so `fetchDataTypeFields`'s return type no longer needs to be
written as `Predef.Map[String, String]` to mean the ordinary immutable `Map`.
- `var` → `val`: every write is an in-place `+=` / `-=`, in both the source
and the specs, so nothing reassigned the reference.
`AsterixDBConnUtilSpec` and `AsterixDBSourceOpExecSpec` manipulate this map
directly with `+=` and `-=`; both are in-place operations on `mutable.Map`, so
both specs compile and pass unchanged.
### Any related issues, documentation, discussions?
Closes #7091
### How was this PR tested?
Existing tests only — this PR adds none. The defect is a data race whose
deterministic reproduction would need thread-interleaving instrumentation the
module has no harness for; the change is a type substitution whose safety
property is provided by the standard library.
Locally, from the repo root with Java 17:
- `sbt "scalafixAll --check"` — clean.
- `sbt scalafmtCheckAll` — clean.
- `sbt "WorkflowOperator/testOnly *AsterixDB*"` — 84 tests, all pass, both
specs unmodified.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude 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]
