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

process, tcpserver: Soft-deprecate multi-process mode #2801

Closed
bdarnell opened this issue Jan 21, 2020 · 8 comments · Fixed by #3149
Closed

process, tcpserver: Soft-deprecate multi-process mode #2801

bdarnell opened this issue Jan 21, 2020 · 8 comments · Fixed by #3149

Comments

@bdarnell
Copy link
Member

Multi-process mode has a lot of subtleties; it is responsible for a large fraction of our questions on stack overflow. And now that we have SO_REUSEPORT, I see little reason to use it instead of multiple independent processes (I've always viewed multiple independent processes as desirable for "serious" production use, but the need for an external load balancer is a significant operational overhead that justifies the shared-listener multiprocess mode. But if you can share a port without fork and its associates subtlety, that's even better)

We should at least update the docs to steer people towards reusing ports instead of forking multiple-processes. We might want to go as far as deprecating the process argument to server.start(), although I'm in no rush to actually remove it (I don't think I'd deprecate the process.fork_processes() function - empirically the people who get in trouble with multi-process mode are all using start(N); explicit forking is much more explicit and either the people who try it are able to get it working without difficulty or no one's trying it).

@mgcdanny
Copy link

I'd love to see an example of how SO_REUSEPORT can be used to replace start(N).

@bdarnell
Copy link
Member Author

SO_REUSEPORT on its own doesn't do it. But what you can do is replace your calls to bind(options.port); start(N) with listen(options.port, reuse_port=True) and then run N copies of the same process.

@lfvillavicencio
Copy link

Can you make an example using the reuse_port? Hot to run the following code with reuse_port ?

import tornado.ioloop
import tornado.web

class MainHandler(tornado.web.RequestHandler):
    def get(self):
        self.write("Hello, world")

def make_app():
    return tornado.web.Application([
        (r"/", MainHandler),
    ])

if __name__ == "__main__":
    app = make_app()
    app.listen(8888)
    tornado.ioloop.IOLoop.current().start()

@bdarnell
Copy link
Member Author

Just add reuse_port=True to the app.listen call, and run multiple copies of the script.

@lfvillavicencio
Copy link

lfvillavicencio commented Oct 23, 2021

oh I see. I wanted that parent process (main application) starts with reuse_port and then child process (tensorboard) starts many visualizations on same port, with different context, i.e localhost:port/tensorboard/path1, localhost:port/tensorboard/path2, localhost:port/tensorboard/path3
But I think that this doesnt work like this

@bdarnell
Copy link
Member Author

reuse_port is for when all the instances behind the port serve exactly the same application. To run different applications in different processes that are distinguished by path, you need a layer 7 proxy like haproxy or nginx to route the requests accordingly.

willangley added a commit to willangley/pdf_sprinkles that referenced this issue Nov 12, 2021
This uses the soft-deprecated multiprocessing mode of Tornado. Later
versions will switch away from it.

Ref: tornadoweb/tornado#2801
@suryapradhan123
Copy link

suryapradhan123 commented Dec 25, 2021

Is it ok if we install a SIGTERM handler in the parent process to explicitly kill the child process on exit? We are seeing this problem #1912 in our space and looking for possible solutions. Want to ensure this wouldn't cause leaks or other possible conflicts

@bdarnell
Copy link
Member Author

Installing a SIGTERM handler in the parent process won't hurt anything, but it's an imperfect solution to orphaned processes as seen in #1912. SIGTERM is not the only reason a process can exit; you could also add handlers for SIGINT and others but you can't have a handler for SIGKILL. A complete solution would need to be in the child processes, either polling getppid or (linux-specific) using pidfd. But if you're mainly concerned about SIGTERM to the parent process, then a SIGTERM handler could work well enough for you.

bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 3, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 4, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

Fixes tornadoweb#2801
bdarnell added a commit to bdarnell/tornado that referenced this issue Jun 4, 2022
This is partially a casualty of the Python 3.10 deprecation
changes, although it's also something I've wanted to do for other
reasons, since it's been a very common source of user confusion.

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

Successfully merging a pull request may close this issue.

4 participants