This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7705-51f38e13d66e42070872ccb532b7796b82273be4 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 80913ed92a4be5d198eb3d5eeaf80bcdc198bad3 Author: Xinyuan Lin <[email protected]> AuthorDate: Wed Aug 19 06:13:08 2026 +0000 chore(amber): remove the unused DeployStrategy family (#7705) ### What changes were proposed in this PR? Deletes the `deploystrategy` package — the `DeployStrategy` trait, its three implementations (`OneOnEach`, `RandomDeployment`, `RoundRobinDeployment`) and their shared spec. Pure deletion, no behaviour change: **−293 lines**. Nothing has called any of them since #1807 (2023-02-02, "[Compiler Refactor 6] Refactor Amber Workflow to use the new PhysicalPlan implementation"). Searching both by symbol name and by package path, the only files that reference the four types are the package's own files and its spec. Worker placement is now inline. `ExecutorDeployment.createWorkers` picks an address straight off `AddressInfo`: ```scala addressInfo.allAddresses(workerIndex % addressInfo.allAddresses.length) ``` — `RoundRobinDeployment`'s logic absorbed into the caller. `Coordinator` and `RegionExecutionManager` reach placement through that path and never touch the trait. > Reviewer note — why this looks alive. The package got two fixes in May 2026, #5028 (reset the iteration cursor in `OneOnEach.initialize`) and #5029 (unify the empty-array errors), plus the unit-test suite from #4723. All three came from starter-task and coverage issues (#4731, #4732, #4722) that polish the code where it sits; none wires it to a caller. Every related issue and PR is closed and nothing open proposes using the trait — but if a worker-placement change is planned that wants this abstraction back, say so and I will close this. > `AddressInfo` lives one package up in `deploysemantics` and is **not** touched — it is what the live path reads. ### Any related issues, documentation, discussions? Closes #7704 ### How was this PR tested? Existing tests only — this PR adds none, since it removes code and the spec that covered it. Locally, from the repo root with Java 17: - `sbt "WorkflowExecutionService/Test/compile"` — success (main and test sources). Verification, re-runnable by a reviewer: ``` git grep -w DeployStrategy OneOnEach RandomDeployment RoundRobinDeployment # only the deleted files git grep -l deploystrategy # only the deleted files ``` > CI note: the build jobs currently fail repo-wide at workflow startup — an org policy blocks the injected `carabiner-dev/actions/install/ampel` action ("not allowed in apache/texera"). `main` fails identically, so it is unrelated to this change; same class as #6989 and #7572. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) --- .../deploystrategy/DeployStrategy.scala | 30 ----- .../deploysemantics/deploystrategy/OneOnEach.scala | 45 ------- .../deploystrategy/RandomDeployment.scala | 41 ------- .../deploystrategy/RoundRobinDeployment.scala | 44 ------- .../deploystrategy/DeployStrategiesSpec.scala | 133 --------------------- 5 files changed, 293 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategy.scala b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategy.scala deleted file mode 100644 index 079d253c84..0000000000 --- a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategy.scala +++ /dev/null @@ -1,30 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.texera.amber.engine.architecture.deploysemantics.deploystrategy - -import org.apache.pekko.actor.Address - -trait DeployStrategy extends Serializable { - - def initialize(available: Array[Address]): Unit - - def next(): Address - -} diff --git a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/OneOnEach.scala b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/OneOnEach.scala deleted file mode 100644 index f35a134811..0000000000 --- a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/OneOnEach.scala +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.texera.amber.engine.architecture.deploysemantics.deploystrategy - -import org.apache.pekko.actor.Address - -object OneOnEach { - def apply() = new OneOnEach() -} - -class OneOnEach extends DeployStrategy { - var available: Array[Address] = _ - var index = 0 - - override def initialize(available: Array[Address]): Unit = { - this.available = available - this.index = 0 - } - - override def next(): Address = { - val i = index - if (i >= available.length) { - throw new NoSuchElementException("no available addresses") - } - index += 1 - available(i) - } -} diff --git a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RandomDeployment.scala b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RandomDeployment.scala deleted file mode 100644 index 5e3ebfa972..0000000000 --- a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RandomDeployment.scala +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.texera.amber.engine.architecture.deploysemantics.deploystrategy - -import org.apache.pekko.actor.Address - -object RandomDeployment { - def apply() = new RandomDeployment() -} - -class RandomDeployment extends DeployStrategy { - var available: Array[Address] = _ - - override def initialize(available: Array[Address]): Unit = { - this.available = available - } - - override def next(): Address = { - if (available.isEmpty) { - throw new NoSuchElementException("no available addresses") - } - available(util.Random.nextInt(available.length)) - } -} diff --git a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RoundRobinDeployment.scala b/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RoundRobinDeployment.scala deleted file mode 100644 index 7047297493..0000000000 --- a/amber/src/main/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/RoundRobinDeployment.scala +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.texera.amber.engine.architecture.deploysemantics.deploystrategy - -import org.apache.pekko.actor.Address - -object RoundRobinDeployment { - def apply() = new RoundRobinDeployment() -} - -class RoundRobinDeployment extends DeployStrategy { - var available: Array[Address] = _ - var index = 0 - - override def initialize(available: Array[Address]): Unit = { - this.available = available - } - - override def next(): Address = { - if (available.isEmpty) { - throw new NoSuchElementException("no available addresses") - } - val i = index - index = (index + 1) % available.length - available(i) - } -} diff --git a/amber/src/test/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategiesSpec.scala b/amber/src/test/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategiesSpec.scala deleted file mode 100644 index 568f58e36f..0000000000 --- a/amber/src/test/scala/org/apache/texera/amber/engine/architecture/deploysemantics/deploystrategy/DeployStrategiesSpec.scala +++ /dev/null @@ -1,133 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.texera.amber.engine.architecture.deploysemantics.deploystrategy - -import org.apache.pekko.actor.Address -import org.scalatest.flatspec.AnyFlatSpec -import org.scalatest.matchers.should.Matchers - -class DeployStrategiesSpec extends AnyFlatSpec with Matchers { - - // Use the "pekko" protocol to match Amber's real node addresses - // (e.g. AmberConfig.masterNodeAddr); "akka" diverges from production and - // can mislead anyone who debugs a failure by comparing addresses. - private val nodeA = Address("pekko", "sys", "host-a", 2552) - private val nodeB = Address("pekko", "sys", "host-b", 2552) - private val nodeC = Address("pekko", "sys", "host-c", 2552) - - // ----- OneOnEach ----- - - "OneOnEach" should "hand out each address exactly once in array order" in { - val strategy = OneOnEach() - strategy.initialize(Array(nodeA, nodeB, nodeC)) - strategy.next() shouldBe nodeA - strategy.next() shouldBe nodeB - strategy.next() shouldBe nodeC - } - - it should "raise NoSuchElementException once the array is exhausted" in { - val strategy = OneOnEach() - strategy.initialize(Array(nodeA)) - strategy.next() shouldBe nodeA - assertThrows[NoSuchElementException](strategy.next()) - } - - it should "raise NoSuchElementException immediately when initialized with an empty array" in { - val strategy = OneOnEach() - strategy.initialize(Array.empty[Address]) - assertThrows[NoSuchElementException](strategy.next()) - } - - it should "reset its iteration cursor on re-initialization" in { - val strategy = OneOnEach() - strategy.initialize(Array(nodeA, nodeB)) - strategy.next() shouldBe nodeA - strategy.next() shouldBe nodeB - strategy.initialize(Array(nodeC)) - strategy.next() shouldBe nodeC - assertThrows[NoSuchElementException](strategy.next()) - } - - "OneOnEach.apply" should "produce a fresh, independent instance" in { - val s1 = OneOnEach() - val s2 = OneOnEach() - s1 should not be theSameInstanceAs(s2) - } - - // ----- RoundRobinDeployment ----- - - "RoundRobinDeployment" should "rotate addresses in a repeating cycle" in { - val strategy = RoundRobinDeployment() - strategy.initialize(Array(nodeA, nodeB, nodeC)) - strategy.next() shouldBe nodeA - strategy.next() shouldBe nodeB - strategy.next() shouldBe nodeC - strategy.next() shouldBe nodeA - strategy.next() shouldBe nodeB - } - - it should "always return the only address when the array has length 1" in { - val strategy = RoundRobinDeployment() - strategy.initialize(Array(nodeA)) - for (_ <- 1 to 5) strategy.next() shouldBe nodeA - } - - it should "raise NoSuchElementException on next() with an empty array" in { - val strategy = RoundRobinDeployment() - strategy.initialize(Array.empty[Address]) - assertThrows[NoSuchElementException](strategy.next()) - } - - "RoundRobinDeployment.apply" should "produce a fresh, independent instance" in { - val s1 = RoundRobinDeployment() - val s2 = RoundRobinDeployment() - s1 should not be theSameInstanceAs(s2) - } - - // ----- RandomDeployment ----- - - "RandomDeployment" should "always return one of the available addresses" in { - val strategy = RandomDeployment() - val pool = Array(nodeA, nodeB, nodeC) - strategy.initialize(pool) - val poolSet = pool.toSet - for (_ <- 1 to 50) { - poolSet should contain(strategy.next()) - } - } - - it should "always return the only address when the array has length 1" in { - val strategy = RandomDeployment() - strategy.initialize(Array(nodeA)) - for (_ <- 1 to 5) strategy.next() shouldBe nodeA - } - - it should "raise NoSuchElementException on next() with an empty array" in { - val strategy = RandomDeployment() - strategy.initialize(Array.empty[Address]) - assertThrows[NoSuchElementException](strategy.next()) - } - - "RandomDeployment.apply" should "produce a fresh, independent instance" in { - val s1 = RandomDeployment() - val s2 = RandomDeployment() - s1 should not be theSameInstanceAs(s2) - } -}
