search

JanusGraph / janusgraph-foundationdb

FoundationDB storage adapter for JanusGraph
Other
53 stars 18 forks source link

Handles potential truncated boundary keys #55

Closed ruweih closed 3 years ago

ruweih commented 3 years ago

Boundary keys might not real keys:

https://github.com/apple/foundationdb/issues/3608

We need handle those truncated key, or it will fail in FoundationDB 6.x client:

java.lang.IllegalArgumentException: No terminator found for bytes starting at 1
    at com.apple.foundationdb.tuple.TupleUtil$DecodeState.findNullTerminator(TupleUtil.java:98)
    at com.apple.foundationdb.tuple.TupleUtil.decode(TupleUtil.java:438)
    at com.apple.foundationdb.tuple.TupleUtil.unpack(TupleUtil.java:676)
    at com.apple.foundationdb.tuple.Tuple.fromBytes(Tuple.java:526)
    at com.apple.foundationdb.subspace.Subspace.unpack(Subspace.java:231)
    at org.janusgraph.diskstorage.foundationdb.FoundationDBKeyValueStore.lambda$getBoundaryKeys$1(FoundationDBKeyValueStore.java:264)
    at java.util.Iterator.forEachRemaining(Iterator.java:116)
    at org.janusgraph.diskstorage.foundationdb.FoundationDBKeyValueStore.getBoundaryKeys(FoundationDBKeyValueStore.java:264)
linux-foundation-easycla[bot] commented 3 years ago

CLA Missing ID

mbrukman commented 3 years ago

@ruweih, thank you for your PR!

FYI, you don't have to close a PR and create a new one; you can reuse this one by fixing the CLA issues and using git push -f on your branch to update this PR in-place.

Also, please don't merge the master branch into your branch as that makes for more complex history; instead, please rebase your branch instead which creates a cleaner, linear history:

$ git checkout rangekeys-ruweih
$ git rebase master
$ git push -f

The missing id might be due to using a different email address on one commit than the other one; compare:

Feel free to reopen this PR and continue your work here.

Thanks again!

ruweih commented 3 years ago

@mbrukman Thanks for the advice. I messed up by committing from a machine without proper setup. It's better to just open a new PR, please take a look and provide you feedback:

https://github.com/JanusGraph/janusgraph-foundationdb/pull/56

mbrukman commented 3 years ago

@ruweih — the new PR looks good from the CLA perspective! Let's follow-up on the other PR.

FWIW, even if you don't have the correct git user config when you created the commits, it's possible to fix them in-place, we have some instructions in case this is useful in the future.