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

Remove local_ssd_count, and convert to ssd #1190

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

another-rex
Copy link
Contributor

@another-rex another-rex commented Apr 4, 2023

Works when running apply locally.

@another-rex another-rex requested a review from michaelkedar April 4, 2023 01:21
Copy link
Member

@michaelkedar michaelkedar left a comment

Choose a reason for hiding this comment

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

Looking at the indexer kubernetes files in osv-config, I can't tell if the indexer was even using the local SSDs

@another-rex
Copy link
Contributor Author

I think the ssd is mounted to /tmp, which is used to clone/copy the repositories that are being analysed.

@another-rex another-rex merged commit fc22abc into google:master Apr 4, 2023
@oliverchang
Copy link
Collaborator

Note that local SSDs are a fair bit faster than pd-ssd: https://cloud.google.com/compute/docs/disks#localssds

Does this have any performance implications for us?

@another-rex
Copy link
Contributor Author

Ohh, I got mixed up persistent disks and local ssds mixed up.

@michaelkedar I see what you mean now, I don't see the configs using the local disk either. Maybe it automatically uses local ssds when they are available?

Does this have any performance implications for us?

Yes and no? This should be faster than what is currently being used (just standard persistent hard disks I believe, since no requests specifically for the ephemeral ssds), but we would probably get a speed boost by switching to using local ssds. I'll look into configuring it.

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.

3 participants