Objectstore multibucket #24760
Objectstore multibucket #24760
Conversation
|
By analyzing the blame information on this pull request, we identified @icewind1991 to be a potential reviewer |
rullzer
force-pushed
the
objectstore_multibucket
branch
from
6d043f9
to
492a026
Compare
|
Unit tests added |
| @@ -52,9 +52,27 @@ public function __construct(IConfig $config) { | |||
| * @return \OCP\Files\Mount\IMountPoint[] | |||
| */ | |||
| public function getHomeMountForUser(IUser $user, IStorageFactory $loader) { | |||
|
|
|||
| $config = $this->multiBucketObjectStore($user); | |||
There was a problem hiding this comment.
getConfigForMultiBuckerObjectStore ? 😉
A method name that doesn't really say what the method does is a bit confusing
|
Please adjust "config.sample.php" for the new values. I'm not 100% convinced about having a separate config section just for multi-bucket. |
|
Remember when we talked about it we said it would be good to be able to specify the Mapper class name. I see you haven't added the config option for that yet. Maybe considering my comments above about the separate config, maybe we could replace the "bucket" argument in the settings with this: So if there is a "bucketMapper" arg, it has precedence over "bucket". |
|
I did not yet edit the I created a separate config as a future proof thingy. Since if we reuse the config there would be no proper way for us to migrate from single to multibucket. |
|
Yes eventually we'd want the mapper to be specifiable. But for now I think 1 default is fine. We should first properly test this all before we do all the fancy stuff. Should be trivial to add btw. |
rullzer
force-pushed
the
objectstore_multibucket
branch
from
492a026
to
f0391bf
Compare
|
Hmmm ok, then fine by me for now 👍 Would be good to know what @butonic @icewind1991 and @owncloud/qa think |
|
Ah forgot to copy the commit that saves the user mapping to the db... will follow later this evening. |
rullzer
force-pushed
the
objectstore_multibucket
branch
from
f0391bf
to
920d253
Compare
|
ok now we properly save the bucket in the preference table. |
| #### DO NOT CHANGE ANYTHING ABOVE THIS LINE #### | ||
|
|
||
| ErrorDocument 403 /core/templates/403.php | ||
| ErrorDocument 404 /core/templates/404.php |
| $mapper = new \OC\Files\ObjectStore\Mapper($user); | ||
| $config['arguments']['bucket'] .= $mapper->getBucket(); | ||
|
|
||
| $this->config->setUserValue($user->getUID(), 'homeobjectstore', 'bucket', $config['arguments']['bucket']); |
rullzer
force-pushed
the
objectstore_multibucket
branch
from
920d253
to
abe338f
Compare
| } | ||
| $config['arguments']['user'] = $user; | ||
|
|
||
| $bucket = $this->config->getUserValue($user->getUID(), 'homeobjectstore', 'bucket', null); |
There was a problem hiding this comment.
For swift you would call this container. In ceph you might want to call it pool. We should stick to one terminology. I guess s3 is the most prominent, so bucket is fine. At least for our internal config.
There was a problem hiding this comment.
Yeah we have to use something... I really don't care what ;)
|
@rullzer @DeepDiver1975 @butonic so should we defer the whole thing for 9.2 to have a more rounded solution ? Or is there a benefit of having this merged first for 9.1 where some people could already setup their new systems to use multi-bucket ? CC @cmonteroluque |
|
I think moving it to 9.2 makes sense. To have things properly tested and reviewed. |
In general, I agree, but we do have a few who are expecting this feature in 9.1. As I understand it, multibucket is ok, but one can't migrate into it, it will only work for all new installations. Is that correct? Is there a logical path to migration from there, or would new installs be isolated and stuck in the future? |
|
They can only be new installations. And then it should be the same. |
|
Ok, then let's get this in in the current form. Needs a second reviewer @butonic @DeepDiver1975 @icewind1991 @Xenopathic |
|
👍 I'm fine with this as an initial version |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
PR to allow multiple buckets being used when objectstore is used as homestorage.
Note that currently only the s3 homeobjectstore will use the mutlibuckets (we could extend swift but for now I would not do that as we have enough to tests with s3 already).
TODO:
CC: @butonic @icewind1991 @PVince81 comments please.