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 ability to not use loopback dev for btrfs #3739

Open
kcmannem opened this issue Apr 15, 2019 · 8 comments
Open

Add ability to not use loopback dev for btrfs #3739

kcmannem opened this issue Apr 15, 2019 · 8 comments
Assignees

Comments

@kcmannem
Copy link
Member

@cirocosta and I noticed when concourse starts trashing memory. The loop0 device used by btrfs creates lots of load. We can get around this by attaching an ephemeral disk to our workers through the cloud-config. Then use that device as the btrfs volume. This will reduce alot of administrative overhead used by concourse.

In the flamegraph posted below, during high load situation (eg max containers reached) kswap+loop0+telegraph use about 60% of system resources. We need to reduce this.

Screen Shot 2019-04-15 at 5 28 19 PM

@kcmannem kcmannem self-assigned this Apr 15, 2019
@kcmannem kcmannem added this to Icebox in Runtime via automation Apr 15, 2019
@kcmannem
Copy link
Member Author

@ddadlani @topherbullock

@cirocosta
Copy link
Member

Hey, another detail before breaking this down to other possible issues - it's also interesting to see how that filepath.Walk that we perform might end up being quite expensive depending on the volumes that we have.

If I understood correctly, such "walking" goes through every file under that volume:

https://github.com/concourse/baggageclaim/blob/6fee2c8029decd1325b5e9d6d4069ea4a7c4578d/volume/driver/btrfs.go#L40-L59

Is that correct? If so, perhaps it'd be good to somehow remove that big walk as that means at least one syscall for each possible file in the volume 🤔 Wdyt?

Thanks!

@ddadlani
Copy link
Contributor

To do this, we would likely need to change baggageclaim. The current behaviour is to create a loopback device on startup and to use that, we would need a way to bypass this behaviour and pass it the mount point of an external disk or something.

@vito
Copy link
Member

vito commented May 13, 2019

@ddadlani It should only do that if it detects that the disk isn't already formatted as btrfs.

@cirocosta
Copy link
Member

cirocosta commented May 14, 2019

(the if that @vito mentioned lives here: https://github.com/concourse/baggageclaim/blob/6fee2c8029decd1325b5e9d6d4069ea4a7c4578d/baggageclaimcmd/driver_linux.go#L64-L78 - I was very recently looking at that portion 👀 )

@ddadlani
Copy link
Contributor

Thanks for pointing that out, looks like it should be straightforward to test with an ephemeral disk for volumes 👍

@ddadlani
Copy link
Contributor

I recall that we couldn't remove the filepath.Walk because of concourse/baggageclaim#21 (comment)

Do we still want to test btrfs with an ephemeral disk?

@kcmannem
Copy link
Member Author

@ddadlani prob at some point but it’s fallen out of our priorities. I think we should revert the default to overlay as we’ve had good times with it.

@ddadlani ddadlani removed this from Backlog in Runtime Jul 30, 2019
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

No branches or pull requests

4 participants