You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/07/22 01:23:32 UTC
(texera) branch gh-readonly-queue/main/pr-6530-3ae7dcca3929848e75e15f2cd97f07cdd47bd80b created (now faca7bac76)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-6530-3ae7dcca3929848e75e15f2cd97f07cdd47bd80b
in repository https://gitbox.apache.org/repos/asf/texera.git
at faca7bac76 test(pyamber): guard executor module isolation across two MainLoops (#6530)
This branch includes the following new commits:
new faca7bac76 test(pyamber): guard executor module isolation across two MainLoops (#6530)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(texera) 01/01: test(pyamber): guard executor module isolation across two MainLoops (#6530)
Posted by gi...@apache.org.
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-3ae7dcca3929848e75e15f2cd97f07cdd47bd80b
in repository https://gitbox.apache.org/repos/asf/texera.git
commit faca7bac76f307de1f955a429cd243ff2f89be26
Author: Xinyuan Lin <xi...@uci.edu>
AuthorDate: Tue Jul 21 18:19:17 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 <xi...@uci.edu>
Co-authored-by: Copilot Autofix powered by AI <17...@users.noreply.github.com>
---
.../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()