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

Add tempfs experiment and gate mounting behind it #490

Merged
merged 5 commits into from
Sep 24, 2024

Conversation

JoshuaKrstic
Copy link
Contributor

@JoshuaKrstic JoshuaKrstic commented Sep 13, 2024

--The binary still needs to be updated.-- Until then, this feature will always be off. Which is fine ofc.

The binary has been updated and the flag exists.

Copy link
Contributor

@alexmwu alexmwu left a comment

Choose a reason for hiding this comment

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

I'm not sure why the cloud build is not failing but you don't actually mount a tmpfs in the cloud build. Probably due to the experiment not being there?

if echo $SERIAL_OUTPUT | grep -q "tmpfs.*${_TMPFS_SIZE_KB}.*/tmp/sized"

Additionally, you aren't gating the /dev/shm feature. You can see /dev/shm size changes to from 64KB to 128000KB

@jkl73
Copy link
Contributor

jkl73 commented Sep 17, 2024

It's probably the string in printed by the launcher "Mounts:[{Destination:/tmp/sized Size:222}]", together with the "tmpfs" print by the kernel, and some number in the timestamp causing the regex to pass the grep. Maybe adding "^" and "$" to the grep pattern in the test may do the trick.

@alexmwu
Copy link
Contributor

alexmwu commented Sep 17, 2024

It's probably the string in printed by the launcher "Mounts:[{Destination:/tmp/sized Size:222}]", together with the "tmpfs" print by the kernel, and some number in the timestamp causing the regex to pass the grep. Maybe adding "^" and "$" to the grep pattern in the test may do the trick.

I think we can also try replacing .* with [[:space:]]

@jkl73
Copy link
Contributor

jkl73 commented Sep 17, 2024

Also we may want to add the flag here instead:

if val, ok := unmarshaledMap[devShmSizeKey]; ok && val != "" {

and

if val, ok := unmarshaledMap[mountKey]; ok && val != "" {

@JoshuaKrstic
Copy link
Contributor Author

/gcbrun

Joshua Krstic added 5 commits September 23, 2024 16:35

Verified

This commit was signed with the committer’s verified signature.
metcoder95 Carlos Fuentes

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@JoshuaKrstic
Copy link
Contributor Author

/gcbrun

@jkl73 jkl73 enabled auto-merge (squash) September 24, 2024 00:12
@jkl73 jkl73 dismissed alexmwu’s stale review September 24, 2024 00:15

function unexported, and Alex is OOO

@jkl73 jkl73 merged commit 7b31d76 into google:main Sep 24, 2024
11 checks passed
@JoshuaKrstic JoshuaKrstic deleted the tempfsexp branch September 24, 2024 18:11
jessieqliu pushed a commit that referenced this pull request Sep 25, 2024
* Add tempfs experiment and gate mounting behind it

* Move experiments to launch spec and gate tempfs change there instead

* fix tests by adding experiment flag

* Move hosttpmpath creation to before the launchspec initialization

* re-export UnmarshalLJson

---------

Co-authored-by: Joshua Krstic <jkrstic@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants