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

Codespaces (conda): Fix No space left on device #36953

Closed
wants to merge 6 commits into from

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Dec 24, 2023

We put the conda package files, only needed during install, in a tmpfs.
This allows the creation of the Docker image to succeed, even on the smallest Codespace machines.

Fixes #36952

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe mkoeppe changed the title .devcontainer/onCreate-conda.sh: Print disk space diagnostics Codespaces: Fix No space left on device Dec 24, 2023
@mkoeppe mkoeppe marked this pull request as ready for review December 24, 2023 02:49
@mkoeppe mkoeppe self-assigned this Dec 24, 2023
@mkoeppe mkoeppe changed the title Codespaces: Fix No space left on device Codespaces (conda): Fix No space left on device Dec 24, 2023
@mkoeppe mkoeppe requested a review from tscrim December 25, 2023 02:40
Copy link
Collaborator

@tscrim tscrim left a comment

Choose a reason for hiding this comment

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

LGTM.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 27, 2023

Thanks!

@tobiasdiez
Copy link
Contributor

As I have explained in #36952, this is not really a nice fix and needs further investigation of what's the actual origin for the space problems (sorry, should have posted this here in the PR instead).

Also, I cannot reproduce the space problems at all. A new codespace created off the default branch has >12gb free.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 28, 2023

As I have explained in #36952, this is not really a nice fix

Also there you did not explain what would make it not "nice".

and needs further investigation of what's the actual origin for the space problems

It does not need further investigation. I investigated it, which enabled me to fix it.

Also, I cannot reproduce the space problems at all.

You'll have to explain what precisely you tried, or this report has no value.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 28, 2023

I'll note that declaring that "the root cause is not understood" is not appropriate review activity. (Previous instance of this method: #36498 (comment))

It is normal and expected - as we invite everyone to participate in review - that not all reviewers understand all technical points of a PR. This does not mean that more info is needed or more work is needed.

I would suggest for future participation in the review process something along the following lines: "Could you add a comment to the file that explains what the added line does, and why it is necessary - after all, a new codespace created off the default branch has >12gb free."

@tobiasdiez
Copy link
Contributor

and needs further investigation of what's the actual origin for the space problems

It does not need further investigation. I investigated it, which enabled me to fix it.

Awesome, could you then please share your analysis and answer the questions raised in #36952 (comment)?

Also, I cannot reproduce the space problems at all.

You'll have to explain what precisely you tried, or this report has no value.

I don't know what more to say then "create a new codespace on the develop branch". Maybe the direct link helps...https://github.com/codespaces/new?hide_repo_select=true&ref=develop&repo=597660615&skip_quickstart=true

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 29, 2023

Also, I cannot reproduce the space problems at all.

You'll have to explain what precisely you tried, or this report has no value.

I don't know what more to say then "create a new codespace on the develop branch".

As you well know, one can select the machine type to use. Which one did you use.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 30, 2023

and needs further investigation of what's the actual origin for the space problems

It does not need further investigation. I investigated it, which enabled me to fix it.

Awesome, could you then please share your analysis and answer the questions raised in #36952 (comment)?

Yes, let's take a look what the questions are.

The conda packages should only take about 2 * 7gb of space. That should still give plenty of space to build sage. Also it looks like that it already fails for you at the download, which would mean that it has less then 7gb to start with. Strange.

No question so far. You're expressing your expectations and puzzlement.

Did you analyze what takes so much space, except for the conda packages?

No, and this question does not make sense because the container on which "docker build" is run in order to build our image is not under our control.

It is simply an observed fact what space is available in the temporary container that "docker build" uses. I already shared this observed fact at
#36952 (comment)

Filesystem      Size  Used Avail Use% Mounted on
overlay          32G   23G  7.5G  76% /
tmpfs            64M     0   64M   0% /dev
shm              64M     0   64M   0% /dev/shm
/dev/sda1        44G   52K   42G   1% /tmp
/dev/root        29G   22G  7.2G  76% /vscode
/dev/loop3       32G   23G  7.5G  76% /workspaces
tmpfs           3.9G     0  3.9G   0% /proc/acpi
tmpfs           3.9G     0  3.9G   0% /proc/scsi                         
tmpfs           3.9G     0  3.9G   0% /sys/firmware

So, not enough space in /opt (where the conda packages go to) as per your estimates.

Can we now please know what would make my solution "not nice", as you say?

@tobiasdiez
Copy link
Contributor

Did you analyze what takes so much space, except for the conda packages?

No, and this question does not make sense because the container on which "docker build" is run in order to build our image is not under our control.

It is simply an observed fact what space is available in the temporary container that "docker build" uses. I already shared this observed fact at #36952 (comment)

I don't understand this. The onCreateCommand is run inside the main container, after the image is build.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 30, 2023

In pictures - it's happening in this step.

Screenshot 2023-12-29 at 11 01 58 PM

@tobiasdiez
Copy link
Contributor

Yes, and whatever is run at that point is inside the main container image that already has been build. So it does make sense to ask the question what takes up these 32gb of space at that moment.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Dec 30, 2023

whatever is run at that point is inside the main container image that already has been build

Images don't "run". Containers do. And how much space they have when they run depends on more than what is "inside it".

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 3, 2024

If you are still continuing your review, set it to "needs review" please.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 9, 2024

@tobiasdiez I had to call you out many times last year for abusive use of your Triage team privileges.
That you are now apparently using an automatic script that manipulates labels is a new escalation.

@tobiasdiez
Copy link
Contributor

@tobiasdiez I had to call you out many times last year for abusive use of your Triage team privileges.

Please stop these repeated, and wrong, accusations of abusive use of privileges. You don't have the rights to publicly judge other members of the community. If you think I've crossed the boundaries, please directed this to the appropriate channels, such as the sage-abuse team, for thorough investigation and resolution.

But do please explain publicly why you think that removing the positive review label on a PR labeled as "disputed" (and thus "controversial, needs community involvement") is an abuse of privileges, or publicly apologize for making such wrong accusations.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 10, 2024

Tobias, as you know, I added the "disputed" label here. I did so to mark this PR as (yet another) one that is affected by your persistent misconduct.

Copy link

Documentation preview for this PR (built with commit d67b011; changes) is ready! 🎉

@vbraun
Copy link
Member

vbraun commented Jan 29, 2024

Editor decision

Will merge since there is a PR already. Don't really care about workarounds for github codespace limitations.

vbraun pushed a commit to vbraun/sage that referenced this pull request Jan 30, 2024
    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

We put the conda package files, only needed during install, in a tmpfs.
This allows the creation of the Docker image to succeed, even on the
smallest Codespace machines.

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
Fixes sagemath#36952

<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36953
Reported by: Matthias Köppe
Reviewer(s): Travis Scrimshaw
Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

@vbraun please reconsider this. With a new codespace created from the develop branch (without any prebuilds, smallest system config) the build goes through without any problems and afterwards one has quite a bit of space available:

@tobiasdiez ➜ /workspaces/sage (develop) $ df -h
Filesystem      Size  Used Avail Use% Mounted on
overlay          32G  3.9G   26G  14% /

While Matthias write in the changed file that he sees with this PR here

# Filesystem      Size  Used Avail Use% Mounted on
# overlay          32G   23G  7.5G  76% /

I have no idea how he got these numbers. Maybe there were some temporary problems with how the codespaces were created and indeed some file from the build directory leaked into the codespace. I have no idea, but whatever it was it seemed to be fixed either on github's side or perhaps by #35986.

So, in my opinion, there is no need for any workaround and, in particular, for removing the conda cache that is quite handy to have when you work with conda to create a new env. (And the whole point of this codespace is to make it convient to work with conda.)

@vbraun
Copy link
Member

vbraun commented Jan 31, 2024

OK if its not needed any more then even better.

In general one should "treat containers as cattle, not pets" though. Don't update the container, just get rid of it and build a new one.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 31, 2024

I can confirm that the "No space left" condition does not repro as of today.

@mkoeppe mkoeppe closed this Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disputed PR is waiting for community vote, see https://groups.google.com/g/sage-devel/c/IgBYUJl33SQ
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Codespace (conda) setup fails with No space left on device
4 participants