Skip to content

Commit f626e1b

Browse files
committed
fix: bind py4j callback server to a dynamic port to avoid 25334 collision
PythonCallback started the py4j callback server with no port argument, so py4j bound the hardcoded default port 25334. Concurrent or repeated runs on the same host using a lambda-based Check then failed with "OSError: [Errno 98] Address already in use (127.0.0.1:25334)". Start the callback server on port 0 and reset the JVM-side callback client to the actually-bound port (the documented py4j pattern), fixing both the bind collision and the "Error while obtaining a new communication channel" failure seen when only port=0 was set. Closes #86, #19, #7, #72, #156, #173, #198
1 parent 2e628b2 commit f626e1b

2 files changed

Lines changed: 96 additions & 9 deletions

File tree

pydeequ/scala_utils.py

Lines changed: 42 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,37 @@
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 CallbackServerParameters, JavaObject
5+
6+
7+
def _start_dynamic_callback_server(gateway):
8+
"""Start the py4j callback server on a dynamically-allocated port and point
9+
the JVM-side callback client back at the port that was actually bound.
10+
11+
Historically PyDeequ called ``gateway.start_callback_server()`` with no
12+
arguments, which makes py4j bind the hardcoded default port (25334). When
13+
two pyspark applications, or two runs in the same host, used a lambda-based
14+
``Check`` at the same time, the second one failed with
15+
``OSError: [Errno 98] Address already in use (127.0.0.1:25334)``
16+
(issues #86, #7, #72, #156, #173, #198).
17+
18+
Binding ``port=0`` lets the OS pick a free port and removes the collision.
19+
However, just passing ``CallbackServerParameters(port=0)`` is not enough:
20+
the JVM still believes the callback client lives on the previously
21+
configured port, so assertions fail with
22+
``Error while obtaining a new communication channel`` (issue #19). The
23+
documented fix is to read back the real listening port and reset the
24+
JVM-side callback client to it. This mirrors py4j's own
25+
``ResetCallbackClientTest`` (py4j/tests/java_callback_test.py).
26+
"""
27+
gateway.start_callback_server(CallbackServerParameters(port=0))
28+
callback_server = gateway.get_callback_server()
29+
listening_port = callback_server.get_listening_port()
30+
gateway.java_gateway_server.resetCallbackClient(
31+
gateway.java_gateway_server.getCallbackClient().getAddress(),
32+
listening_port,
33+
)
34+
return listening_port
535

636

737
class PythonCallback:
@@ -12,17 +42,20 @@ def __init__(self, gateway):
1242
# P4j will return false if the callback server is already started
1343
# https://github.com/bartdag/py4j/blob/master/py4j-python/src/py4j/java_gateway.py
1444
callback_server = self.gateway.get_callback_server()
15-
# TODO clean
1645
if callback_server is None:
17-
self.gateway.start_callback_server()
18-
print("Python Callback server started!") # TODO Logging
46+
# No callback server yet: start one on a dynamic port to avoid the
47+
# default-port (25334) collision and reset the JVM callback client.
48+
listening_port = _start_dynamic_callback_server(self.gateway)
49+
print(f"Python Callback server started on port {listening_port}!") # TODO Logging
1950
elif callback_server.is_shutdown:
51+
# The previous callback server was shut down (e.g. a prior Spark
52+
# session was stopped). Tear it down fully and re-derive a fresh
53+
# dynamic port instead of reverting to the default port.
2054
callback_server.close()
21-
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?
25-
print("PythonCallback server restarted!")
55+
self.gateway.shutdown_callback_server()
56+
self.gateway._callback_server = None
57+
listening_port = _start_dynamic_callback_server(self.gateway)
58+
print(f"PythonCallback server restarted on port {listening_port}!")
2659

2760

2861
class ScalaFunction1(PythonCallback):

tests/test_checks.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,60 @@ 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+
The old PythonCallback called ``gateway.start_callback_server()`` with no
461+
port argument, so py4j bound the hardcoded default port 25334. If that port
462+
was already taken (a concurrent/repeated run on the same host), a lambda
463+
assertion failed with ``OSError: [Errno 98] Address already in use``.
464+
465+
This test occupies 25334 *before* the callback server is started, then runs
466+
a lambda-assertion Check. With the fix it binds a free dynamic port and
467+
succeeds; with the old code it would raise the bind error. It also asserts
468+
the callback server is not listening on the legacy default port.
469+
"""
470+
import socket
471+
472+
gateway = self.spark.sparkContext._gateway
473+
# Ensure no callback server is lingering from earlier tests so that this
474+
# Check is the one that (re)starts it. shutdown is a no-op if not started.
475+
gateway.shutdown_callback_server()
476+
gateway._callback_server = None
477+
478+
# Squat on the legacy default port so the old code path would collide.
479+
blocker = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
480+
blocker.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 0)
481+
squatting_default_port = False
482+
try:
483+
blocker.bind(("127.0.0.1", 25334))
484+
blocker.listen(1)
485+
squatting_default_port = True
486+
except OSError:
487+
# If 25334 cannot be bound here (rare), the test still validates the
488+
# success path; it just cannot prove the collision specifically.
489+
blocker.close()
490+
491+
try:
492+
# A lambda assertion forces a py4j callback server start.
493+
result = self.hasSize(lambda x: x == 3.0)
494+
self.assertEqual(result, [Row(constraint_status="Success")])
495+
496+
callback_server = gateway.get_callback_server()
497+
self.assertIsNotNone(callback_server)
498+
listening_port = callback_server.get_listening_port()
499+
if squatting_default_port:
500+
self.assertNotEqual(
501+
listening_port,
502+
25334,
503+
"Callback server must not use the hardcoded default port 25334",
504+
)
505+
finally:
506+
if squatting_default_port:
507+
blocker.close()
508+
455509
def test_fail_isUnique(self):
456510
self.assertEqual(self.isUnique("d"), [Row(constraint_status="Failure")])
457511
self.assertEqual(self.isUnique("f", "All rows are unique"), [Row(constraint_status="Failure")])

0 commit comments

Comments
 (0)