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

Run git gc periodically on the crates.io index #975

Merged
merged 5 commits into from Aug 15, 2020
Merged

Run git gc periodically on the crates.io index #975

merged 5 commits into from Aug 15, 2020

Conversation

ohaddahan
Copy link
Contributor

@Kixiron @jyn514 @pietroalbini

Deleted my fork and restart my changes, also separated them into smaller chunks.

Hopefully I got understood and implemented your comments properly.

(#956)

src/index/mod.rs Outdated Show resolved Hide resolved
src/utils/daemon.rs Outdated Show resolved Hide resolved
src/utils/daemon.rs Outdated Show resolved Hide resolved
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Looks very nice :)

src/docbuilder/queue.rs Show resolved Hide resolved
src/index/mod.rs Outdated Show resolved Hide resolved
@jyn514 jyn514 added A-admin Area: Administration of the production docs.rs server C-enhancement Category: This is a new feature S-waiting-on-author Status: This PR is incomplete or needs to address review comments labels Aug 15, 2020
ohaddahan and others added 4 commits August 15, 2020 19:11
Co-authored-by: Chase Wilson <buckshot1233@gmail.com>
Co-authored-by: Chase Wilson <buckshot1233@gmail.com>
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
Co-authored-by: Chase Wilson <buckshot1233@gmail.com>
@jyn514 jyn514 merged commit 3ed00ad into rust-lang:master Aug 15, 2020
@ohaddahan
Copy link
Contributor Author

BTW, maybe we should add some logging on success too, just to be sure it's actually working as expected.
What do you think?

pub fn run_git_gc(&self) {

@jyn514
Copy link
Member

jyn514 commented Aug 16, 2020

Sounds good to me. Time for another PR? 😆

@Byron
Copy link
Member

Byron commented Aug 16, 2020

It's my stated goal to as-soon-as-reasonable switch crates-index-diff over to use gitoxide, the main motivation being slightly faster pulls and smart-gc. I have been bitten by this myself more than once, and believe this pitfall shouldn't be one anymore.

I was thinking that we really want to run GC if there are too many packs, and too many really means more than we are allowed to open as per the file handle limit. What if gitoxide would track the amount of open handles and GC automatically when they run out. The GC could also be optimized to do only minimal work in favor of producing a combined pack fast, which from what I know now is totally possible.

Here is the tracking issue for feedback, comments and general brainstorming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-admin Area: Administration of the production docs.rs server C-enhancement Category: This is a new feature S-waiting-on-author Status: This PR is incomplete or needs to address review comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants