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

Allow appdata being stored in a separate object storage multibucket #26142

Closed

Conversation

MorrisJobke
Copy link
Member

Disclaimer: This originated in a custom patchset and I just brought it into a bit better shape. It is still far from a proper solution, but I want to bring this up for discussions.

Why?

By default on a multibucket object storage setup the appdata files are all put into the bucket 0. This might cause some problems on various systems, where there is a lot of data (which are obviously the ones with an multibucket object store). The idea here is to allow to also put the appdata into a separate list of object store buckets to not have a lot of data (previews for example) in a single bucket.

What this does:

It basically replaces the object store instance on the fly (right before the write/read/unlink) operation with the one that fits the current path. It's definitely not a good way of handling this. Best would be to put this one layer above and register it as a proper Nextcloud storage.

Existing issues:

  • copyFromStorage does only check the storage ID and it is the same as this is only a transparent replacement and thus it most likely will fail when something is moved between appdata buckets or out of appdata bucket into another one.
  • getObjectStore is not covered and might randomly return one of the both instances (depending on what path was written/read from before)

@icewind1991
Copy link
Member

I would much prefer mounting a separate AppdataObjectStore at the appdata folder than jamming the logic in here.

@@ -591,6 +625,7 @@ private function copyFile(ICacheEntry $sourceEntry, string $to) {
$targetUrn = $this->getURN($targetEntry->getId());

try {
$this->verifyStorage($path);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
$this->verifyStorage($path);
$this->verifyStorage($to);

This was referenced May 20, 2021
@blizzz blizzz mentioned this pull request Jun 2, 2021
57 tasks
@blizzz
Copy link
Member

blizzz commented Jun 2, 2021

what's the state here?

@MorrisJobke
Copy link
Member Author

what's the state here?

Most likely this needs to be solved in a different way:

I would much prefer mounting a separate AppdataObjectStore at the appdata folder than jamming the logic in here.


Let's close this one then.

@MorrisJobke MorrisJobke closed this Jun 7, 2021
@MorrisJobke MorrisJobke deleted the feature/noid/allow-appdata-in-separate-storage branch June 7, 2021 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants