dtenedor commented on code in PR #57686:
URL: https://github.com/apache/spark/pull/57686#discussion_r3855564534


##########
python/pyspark/sql/tests/connect/test_connect_local_server_pool.py:
##########
@@ -65,15 +67,47 @@ def _non_listening_socket():
 def _spawn_live_process() -> "subprocess.Popen":
     """A child blocked on its parent pipe, standing in for a live pool 
server."""
     return subprocess.Popen(
-        [sys.executable, "-c", "import sys; sys.stdin.buffer.read()"],
+        [sys.executable, "-c", "import sys; sys.stdin.buffer.read()", 
_SERVER_CLASS],
         stdin=subprocess.PIPE,
         stdout=subprocess.DEVNULL,
         stderr=subprocess.DEVNULL,
     )
 
 
+def _spawn_stubborn_sleeper() -> "subprocess.Popen":
+    """A sleeper that ignores SIGTERM, standing in for a server hanging in 
shutdown. It
+    prints one line once its handler is installed so tests do not signal it 
too early."""
+    proc = subprocess.Popen(
+        [
+            sys.executable,
+            "-c",
+            "import signal, time\n"
+            "signal.signal(signal.SIGTERM, signal.SIG_IGN)\n"
+            "print('ready', flush=True)\n"
+            "time.sleep(300)",

Review Comment:
   Sleeping in tests could lead to flakiness and slow tests. Is it possible to 
use explicit synchronization instead?



##########
python/pyspark/sql/connect/local_server_pool.py:
##########
@@ -107,13 +113,146 @@ def resolved(command: str) -> str:
 # layers above, which measure a member's age as ``time.time() - created``.
 _MAX_CREATED = 253402300799
 
+# A retired server still alive this long after retirement is hard-killed. 
After the give-up
+# age, tracking is removed only once the process is gone, replaced, or 
successfully signalled.
+_RETIRE_KILL_AFTER = 30
+_RETIRE_GIVE_UP = 600
+_POOL_STATE_TEMP_PREFIX = ".pool-state-"
+
+
+def _positive_pid(value: Any) -> Optional[int]:

Review Comment:
   can we move these helper functions and the above magic numbers to a class 
that makes sense, for better encapsulation?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to