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

Review missing timeouts #603

Open
7 of 9 tasks
josecelano opened this issue Jan 12, 2024 · 2 comments
Open
7 of 9 tasks

Review missing timeouts #603

josecelano opened this issue Jan 12, 2024 · 2 comments
Labels
Bug Incorrect Behavior Enhancement / Feature Request Something New EPIC Contains several subissues
Milestone

Comments

@josecelano
Copy link
Member

josecelano commented Jan 12, 2024

I've opened this issue to collect and track all past, current and future problems related to timeouts.

Recently we have had problems with the live demo because the http_health_check binary does not have a timeout. Healthchecks started by the docker daemon are waiting forever if they don't receive a response, consuming more and more resources until the container crashes.

There are more places where we could have such problems. I'm going to describe here some places potentially affected by this type of issue.

Potentially affected code

Healthcheck binary

It's used in the tracker but also in the Index.

It's currently being fixed:

Tracker API

It uses Axum. All services using Axum have the same problem. For more info see the issue opened in the Index. It applies the same here.

HTTP Tracker

It also uses Axum.

UDP Tracker

We do not use any framework. We handle requests directly with a loop.

  • We don't have timeouts.

  • If I'm not wrong all requests are executed sequentially. We don't use threads. If one request takes forever for some reason the server can not handle more requests and there is no timeout.

  • Add timeout to UDP tracker request processing #609

@da2ce7 is working on some improvements like:

  • Create a worker that can handle 50 requests at the most.
  • It spawns a new thread for each request.
  • It implements a graceful shutdown waiting for the active requests to finish when the application receives a halt signal.

I think he has not added yet the timeout feature.

By the way @WarmBeer since you are working on performance issues:

I wonder if improving the repository can improve the UDP tracker since requests are handled sequentially anyway or am I missing something @WarmBeer?

Tracker Checker

Conclusion

There might be other cases.

Relates to:

@mickvandijke
Copy link
Member

I wonder if improving the repository can improve the UDP tracker since requests are handled sequentially anyway or am I missing something @WarmBeer?

Yeah the performance improvements I'm working on only improve parallel access to the torrent repository. Sequential access should be the same.

#565

@josecelano
Copy link
Member Author

All known issues on the server side have been solved. For the client side, @da2ce7 did it (at least for the Tracker Checker), but it's still pending to merge.

@josecelano josecelano removed their assignment Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Incorrect Behavior Enhancement / Feature Request Something New EPIC Contains several subissues
Projects
Status: No status
Development

No branches or pull requests

3 participants