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

Attempting to install Rustup 1.24.2 hangs #2774

Closed
luqmana opened this issue May 18, 2021 · 15 comments
Closed

Attempting to install Rustup 1.24.2 hangs #2774

luqmana opened this issue May 18, 2021 · 15 comments
Labels

Comments

@luqmana
Copy link
Member

luqmana commented May 18, 2021

Problem

After #2756 rustup will just hang if it's not able to determine the effective max ram of the system. The differences seem to be:

  1. The fallback for when effective_limits::memory_limit changed from 500MB to 32MB.
  2. Rustup gets stuck waiting for some memory to free up here:
    while !io_executor.buffer_available(size as usize) {
    flush_ios::<tar::Entry<'_, R>, _>(
    &mut *io_executor,
    &mut directories,
    None,
    &full_path,
    )?;
    }

I encountered this running on an illumos machine.

Steps
On a machine not supported by effective_limits v0.5.2`:

$ curl https://sh.rustup.rs -sSf | sh -s -- --profile minimal --default-toolchain nightly

Possible Solution(s)

  1. Choose a larger default than 32MB (note, 64MB is still not enough for --profile complete).
  2. v0.5.3-alpha of the effective-limits crate supports illumos but hasn't been released yet. (New release rbtcollins/effective-limits.rs#15)

Notes

Output of rustup --version: 1.24.2 (755e2b07e 2021-05-12)

@luqmana luqmana added the bug label May 18, 2021
@chrisduerr
Copy link

This also breaks on FreeBSD, which I've noticed when my CI suddenly started failing. The default 32MB aren't even enough for the minimal install, so I think platforms without effective limits just can't build right now.

@rbtcollins
Copy link
Contributor

hmm, a hang is unexpected: the threaded executor allocates a chunk from every slab size, and that will allow at least one io to proceed; the streaming ios also don't have busy waits that I remember, so it should always just loop until something completes; mkdirs to let something flush don't get accounted for against the budget (sssh), so it should always be possible for flush_ios to complete.

obviously that isn't happening, which means figuring out why not.

you can workaround using single threaded mode: export RUSTUP_IO_THREADS=1 and you'll disable the fast path.

@Darksonn
Copy link

Adding the RUSTUP_IO_THREADS=1 environment variable fixed FreeBSD CI for us.

@kinnison
Copy link
Contributor

I can confirm this is reproducable on Linux, which means we (well I) stand a chance of diagnosing at least. It's a little odd that you say 64M isn't enough for complete though - can you let me know which component it gets stuck on in complete which isn't in default ? I've managed to reproduce the issue at 32M with minimal profile because the rustc component seems enough.

Is the CI you use for FreeBSD compatible with Github ? I'd be prepared to add more CI options to our master branch build if it helps us catch this kind of problem in the future.

@Darksonn
Copy link

We have a Cirrus CI setup on the Tokio github repository that we use for FreeBSD.

fnichol added a commit to fnichol/fnichol-cime that referenced this issue May 19, 2021
fnichol added a commit to fnichol/workstation that referenced this issue May 19, 2021
wez added a commit to wez/wezterm that referenced this issue May 22, 2021
sylvestre added a commit to sylvestre/coreutils that referenced this issue May 22, 2021
rust-lang/rustup#2774

It is failing currently on:
```
info: installing component 'cargo'
error: error: 'sysinfo not supported on this platform'
```
with 1.52.1
sylvestre added a commit to sylvestre/coreutils that referenced this issue May 22, 2021
rust-lang/rustup#2774

It is failing currently on:
```
info: installing component 'cargo'
error: error: 'sysinfo not supported on this platform'
```
with 1.52.1
sylvestre added a commit to sylvestre/coreutils that referenced this issue May 22, 2021
rust-lang/rustup#2774

It is failing currently on:
```
info: installing component 'cargo'
error: error: 'sysinfo not supported on this platform'
```
with 1.52.1
@Thomasdezeeuw
Copy link

Since this breaking/blocking some stuff in the ecosystem would it be possible to just use 500MB for the time being as a workout?

@jclulow
Copy link
Contributor

jclulow commented May 22, 2021

It's probably best to revert the regressing change for now -- the motivation was ostensibly a "hypothetical" memory fragmentation issue, whereas the hang is very real.

@kinnison
Copy link
Contributor

@jclulow the fragmentation problem was very real on tier 1 platforms. I have what I think is a fix for this hang and I hope I'll get a beta out early next week with it.

kinnison added a commit to kinnison/rustup that referenced this issue May 23, 2021
In rust-lang#2774 we have seen hangs during installation for some low tier
platforms for whom the 32M default if sysinfo doesn't work causes
problems.  This change *exposes* that problem in a failing test.

Signed-off-by: Daniel Silverstone <[email protected]>
kinnison added a commit to kinnison/rustup that referenced this issue May 23, 2021
In incremental file processing we need to correctly account for
the final chunk being released, otherwise we end up holding a chunk
for the duration of the rest of the component for every incremental
file in the tarball.  This is what caused the issue surfaced in the
previous commit, and thus this commit fixes rust-lang#2774

Signed-off-by: Daniel Silverstone <[email protected]>
asomers added a commit to nix-rust/nix that referenced this issue May 23, 2021
A regression in rustup has broken that tool on FreeBSD.  Set
RUSTUP_IO_THREADS=1 as a workaround.
rust-lang/rustup#2774
@rbtcollins
Copy link
Contributor

Yeah - to be clear, the problem was concrete, the hypothetical aspect was whether fragmentation was the cause.

rbtcollins pushed a commit to kinnison/rustup that referenced this issue May 28, 2021
In incremental file processing we need to correctly account for
the final chunk being released, otherwise we end up holding a chunk
for the duration of the rest of the component for every incremental
file in the tarball.  This is what caused the issue surfaced in the
previous commit, and thus this commit fixes rust-lang#2774

Signed-off-by: Daniel Silverstone <[email protected]>
kinnison added a commit to kinnison/rustup that referenced this issue May 28, 2021
In rust-lang#2774 we have seen hangs during installation for some low tier
platforms for whom the 32M default if sysinfo doesn't work causes
problems.  This change *exposes* that problem in a failing test.

Signed-off-by: Daniel Silverstone <[email protected]>
@Thomasdezeeuw
Copy link

Can we get a release for this? It's breaking the CI for a lot of people still.

@kinnison
Copy link
Contributor

@Thomasdezeeuw My intention is to prepare a beta release tonight, and hopefully to get it onto dev-static tomorrow. Keep an eye on the users forum for a call to test it.

@Tim-Zhang
Copy link

@kinnison How are things going?

@Rustin170506
Copy link
Member

@kinnison How are things going?

The beta release has been released and you can find the test method here.

@Rustin170506
Copy link
Member

Rustin170506 commented Jun 2, 2021

on the user forum.

It seems to be on the internals forum?

@kinnison
Copy link
Contributor

kinnison commented Jun 2, 2021

It seems to be on the internals forum?

Yeah, I get the two mixed up, sorry :(

rbtcollins pushed a commit to rbtcollins/rustup.rs that referenced this issue Jul 11, 2021
In rust-lang#2774 we have seen hangs during installation for some low tier
platforms for whom the 32M default if sysinfo doesn't work causes
problems.  This change *exposes* that problem in a failing test.

Signed-off-by: Daniel Silverstone <[email protected]>
rbtcollins pushed a commit to rbtcollins/rustup.rs that referenced this issue Jul 11, 2021
In incremental file processing we need to correctly account for
the final chunk being released, otherwise we end up holding a chunk
for the duration of the rest of the component for every incremental
file in the tarball.  This is what caused the issue surfaced in the
previous commit, and thus this commit fixes rust-lang#2774

Signed-off-by: Daniel Silverstone <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants