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.

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

Jump to bottom

feat: add timeout to get_most_recent_version to prevent hang/deadlock #1972

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add timeout to get_most_recent_version to prevent hang/deadlock #1972

wants to merge 1 commit into from

Conversation

corneliusroemer
Copy link
Member

@corneliusroemer corneliusroemer commented Jul 11, 2024 •
edited
Loading

Mitigates #1973

Checklist

  • Added a news entry
  • Regenerated schema JSON if schema altered (python conda_smithy/schema.py)

Conda smithy makes an HTTP request without timeout. This can cause a hang/deadlock - in my case for longer than 1 minute. Nothing was happening, no CPU in htop, no logs.

When I hit ctrl+c the traceback showed the program got stuck in:

def get_most_recent_version(name, include_broken=False):
    request = requests.get(
        "https://api.anaconda.org/package/conda-forge/" + name
    )

which lacks a timeout.

This PR adds a timeout of 60 seconds which should be reasonable and prevent deadlocks. As smithy runs in CI, it's probably a good idea to have it fail rather than stall indefinitely.

@corneliusroemer corneliusroemer requested a review from a team as a code owner July 11, 2024 13:27
@corneliusroemer corneliusroemer mentioned this pull request Jul 11, 2024
Open
jaimergp
jaimergp approved these changes Jul 11, 2024
View reviewed changes
Copy link
Member

@jaimergp jaimergp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I measured the responses for some packages to see if those 60s were well chosen. Spoiler: they are.

The code is:

for pkg in python curl vim requests black gcc_linux-64; do
  echo "Measuring $pkg"; 
  for i in 1 2 3; do 
    echo -n "  #$i ->"; 
    time curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null; 
  done; 
done

The results:

Measuring python
  #1 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.08s user 0.02s system 1% cpu 7.910 total
  #2 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.09s user 0.03s system 1% cpu 9.860 total
  #3 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.09s user 0.03s system 1% cpu 7.318 total
Measuring curl
  #1 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.04s user 0.02s system 0% cpu 7.747 total
  #2 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.04s user 0.02s system 1% cpu 4.570 total
  #3 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.04s user 0.01s system 0% cpu 5.204 total
Measuring vim
  #1 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.44s user 0.09s system 1% cpu 32.010 total
  #2 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.38s user 0.09s system 1% cpu 38.594 total
  #3 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.34s user 0.07s system 1% cpu 35.962 total
Measuring requests
  #1 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.02s user 0.01s system 1% cpu 2.730 total
  #2 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.03s user 0.01s system 1% cpu 2.403 total
  #3 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.03s user 0.01s system 2% cpu 1.816 total
Measuring black
  #1 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.03s user 0.01s system 1% cpu 2.604 total
  #2 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.03s user 0.01s system 2% cpu 2.231 total
  #3 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.05s user 0.02s system 2% cpu 2.920 total
Measuring gcc_linux-64
  #1 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.03s user 0.02s system 2% cpu 1.616 total
  #2 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.04s user 0.02s system 3% cpu 1.731 total
  #3 ->curl -sL https://api.anaconda.org/package/conda-forge/$pkg > /dev/null  0.04s user 0.01s system 2% cpu 2.429 total

The longest calls are for vim because of the sheer amount of artifacts involved. Interesting how anaconda.org does not do any paging or caching 🤔

@jaimergp
Copy link
Member

Ah, btw, please add a news/ file with a brief description of the changes and the PR number. Something like:

* Add a 60s timeout in Anaconda.org API calls to prevent hangs. (#1972)

isuruf
isuruf requested changes Jul 11, 2024
View reviewed changes
Copy link
Member

@isuruf isuruf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It takes 25 seconds for me for conda-forge-pinning and I would guess anyone with a slower connection will hit the timeout. Let's increase the timeout and also add an escape hatch (an env variable).

beckermr
beckermr requested changes Jul 11, 2024
View reviewed changes
Copy link
Member

@beckermr beckermr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the import changes from running isort? If so can you back them out or move them to a new pr?

@jaimergp jaimergp linked an issue Jul 16, 2024 that may be closed by this pull request
conda smithy rerender -c auto hangs for >1min in get_most_recent_version("conda-forge-pinning") function #1973
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isuruf isuruf isuruf requested changes

@beckermr beckermr beckermr requested changes

@jaimergp jaimergp jaimergp approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

conda smithy rerender -c auto hangs for >1min in get_most_recent_version("conda-forge-pinning") function
4 participants
@corneliusroemer @jaimergp @isuruf @beckermr