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-6530-fb3f3aef4a3239bf3f1d9d6afcc1dc1b9835b78e in repository https://gitbox.apache.org/repos/asf/texera.git
commit dff1f5ce02fd6c28baa41732c49cddc6db0bbe95 Author: Xinyuan Lin <[email protected]> AuthorDate: Tue Jul 21 17:20:24 2026 -0700 test(pyamber): guard executor module isolation across two MainLoops (#6530) ### What changes were proposed in this PR? Adds a regression test, `test_two_main_loops_load_distinct_operator_classes`, in `amber/src/test/python/core/runnables/test_main_loop.py`. No source changes. It creates two `MainLoop`s in the same process, initializes the first with `EchoOperator` and deliberately leaves that worker "unfinished" (its temp fs is never closed, like a crashed/never-completed worker), then initializes the second with `CountBatchOperator` and asserts the second executor is exactly `CountBatchOperator`. This guards `ExecutorManager`'s process-globally-unique module naming end to end. Executor modules were historically named `udf-v<per-instance-counter>`, so every loop's first executor was `udf-v1` in the process-wide `sys.modules`; a loop whose worker never completed left a stale `udf-v1.py` on `sys.path`, and the next loop re-resolved `udf-v1` to that older file — silently running the wrong operator. That collision was fixed by switching to a process-wide counter, but the suite had no end-to-end test spanning two `MainLoop`s, so a regression back to a per-instance counter would still pass CI. This closes that gap. The test uses **no** monkeypatch of `gen_module_file_name` — module names must be process-globally unique on their own. ### Any related issues, documentation, discussions? Regression coverage for the module-name collision fixed in #4717 (issue #4705). ### How was this PR tested? `cd amber && python -m pytest src/test/python/core/runnables/test_main_loop.py` — the new test passes on `main`. `ruff check` and `ruff format --check` are clean. Confirmed it is a genuine guard, not a tautology: on the pre-fix code (per-instance `udf-v<counter>` naming) the second loop resolves `udf-v1` to the first loop's file, so the second executor loads `EchoOperator` and the assertion fails — I reproduced that red state before it passed against the fixed code. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8 [1M context]) --------- Signed-off-by: Xinyuan Lin <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> --- .../test/python/core/runnables/test_main_loop.py | 63 ++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/amber/src/test/python/core/runnables/test_main_loop.py b/amber/src/test/python/core/runnables/test_main_loop.py index 5884edf27b..2bbefe8c43 100644 --- a/amber/src/test/python/core/runnables/test_main_loop.py +++ b/amber/src/test/python/core/runnables/test_main_loop.py @@ -51,6 +51,7 @@ from proto.org.apache.texera.amber.core import ( OpExecInitInfo, EmbeddedControlMessageIdentity, ) +from core.architecture.managers.context import Context from core.architecture.managers.pause_manager import PauseType from core.util.console_message.timestamp import current_time_in_local_timezone from proto.org.apache.texera.amber.engine.architecture.rpc import ( @@ -2602,3 +2603,65 @@ class TestMainLoop: assert rpc_calls == [], "must fail before the jump RPC" assert write_log == [], "must fail before touching storage" + + @pytest.mark.timeout(10) + def test_two_main_loops_load_distinct_operator_classes(self): + """ + Two worker Contexts created in the same process with DIFFERENT operator + classes must each load exactly the class they were given. + + Regression test for executor-module contamination (#4705): executor + modules were once named ``udf-v<per-instance-counter>``, so every + loop's first executor was ``udf-v1`` in the process-wide + ``sys.modules``. A loop whose worker never completes never closes its + temp fs, so its ``udf-v1.py`` lingered on ``sys.path`` and the next + loop re-resolved ``udf-v1`` to that older file, silently running the + wrong operator. This test uses NO monkeypatch of + ``gen_module_file_name`` -- module names must be process-globally + unique on their own. + + The module-naming collision lives entirely in ``ExecutorManager``, which + each ``MainLoop`` owns via its ``Context``. We construct ``Context`` + directly (rather than ``MainLoop``) so the regression is exercised + without spawning the per-loop ``DataProcessor`` daemon thread that a + full ``MainLoop`` would leave running for the rest of the test session. + """ + echo_code = "from pytexera import *\n" + inspect.getsource(EchoOperator) + count_code = "from pytexera import *\n" + inspect.getsource(CountBatchOperator) + + first = Context("worker-first", InternalQueue()) + second = Context("worker-second", InternalQueue()) + try: + # The first loop loads EchoOperator and is intentionally left "unfinished" + # until after the second loop is initialized: its temp fs is not closed yet, + # so its udf module and sys.path entry linger exactly as a crashed / + # never-completed worker's would. + first.executor_manager.initialize_executor( + echo_code, is_source=False, language="python" + ) + first_cls = type(first.executor_manager.executor).__name__ + first_module = first.executor_manager.operator_module_name + assert first_cls == "EchoOperator" + + # The second loop asks for a DIFFERENT class. It must get that + # class, not the first loop's EchoOperator via a udf module-name + # collision in the shared sys.modules / sys.path. + second.executor_manager.initialize_executor( + count_code, is_source=False, language="python" + ) + second_cls = type(second.executor_manager.executor).__name__ + second_module = second.executor_manager.operator_module_name + assert second_cls == "CountBatchOperator" + + # The module names themselves must be process-globally unique -- a + # per-instance counter would name both loops' first executor + # "udf-v1" and reintroduce the sys.modules collision. Asserting the + # names differ ties the guard directly to the root cause, not just + # the (downstream) loaded class. + assert first_module != second_module, ( + "executor module names must be process-globally unique; " + f"both loops used {first_module!r}" + ) + finally: + first.executor_manager.close() + second.executor_manager.close()
