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

Make rm faster #1639

Merged
merged 1 commit into from
Oct 23, 2018
Merged

Make rm faster #1639

merged 1 commit into from
Oct 23, 2018

Conversation

baude
Copy link
Member

@baude baude commented Oct 12, 2018

When doing rm, we now parallelize the actual conainter deletions so they
can complete faster. This speeds up operations like rm -a.

Signed-off-by: baude bbaude@redhat.com

@baude
Copy link
Member Author

baude commented Oct 12, 2018

/approve

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: baude

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 12, 2018
@baude
Copy link
Member Author

baude commented Oct 12, 2018

bot, retest this please

})
}

deleteErrors := parallelExecuteWorkerPool(rt.NumCPU()*3, deleteFuncs)
Copy link
Member

Choose a reason for hiding this comment

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

I think we'd this "rt.NumCPU()*3" in another piece of code. If we'll need to change/adjust them in both places potentially, further emphasis on getting the 3 into a conf file.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe a global Podman flag for now - "--numworkers" or somesuch

Copy link
Member

Choose a reason for hiding this comment

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

a global flag would work for me instead of a conf file entry.

Copy link
Member Author

Choose a reason for hiding this comment

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

there probably needs to be a little bit of a heuristic function for this. Nothing too complicated; in fact, I've been taking notes along the way. Because the possible functions being called can differ, it makes sense. The relationship is between the task, number of CPUs, and number of parallel functions that need to be executed. At that time, I think it would also make sense for a --max-parallel jobs flag.

@baude baude force-pushed the pararm branch 2 times, most recently from 26f5d28 to 506cabd Compare October 18, 2018 16:10
@baude
Copy link
Member Author

baude commented Oct 18, 2018

ready for merge

@mheon
Copy link
Member

mheon commented Oct 18, 2018

I'll merge, but we should get a flag in to control max worker count soon

@mheon
Copy link
Member

mheon commented Oct 18, 2018

LGTM

@TomSweeneyRedHat
Copy link
Member

If you haven't already, could you make a jira card or libpod issue for the flag please?

@baude
Copy link
Member Author

baude commented Oct 18, 2018

i did

When doing rm, we now parallelize the actual conainter deletions so they
can complete faster.  This speeds up operations like rm -a.

Signed-off-by: baude <bbaude@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Oct 23, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 23, 2018
@openshift-merge-robot openshift-merge-robot merged commit a65fdf5 into containers:master Oct 23, 2018
@baude baude deleted the pararm branch December 22, 2019 19:01
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants