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

Fix unnecessary quota updates #13813

Conversation

mariusklocke
Copy link

@mariusklocke mariusklocke commented Jul 24, 2024

This is a bugfix for the issue reported here. I also removed the call to deleteProject, because in case a project ID has actually changed, it does not make sense to update the file attributes twice.

If want to emphasize that this is not only a performance tweak. The iteration of all files of a container can take several minutes. If a file gets deleted after LXD has build its file list and before the file attributes update has been performed, a disk resize operation will fail due to "no such file or directory" error. I observed this error multiple times in a productive workload.

If there are any questions, please ask.

Signed-off-by: Marius Klocke <[email protected]>
@mariusklocke mariusklocke force-pushed the fix-unnecessary-quota-updates branch from 473b4f3 to 20d26ba Compare July 24, 2024 15:35
@mariusklocke
Copy link
Author

I just rebased after merge of #13815

@mariusklocke
Copy link
Author

I just signed the CLA. Can somebody re-run the CI and assign a reviewer?

@tomponline
Copy link
Member

@mariusklocke i requested it get refreshed and then i can re-run the check

@mariusklocke
Copy link
Author

mariusklocke commented Aug 6, 2024

@tomponline I signed up for Launchpad / Ubuntu One yesterday. Can you re-run the check?

//edit: Just saw, that the check ran an hour ago. Now says "has not signed the CLA". I just filled the form here. But there is no signature field in these forms? What am I supposed to do here?

@tomponline
Copy link
Member

@mariusklocke thanks if you've submitted that form, then ill ask them to refresh the list internally (its manual at the moment).

@mariusklocke
Copy link
Author

@tomponline Pipeline is still red. Is there anything to do for me here?

@tomponline
Copy link
Member

We were waiting for an internal refresh from Canonical's web team for the CLA list, I believe this has been done now, but GH isn't offering me the retry test button.

Also I've asked @hamistao to look through this PR too.

@tomponline
Copy link
Member

@hamistao once you've reviewed this PR, there is also a PR of the same thing going into Incus lxc/incus#1163 so it may turn out easier to cherry-pick that (once merged) if the CLA check isn't going to pass.

@hamistao
Copy link
Contributor

@mariusklocke The changes look fine but we will cherry pick a similar commit from Incus to bypass the CLA check. Sorry for the delay on the review and thanks for the report and the solution!

@mariusklocke
Copy link
Author

@hamistao Fine by me! Can you make the fix available in a 5.21.x version?

tomponline added a commit that referenced this pull request Aug 30, 2024
Fixes #13115

This avoids setting project quotas when the project does not change
([from Incus](lxc/incus#1163)), and also avoids
unnecessary project setting on `DeleteProject`.

Reported-by: Marius Klocke
[[email protected]](mailto:[email protected])

Closes #13813
@tomponline
Copy link
Member

@hamistao Fine by me! Can you make the fix available in a 5.21.x version?

Yeah ill include this in back ports so goes into next 5.21.x release

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