Skip to content
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.

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

Socket timeout when pushing a significant amount of data #2483

Closed
carver opened this issue May 25, 2022 · 3 comments
Closed

Socket timeout when pushing a significant amount of data #2483

carver opened this issue May 25, 2022 · 3 comments

Comments

@carver
Copy link
Collaborator

carver commented May 25, 2022

  • Version: 5.29.0
  • Python: 3.8
  • OS: linux

What was wrong?

When pushing a big chunk of data to trin (>400 thousand hex chars), I got this:

    w3.provider.make_request('portal_historyStore', [content_key_hex, content_value_hex])
  File "/home/jcarver/code/eth-portal/venv-py3.8/lib/python3.8/site-packages/web3/providers/ipc.py", line 240, in make_request
    sock.sendall(request)
socket.timeout: timed out

I'm pretty open to the idea that trin is doing something wrong here, but I'm not sure what it might be yet. But it seems likely to me that I'm pushing more data than usual here. Note that increasing the timeout does seem to prevent the error. (It's flaky, but happened once in the first two runs with the original timeout, and hasn't happened since on 5+ runs with the longer timeout)

How can it be fixed?

The workaround I found so far is to patch my virtualenv copy of web3.py in web3/providers/ipc.py at:

- def get_ipc_socket(ipc_path: str, timeout: float = 0.1) -> socket.socket:
+ def get_ipc_socket(ipc_path: str, timeout: float = 2.0) -> socket.socket:

It seems that the timeout field is not set when PersistantSocket calls get_ipc_socket(), so there is no obvious way to configure it as a user.

This workaround won't work for long, because I can't ask users to do that. I guess I would have to do some pretty ugly monkey-patching to hide the issue from users.

@kclowes
Copy link
Collaborator

kclowes commented May 25, 2022

I don't see a reason why we can't make it configurable. Maybe something like:

class IPCProvider(JSONBaseProvider):
    ...
    def __init__(self,
        ipc_path: Union[str, Path] = None,
        ipc_socket_timeout: float = 2.0,
        timeout: int = 10,
        *args: Any,
        **kwargs: Any) -> None:

        if ipc_path is None:
            self.ipc_path = get_default_ipc_path()
        elif isinstance(ipc_path, str) or isinstance(ipc_path, Path):
            self.ipc_path = str(Path(ipc_path).expanduser().resolve())
        else:
            raise TypeError("ipc_path must be of type string or pathlib.Path")

        self.ipc_socket_timeout = ipc_socket_timeout
        self.timeout = timeout
        self._lock = threading.Lock()
        self._socket = PersistantSocket(self.ipc_path, timeout=self.ipc_socket_timeout)
        super().__init__()
 
    ...

and then when PersistantSocket calls get_ipc_socket, that timeout can get passed through.

In the meantime, I don't see any problem with bumping that IPC socket timeout some.

@carver
Copy link
Collaborator Author

carver commented May 25, 2022

In the meantime, I don't see any problem with bumping that IPC socket timeout some.

Cool, yeah, I don't see any problem in bumping it up either.

@carver
Copy link
Collaborator Author

carver commented Jun 1, 2022

I guess I can close this, since #2485 was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants