Skip to content

nydus-backend-proxy reload blob objects map #556

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

Merged
merged 3 commits into from
Jul 5, 2022
Merged

nydus-backend-proxy reload blob objects map #556

merged 3 commits into from
Jul 5, 2022

Conversation

changweige
Copy link
Contributor

@changweige changweige commented Jul 4, 2022

Current code is hard to implement blobs fd map reload since blobs manager has to be mutable among different futures.
At present nydus-backend-proxy only loads blobs info when it starts by readdir(). So blob which is generated and stored in the blobsdir can't be found responding to the HTTP requests.

This PR refactors the proxy a bit and prepares to reload blobs, it will benefit nydus-test since nydus-test is testing container images with multiple layers.

With blob fd map reload, backend-proxy will try to readdir again when blob is not found.

Current code is hard to implement blobs fd map reload since
blobs manager has to be mutable among different futures.

Signed-off-by: Changewei Ge <[email protected]>
@yqleng1987
Copy link
Contributor

@changweige , a new test job has been submitted. Please wait in patience.

@changweige
Copy link
Contributor Author

changweige commented Jul 4, 2022

nydus-backend-proxy has no file header license declaration, should we add it? @bergwolf @liubin @jiangliu

@yqleng1987
Copy link
Contributor

@changweige , The CI test is completed, please check result:

Test CaseTest Result
merge-target-branch✅SUCCESS
build-docker-image✅SUCCESS
compile-nydus✅SUCCESS
compile-ctr-remote✅SUCCESS
compile-nydus-snapshotter✅SUCCESS
start-nydus-snapshotter-config-containerd✅SUCCESS
run-container-with-nydus-image✅SUCCESS

Congratulations, your test job passed!

@jiangliu
Copy link
Collaborator

jiangliu commented Jul 4, 2022

nydus-backend-proxy has no file header license declaration, should we add it? @bergwolf @liubin @jiangliu

Please add license claim for it:)

Changewei Ge added 2 commits July 5, 2022 14:04
Signed-off-by: Changewei Ge <[email protected]>
Collaborating system probably put files after the server starts.
So the nydus-backend-proxy http server is better to re-scan
files if it can't find the requested object.

Signed-off-by: Changewei Ge <[email protected]>
@yqleng1987
Copy link
Contributor

@changweige , your pull request has been updated. A new test job will be submitted. Please wait in patience.

@yqleng1987
Copy link
Contributor

@changweige , your test job has passed, and no need to test again.

@changweige changweige changed the title backend-proxy: prepare nydus backend proxy fd map reload nydus-backend-proxy reload blob objects map Jul 5, 2022
@changweige
Copy link
Contributor Author

nydus-backend-proxy has no file header license declaration, should we add it? @bergwolf @liubin @jiangliu

Please add license claim for it:)

Added

@changweige
Copy link
Contributor Author

I already ran nydus-test using this nydus-banckend-proxy as storage backend without any data corruption failure.
So it is tested.

blobs: HashMap<String, fs::File>,
}
impl BlobBackend {
fn populate_blobs_map(&mut self) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it necessary to load the whole blob directory if the blob is not found on fetching?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought is to load all new files into fd map after re-scanning since the nydusd-backend-proxy 's exports all the files int blobsdir , which I think is its SLO

@jiangliu jiangliu merged commit b4fcccc into dragonflyoss:master Jul 5, 2022
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.

5 participants