You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Bertrand Delacretaz <bd...@apache.org> on 2015/08/21 11:57:02 UTC
Is the hashing of long paths still needed?
Hi,
Continuing to play with Robert's MultiplexingDocumentStore [1] I got a
failure in the oak-jcr module's LongPathTest.
That's due to the conversion of long paths to their hashed variants -
those cannot be used to locate the appropriate DocumentStore when
multiplexed, as that decision is based on the real path. In some cases
like UpdateOp the real path is saved alongside the hashed one, but not
always.
Removing the path hashing as shown in the below patch removes that
problem, and all oak-core tests pass with this change.
Is that hashing still needed, or was that created due to backend
limitations which are gone now?
If still needed, could it be made configurable so that users can make
their own tradeoff (MultiplexingDocumentStore vs. supporting backends
that require that), or are there other constraints?
-Bertrand
[1] https://github.com/bdelacretaz/jackrabbit-oak/tree/bertrand-multiplex
diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
index 4577c4b..d97ae76 100644
--- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
+++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
@@ -254,6 +254,7 @@ public class Utils {
}
public static String getIdFromPath(String path) {
+ /*
if (isLongPath(path)) {
MessageDigest digest;
try {
@@ -267,6 +268,7 @@ public class Utils {
String name = PathUtils.getName(path);
return depth + ":h" + Hex.encodeHexString(hash) + "/" + name;
}
+ */
int depth = Utils.pathDepth(path);
return depth + ":" + path;
}
diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
index b9f619e..5ed1a95 100644
--- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
+++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
@@ -32,6 +32,7 @@ import org.junit.Test;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
@@ -62,7 +63,8 @@ public class UtilsTest {
String longPath = PathUtils.concat("/"+Strings.repeat("p",
Utils.PATH_LONG + 1), "foo");
assertTrue(Utils.isLongPath(longPath));
- assertNull(Utils.getParentId(Utils.getIdFromPath(longPath)));
+ // updated to match the changes to Utils.getIdFromPath
+ assertNotNull(Utils.getParentId(Utils.getIdFromPath(longPath)));
assertNull(Utils.getParentId(Utils.getIdFromPath("/")));
assertEquals("1:/foo",Utils.getParentId("2:/foo/bar"));
Re: Is the hashing of long paths still needed?
Posted by Michael Dürig <md...@apache.org>.
Hi,
This is probably https://issues.apache.org/jira/browse/OAK-333.
Michael
On 21.8.15 11:57 , Bertrand Delacretaz wrote:
> Hi,
>
> Continuing to play with Robert's MultiplexingDocumentStore [1] I got a
> failure in the oak-jcr module's LongPathTest.
>
> That's due to the conversion of long paths to their hashed variants -
> those cannot be used to locate the appropriate DocumentStore when
> multiplexed, as that decision is based on the real path. In some cases
> like UpdateOp the real path is saved alongside the hashed one, but not
> always.
>
> Removing the path hashing as shown in the below patch removes that
> problem, and all oak-core tests pass with this change.
>
> Is that hashing still needed, or was that created due to backend
> limitations which are gone now?
>
> If still needed, could it be made configurable so that users can make
> their own tradeoff (MultiplexingDocumentStore vs. supporting backends
> that require that), or are there other constraints?
>
> -Bertrand
>
> [1] https://github.com/bdelacretaz/jackrabbit-oak/tree/bertrand-multiplex
>
>
>
> diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
> b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
> index 4577c4b..d97ae76 100644
> --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
> +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/Utils.java
> @@ -254,6 +254,7 @@ public class Utils {
> }
>
> public static String getIdFromPath(String path) {
> + /*
> if (isLongPath(path)) {
> MessageDigest digest;
> try {
> @@ -267,6 +268,7 @@ public class Utils {
> String name = PathUtils.getName(path);
> return depth + ":h" + Hex.encodeHexString(hash) + "/" + name;
> }
> + */
> int depth = Utils.pathDepth(path);
> return depth + ":" + path;
> }
> diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
> b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
> index b9f619e..5ed1a95 100644
> --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
> +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/UtilsTest.java
> @@ -32,6 +32,7 @@ import org.junit.Test;
>
> import static org.junit.Assert.assertEquals;
> import static org.junit.Assert.assertNull;
> +import static org.junit.Assert.assertNotNull;
> import static org.junit.Assert.assertSame;
> import static org.junit.Assert.assertTrue;
>
> @@ -62,7 +63,8 @@ public class UtilsTest {
> String longPath = PathUtils.concat("/"+Strings.repeat("p",
> Utils.PATH_LONG + 1), "foo");
> assertTrue(Utils.isLongPath(longPath));
>
> - assertNull(Utils.getParentId(Utils.getIdFromPath(longPath)));
> + // updated to match the changes to Utils.getIdFromPath
> + assertNotNull(Utils.getParentId(Utils.getIdFromPath(longPath)));
>
> assertNull(Utils.getParentId(Utils.getIdFromPath("/")));
> assertEquals("1:/foo",Utils.getParentId("2:/foo/bar"));
>
Re: Is the hashing of long paths still needed?
Posted by Julian Reschke <ju...@gmx.de>.
On 2015-08-21 12:17, Bertrand Delacretaz wrote:
> On Fri, Aug 21, 2015 at 12:11 PM, Julian Reschke
> <ju...@greenbytes.de> wrote:
>>
>> ...It is still needed, as both persistence implementations (Mongo, RDB) are
>> restricted to IDs of 512 bytes length (yes, bytes, not characters)....
>
> Ok, thanks for the info!
>
> Interestingly, no oak-core or oak-jcr tests fail when I remove this
> feature and test with Mongo, I'll dig deeper.
>
> -Bertrand
Right, that's something we should fix.
Re: Is the hashing of long paths still needed?
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Fri, Aug 21, 2015 at 1:46 PM, Davide Giannella <da...@apache.org> wrote:
...
> mvn clean install -PintegrationTesting -Dnsfixtures=
Ah ok, the LongPathTest in oak-jcr does fail with my patch when using
-Dnsfixtures=
Sorry for the noise - we'll have to find a way to cope with this for
the MultiplexingDocumentStore.
-Bertrand
Re: Is the hashing of long paths still needed?
Posted by Davide Giannella <da...@apache.org>.
On 21/08/2015 12:17, Bertrand Delacretaz wrote:
> On Fri, Aug 21, 2015 at 12:11 PM, Julian Reschke
> <ju...@greenbytes.de> wrote:
>> ...It is still needed, as both persistence implementations (Mongo, RDB) are
>> restricted to IDs of 512 bytes length (yes, bytes, not characters)....
> Ok, thanks for the info!
>
> Interestingly, no oak-core or oak-jcr tests fail when I remove this
> feature and test with Mongo, I'll dig deeper.
>
Did you run something like the following having a mongodb up and
running? If not, could you try?
mvn clean install -PintegrationTesting -Dnsfixtures=
Davide
Re: Is the hashing of long paths still needed?
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Fri, Aug 21, 2015 at 12:11 PM, Julian Reschke
<ju...@greenbytes.de> wrote:
>
> ...It is still needed, as both persistence implementations (Mongo, RDB) are
> restricted to IDs of 512 bytes length (yes, bytes, not characters)....
Ok, thanks for the info!
Interestingly, no oak-core or oak-jcr tests fail when I remove this
feature and test with Mongo, I'll dig deeper.
-Bertrand
Re: Is the hashing of long paths still needed?
Posted by Julian Reschke <ju...@greenbytes.de>.
On 2015-08-21 11:57, Bertrand Delacretaz wrote:
> Hi,
>
> Continuing to play with Robert's MultiplexingDocumentStore [1] I got a
> failure in the oak-jcr module's LongPathTest.
>
> That's due to the conversion of long paths to their hashed variants -
> those cannot be used to locate the appropriate DocumentStore when
> multiplexed, as that decision is based on the real path. In some cases
> like UpdateOp the real path is saved alongside the hashed one, but not
> always.
>
> Removing the path hashing as shown in the below patch removes that
> problem, and all oak-core tests pass with this change.
>
> Is that hashing still needed, or was that created due to backend
> limitations which are gone now?
It is still needed, as both persistence implementations (Mongo, RDB) are
restricted to IDs of 512 bytes length (yes, bytes, not characters).
> If still needed, could it be made configurable so that users can make
> their own tradeoff (MultiplexingDocumentStore vs. supporting backends
> that require that), or are there other constraints?
I don't think disabling is an option; it's very easy to hit this length
limit.
Best regards, Julian
Re: Is the hashing of long paths still needed?
Posted by Robert Munteanu <ro...@apache.org>.
Hi,
On Fri, 2015-08-21 at 14:07 +0000, Thomas Mueller wrote:
> Hi,
>
> The DocumentStore doesn't really know the path, it only knows the
> key, and
> if the key is hashed you can't calculate the path.
>
> There are some options:
>
> (a) Each document that has a hashed path as the key also has a "path"
> property (with the real path). You could use that (cache it, read it
> if
> needed, possibly from all backends).
That's mostly the approach that we have taken for now. I don't think a
(slight) performance penalty for hashed documents is a huge issue in
the prototype stage, so if I can't extract the path out of the key I
simply ask all the stores ( details at [1] ). This seems to work out
fine so far.
Thanks,
Robert
[1]: https://github.com/rombert/jackrabbit-oak/blob/f05398694fa3bb7b2fb
7feb96e16efc2d97a7e84/oak
-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Multiple
xingDocumentStore.java#L32-L62
Re: Is the hashing of long paths still needed?
Posted by Thomas Mueller <mu...@adobe.com>.
Hi,
The DocumentStore doesn't really know the path, it only knows the key, and
if the key is hashed you can't calculate the path.
There are some options:
(a) Each document that has a hashed path as the key also has a "path"
property (with the real path). You could use that (cache it, read it if
needed, possibly from all backends).
(b) Change the DocumentStore API: add the path in addition to the key.
This is quite some work, and errors could be introduced here (the wrong
path is passed and so on).
Regards,
Thomas
On 21/08/15 11:57, "Bertrand Delacretaz" <bd...@apache.org> wrote:
>Hi,
>
>Continuing to play with Robert's MultiplexingDocumentStore [1] I got a
>failure in the oak-jcr module's LongPathTest.
>
>That's due to the conversion of long paths to their hashed variants -
>those cannot be used to locate the appropriate DocumentStore when
>multiplexed, as that decision is based on the real path. In some cases
>like UpdateOp the real path is saved alongside the hashed one, but not
>always.
>
>Removing the path hashing as shown in the below patch removes that
>problem, and all oak-core tests pass with this change.
>
>Is that hashing still needed, or was that created due to backend
>limitations which are gone now?
>
>If still needed, could it be made configurable so that users can make
>their own tradeoff (MultiplexingDocumentStore vs. supporting backends
>that require that), or are there other constraints?
>
>-Bertrand
>
>[1] https://github.com/bdelacretaz/jackrabbit-oak/tree/bertrand-multiplex
>
>
>
>diff --git
>a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tils.java
>b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tils.java
>index 4577c4b..d97ae76 100644
>---
>a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tils.java
>+++
>b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tils.java
>@@ -254,6 +254,7 @@ public class Utils {
> }
>
> public static String getIdFromPath(String path) {
>+ /*
> if (isLongPath(path)) {
> MessageDigest digest;
> try {
>@@ -267,6 +268,7 @@ public class Utils {
> String name = PathUtils.getName(path);
> return depth + ":h" + Hex.encodeHexString(hash) + "/" + name;
> }
>+ */
> int depth = Utils.pathDepth(path);
> return depth + ":" + path;
> }
>diff --git
>a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tilsTest.java
>b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tilsTest.java
>index b9f619e..5ed1a95 100644
>---
>a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tilsTest.java
>+++
>b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/util/U
>tilsTest.java
>@@ -32,6 +32,7 @@ import org.junit.Test;
>
> import static org.junit.Assert.assertEquals;
> import static org.junit.Assert.assertNull;
>+import static org.junit.Assert.assertNotNull;
> import static org.junit.Assert.assertSame;
> import static org.junit.Assert.assertTrue;
>
>@@ -62,7 +63,8 @@ public class UtilsTest {
> String longPath = PathUtils.concat("/"+Strings.repeat("p",
>Utils.PATH_LONG + 1), "foo");
> assertTrue(Utils.isLongPath(longPath));
>
>- assertNull(Utils.getParentId(Utils.getIdFromPath(longPath)));
>+ // updated to match the changes to Utils.getIdFromPath
>+ assertNotNull(Utils.getParentId(Utils.getIdFromPath(longPath)));
>
> assertNull(Utils.getParentId(Utils.getIdFromPath("/")));
> assertEquals("1:/foo",Utils.getParentId("2:/foo/bar"));