Skip to content

Commit 4ad5b3b

Browse files
committed
fix: avoid py4j callback-server default-port (25334) collision
PythonCallback let py4j bind the hardcoded default callback port 25334, so concurrent or repeated runs on the same host using a lambda-based Check failed with "OSError: [Errno 98] Address already in use (127.0.0.1:25334)". Force a dynamic (OS-assigned) port by setting the gateway's existing callback_server_parameters to port=0 before starting the server the stock way. Reusing PySpark's own parameters (rather than passing a fresh CallbackServerParameters + resetCallbackClient) keeps the JVM callback client correctly wired (no "Error while obtaining a new communication channel", #19) and, crucially, lets shutdown_callback_server() return cleanly at teardown on both Linux and macOS. Closes #86, #19, #7, #72, #156, #173, #198
1 parent 2e628b2 commit 4ad5b3b

2 files changed

Lines changed: 88 additions & 5 deletions

File tree

pydeequ/scala_utils.py

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,34 @@
11
# -*- coding: utf-8 -*-
22
"""A collection of utility functions and classes for manipulating with scala objects anc classes through py4j
33
"""
4-
from py4j.java_gateway import JavaObject
4+
from py4j.java_gateway import DEFAULT_PYTHON_PROXY_PORT, JavaObject
5+
6+
7+
def _ensure_dynamic_callback_port(gateway):
8+
"""Make sure the py4j callback server will NOT bind the hardcoded default
9+
port (25334) before it is started.
10+
11+
Historically PyDeequ called ``gateway.start_callback_server()`` with no
12+
arguments. With older py4j/pyspark gateways that left the callback server
13+
on the hardcoded default port ``DEFAULT_PYTHON_PROXY_PORT`` (25334), so two
14+
pyspark applications -- or two runs on the same host -- using a lambda-based
15+
``Check`` collided with
16+
``OSError: [Errno 98] Address already in use (127.0.0.1:25334)``
17+
(issues #86, #19, #7, #72, #156, #173, #198).
18+
19+
The fix is simply to ask the OS for a free port (``port = 0``). We mutate
20+
the gateway's EXISTING ``callback_server_parameters`` rather than passing a
21+
fresh ``CallbackServerParameters`` object, so PySpark's own callback wiring
22+
is preserved -- the JVM-side callback client stays consistent (no "Error
23+
while obtaining a new communication channel", #19) and, crucially,
24+
``shutdown_callback_server()`` still returns cleanly at teardown. Passing a
25+
fresh parameters object + ``resetCallbackClient`` instead makes the accept
26+
thread un-joinable and hangs shutdown on Linux and macOS, which would hang
27+
the test suite's ``tearDownClass``.
28+
"""
29+
params = getattr(gateway, "callback_server_parameters", None)
30+
if params is not None and getattr(params, "port", 0) == DEFAULT_PYTHON_PROXY_PORT:
31+
params.port = 0
532

633

734
class PythonCallback:
@@ -12,16 +39,19 @@ def __init__(self, gateway):
1239
# P4j will return false if the callback server is already started
1340
# https://github.com/bartdag/py4j/blob/master/py4j-python/src/py4j/java_gateway.py
1441
callback_server = self.gateway.get_callback_server()
15-
# TODO clean
1642
if callback_server is None:
43+
# No callback server yet: ensure a dynamic port (avoid the 25334
44+
# collision) and start it the stock way so PySpark's callback
45+
# wiring -- and clean shutdown -- are preserved.
46+
_ensure_dynamic_callback_port(self.gateway)
1747
self.gateway.start_callback_server()
1848
print("Python Callback server started!") # TODO Logging
1949
elif callback_server.is_shutdown:
50+
# The previous callback server was shut down (e.g. a prior Spark
51+
# session was stopped). Restart it (also dynamic-port safe).
2052
callback_server.close()
53+
_ensure_dynamic_callback_port(self.gateway)
2154
self.gateway.restart_callback_server()
22-
# Have you tried turning it off and on again?
23-
# TODO why do we need to restart this every time?
24-
# TODO Will this break during chained function calls?
2555
print("PythonCallback server restarted!")
2656

2757

tests/test_checks.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,59 @@ def test_isUnique(self):
452452
self.assertEqual(self.isUnique("b", "All rows are unique"), [Row(constraint_status="Success")])
453453
self.assertEqual(self.isUnique("email", "All rows are unique"), [Row(constraint_status="Success")])
454454

455+
def test_lambda_check_uses_dynamic_callback_port(self):
456+
"""Regression test for the py4j callback-server default-port (25334) collision.
457+
458+
Closes #86, #19, #7, #72, #156, #173, #198.
459+
460+
With older gateways PythonCallback let py4j bind the hardcoded default
461+
callback port 25334. Concurrent or repeated runs on the same host that
462+
used a lambda-based Check then collided with
463+
``OSError: [Errno 98] Address already in use (127.0.0.1:25334)``. The fix
464+
forces a dynamic (OS-assigned) port by setting the gateway's existing
465+
callback-server parameters to ``port = 0`` before starting the server the
466+
stock way, so PySpark's callback wiring -- and clean shutdown -- are
467+
preserved.
468+
469+
This test runs lambda-assertion Checks and asserts that the resulting
470+
callback server is listening on a dynamic port -- never the hardcoded
471+
default 25334. The second lambda assertion additionally exercises the #19
472+
path: reusing PySpark's parameters keeps the JVM callback client pointed
473+
at the bound port, otherwise the check would fail with "Error while
474+
obtaining a new communication channel".
475+
476+
This test is deliberately NON-INVASIVE: it does not start, stop, restart,
477+
or close the shared py4j callback server, and it does not bind any port
478+
itself. Manipulating the shared callback server here was found to either
479+
hang ``tearDownClass``'s ``shutdown_callback_server`` (a callback thread
480+
left blocked in ``recv`` never joins) or break later lambda tests that
481+
reuse the connection. So we only observe the port the production fix chose.
482+
"""
483+
gateway = self.spark.sparkContext._gateway
484+
485+
# A lambda assertion ensures the py4j callback server is running (started by
486+
# PythonCallback via the fix on an OS-assigned dynamic port).
487+
result = self.hasSize(lambda x: x == 3.0)
488+
self.assertEqual(result, [Row(constraint_status="Success")])
489+
490+
callback_server = gateway.get_callback_server()
491+
self.assertIsNotNone(callback_server, "a lambda Check should have started the callback server")
492+
listening_port = callback_server.get_listening_port()
493+
self.assertNotEqual(
494+
listening_port,
495+
25334,
496+
"Callback server must not use the hardcoded default port 25334; "
497+
f"got {listening_port}",
498+
)
499+
self.assertGreater(listening_port, 0, "callback server should be bound to a real port")
500+
501+
# A second lambda assertion in the same process exercises the #19 path
502+
# (the JVM callback client must point at the dynamic port). If the client
503+
# were not reset, this would raise "Error while obtaining a new
504+
# communication channel" instead of returning a result.
505+
result2 = self.hasSize(lambda x: x >= 2.0 and x < 5.0)
506+
self.assertEqual(result2, [Row(constraint_status="Success")])
507+
455508
def test_fail_isUnique(self):
456509
self.assertEqual(self.isUnique("d"), [Row(constraint_status="Failure")])
457510
self.assertEqual(self.isUnique("f", "All rows are unique"), [Row(constraint_status="Failure")])

0 commit comments

Comments
 (0)