Yicong-Huang commented on code in PR #8083:
URL: https://github.com/apache/texera/pull/8083#discussion_r3885550795
##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/coordinator/execution/OperatorExecution.scala:
##########
@@ -52,7 +52,7 @@ case class OperatorExecution() {
*/
def initWorkerExecution(workerId: ActorVirtualIdentity): WorkerExecution = {
assert(
- !workerExecutions.contains(workerId),
+ !workerExecutions.containsKey(workerId),
Review Comment:
Optional, and not a defect report — no production path issues concurrent
`initWorkerExecution` calls for one `OperatorExecution`, so today's behavior is
identical either way.
The guard is a two-step check-then-act on a concurrent container
(`containsKey`, then `put`), followed by a third map operation:
`getWorkerExecution(workerId)` re-reads the value just inserted rather than
returning the instance in hand. `putIfAbsent` collapses all three into one
atomic call.
It is also the shape both peers already use — `RegionExecution` and
`WorkflowExecution` insert via `getOrElseUpdate`, so their guards cannot
interleave by construction. Worth considering on a method whose whole purpose
in this PR became enforcing that invariant: it would hold structurally rather
than by caller discipline.
--
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]
