Skip to content

Move pid file write/remove within AcceptorPool #708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 8, 2021
Merged

Move pid file write/remove within AcceptorPool #708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ad1b6cf
Move pid file write/remove within AcceptorPool
abhinavsingh Nov 8, 2021
6c4b223
Remove unused
abhinavsingh Nov 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1780,18 +1780,31 @@ ssl.SSLEOFError: EOF occurred in violation of protocol (_ssl.c:997)

```console
+-------------+
| |
| Proxy([]) |
| |
+------+------+
|
|
+-----------v--------------+
| |
| AcceptorPool(...) |
| |
+------------+-------------+
|
|
+-----------------+ | +-----------------+
| | | | |
| Acceptor(..) <-------------+-----------> Acceptor(..) |
+-----------------+ +-----------------+
| | | |
+---+-------------+ +---------+-------+
| |
| |
| +------++------++------++------++------+ |
| | || || || || | |
+----> || || || || <-----+
| || || || || |
+------++------++------++------++------+
Threadless Worker Processes
```

`proxy.py` is made with performance in mind. By default, `proxy.py`
Expand Down
14 changes: 14 additions & 0 deletions proxy/core/acceptor/pool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

from ..event import EventQueue

from ...common.utils import bytes_
from ...common.flag import flags
from ...common.constants import DEFAULT_BACKLOG, DEFAULT_IPV6_HOSTNAME
from ...common.constants import DEFAULT_NUM_WORKERS, DEFAULT_PORT
Expand Down Expand Up @@ -126,6 +127,7 @@ def __exit__(

def setup(self) -> None:
"""Setup socket and acceptors."""
self._write_pid_file()
if self.flags.unix_socket_path:
self._listen_unix_socket()
else:
Expand Down Expand Up @@ -155,6 +157,7 @@ def shutdown(self) -> None:
acceptor.join()
if self.flags.unix_socket_path:
os.remove(self.flags.unix_socket_path)
self._delete_pid_file()
logger.debug('Acceptors shutdown')

def _listen_unix_socket(self) -> None:
Expand Down Expand Up @@ -192,3 +195,14 @@ def _start_acceptors(self) -> None:
self.acceptors.append(acceptor)
self.work_queues.append(work_queue[0])
logger.info('Started %d workers' % self.flags.num_workers)

def _write_pid_file(self) -> None:
if self.flags.pid_file is not None:
# NOTE: Multiple instances of proxy.py running on
# same host machine will currently result in overwriting the PID file
with open(self.flags.pid_file, 'wb') as pid_file:
pid_file.write(bytes_(os.getpid()))

def _delete_pid_file(self) -> None:
if self.flags.pid_file and os.path.exists(self.flags.pid_file):
os.remove(self.flags.pid_file)
28 changes: 6 additions & 22 deletions proxy/proxy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
:copyright: (c) 2013-present by Abhinav Singh and contributors.
:license: BSD, see LICENSE for more details.
"""
import os
import sys
import time
import logging
Expand All @@ -18,7 +17,6 @@

from proxy.core.acceptor.work import Work

from .common.utils import bytes_
from .core.acceptor import AcceptorPool
from .http.handler import HttpProtocolHandler
from .core.event import EventManager
Expand Down Expand Up @@ -93,15 +91,14 @@


class Proxy:
"""Context manager to control core AcceptorPool server lifecycle.
"""Context manager to control AcceptorPool & Eventing core lifecycle.

By default, AcceptorPool is started with HttpProtocolHandler worker class
i.e. we are only expecting HTTP traffic to flow between clients and server.
By default, AcceptorPool is started with `HttpProtocolHandler` work class.
By definition, it expects HTTP traffic to flow between clients and server.

Optionally, also initialize a global event queue.
It is a multiprocess safe queue which can be used to
build pubsub patterns for message sharing or signaling
within the running proxy environment.
Optionally, it also initializes the eventing core, a multiprocess safe
pubsub system queue which can be used to build various patterns
for message sharing and/or signaling.
"""

def __init__(self, input_args: Optional[List[str]], **opts: Any) -> None:
Expand All @@ -113,17 +110,6 @@ def __init__(self, input_args: Optional[List[str]], **opts: Any) -> None:
self.work_klass: Type[Work] = HttpProtocolHandler
self.event_manager: Optional[EventManager] = None

def write_pid_file(self) -> None:
if self.flags.pid_file is not None:
# TODO(abhinavsingh): Multiple instances of proxy.py running on
# same host machine will currently result in overwriting the PID file
with open(self.flags.pid_file, 'wb') as pid_file:
pid_file.write(bytes_(os.getpid()))

def delete_pid_file(self) -> None:
if self.flags.pid_file and os.path.exists(self.flags.pid_file):
os.remove(self.flags.pid_file)

def __enter__(self) -> 'Proxy':
if self.flags.enable_events:
logger.info('Core Event enabled')
Expand All @@ -135,7 +121,6 @@ def __enter__(self) -> 'Proxy':
event_queue=self.event_manager.event_queue if self.event_manager is not None else None,
)
self.pool.setup()
self.write_pid_file()
return self

def __exit__(
Expand All @@ -149,7 +134,6 @@ def __exit__(
if self.flags.enable_events:
assert self.event_manager is not None
self.event_manager.stop_event_dispatcher()
self.delete_pid_file()


def main(
Expand Down
23 changes: 21 additions & 2 deletions tests/core/test_acceptor_pool.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,23 @@
:copyright: (c) 2013-present by Abhinav Singh and contributors.
:license: BSD, see LICENSE for more details.
"""
import unittest
import os
import socket
import tempfile
import unittest

from unittest import mock

from proxy.common.utils import bytes_
from proxy.common.flag import FlagParser
from proxy.core.acceptor import AcceptorPool


class TestAcceptorPool(unittest.TestCase):

@mock.patch('os.remove')
@mock.patch('os.path.exists')
@mock.patch('builtins.open')
@mock.patch('proxy.core.acceptor.pool.send_handle')
@mock.patch('multiprocessing.Pipe')
@mock.patch('socket.socket')
Expand All @@ -28,15 +35,19 @@ def test_setup_and_shutdown(
mock_socket: mock.Mock,
mock_pipe: mock.Mock,
mock_send_handle: mock.Mock,
mock_open: mock.Mock,
mock_exists: mock.Mock,
mock_remove: mock.Mock,
) -> None:
acceptor1 = mock.MagicMock()
acceptor2 = mock.MagicMock()
mock_acceptor.side_effect = [acceptor1, acceptor2]

num_workers = 2
pid_file = os.path.join(tempfile.gettempdir(), 'pid')
sock = mock_socket.return_value
work_klass = mock.MagicMock()
flags = FlagParser.initialize(num_workers=2)
flags = FlagParser.initialize(num_workers=2, pid_file=pid_file)

pool = AcceptorPool(flags=flags, work_klass=work_klass)
pool.setup()
Expand Down Expand Up @@ -66,5 +77,13 @@ def test_setup_and_shutdown(
sock.close.assert_called()

pool.shutdown()

mock_open.assert_called_with(pid_file, 'wb')
mock_open.return_value.__enter__.return_value.write.assert_called_with(
bytes_(os.getpid()),
)
mock_exists.assert_called_with(pid_file)
mock_remove.assert_called_with(pid_file)

acceptor1.join.assert_called()
acceptor2.join.assert_called()
36 changes: 0 additions & 36 deletions tests/test_main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
:license: BSD, see LICENSE for more details.
"""
import unittest
import tempfile
import os

from unittest import mock

Expand Down Expand Up @@ -208,40 +206,6 @@ def test_enable_devtools(
# Currently --enable-devtools alone doesn't enable eventing core
mock_event_manager.assert_not_called()

@mock.patch('time.sleep')
@mock.patch('os.remove')
@mock.patch('os.path.exists')
@mock.patch('builtins.open')
@mock.patch('proxy.proxy.EventManager')
@mock.patch('proxy.proxy.AcceptorPool')
@mock.patch('proxy.common.flag.FlagParser.parse_args')
def test_pid_file_is_written_and_removed(
self,
mock_parse_args: mock.Mock,
mock_acceptor_pool: mock.Mock,
mock_event_manager: mock.Mock,
mock_open: mock.Mock,
mock_exists: mock.Mock,
mock_remove: mock.Mock,
mock_sleep: mock.Mock,
) -> None:
pid_file = os.path.join(tempfile.gettempdir(), 'pid')
mock_sleep.side_effect = KeyboardInterrupt()
mock_args = mock_parse_args.return_value
self.mock_default_args(mock_args)
mock_args.pid_file = pid_file
main(['--pid-file', pid_file])
mock_parse_args.assert_called_once()
mock_acceptor_pool.assert_called()
mock_acceptor_pool.return_value.setup.assert_called()
mock_event_manager.assert_not_called()
mock_open.assert_called_with(pid_file, 'wb')
mock_open.return_value.__enter__.return_value.write.assert_called_with(
bytes_(os.getpid()),
)
mock_exists.assert_called_with(pid_file)
mock_remove.assert_called_with(pid_file)

@mock.patch('time.sleep')
@mock.patch('proxy.proxy.EventManager')
@mock.patch('proxy.proxy.AcceptorPool')
Expand Down