orientechnologies / orientdb-gremlin

TinkerPop3 Graph Structure Implementation for OrientDB
Apache License 2.0
91 stars 32 forks source link

Bump orient version to 2.2 #74

Closed velo closed 8 years ago

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-7.2%) to 73.909% when pulling c93dbf70db07f2f636a9a865234235c4c9b10b7d on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

velo commented 8 years ago

@mpollmeier hey, do you have the means to summon luca here?

mpollmeier commented 8 years ago

On travis it fails with an OutOfMemory error, or is there anything else I missed?

Running org.apache.tinkerpop.gremlin.orientdb.gremlintest.structure.OrientGraphStructureStandardIT
Exception in thread "Thread-8" java.lang.OutOfMemoryError: Java heap space

That's something we should be able to fix, probably with a change in .travis.yml?

I just ran mvn install locally and only get one error which looks easy to fix:

searchMatching(org.apache.tinkerpop.gremlin.orientdb.MatcherStepTest)  Time elapsed: 0.569 sec  <<< FAILURE!
java.lang.AssertionError:
Expected: (map containing ["a"->"josh"] and map containing ["c"->"marko"])
     but: map containing ["a"->"josh"] map was [<a=peter>, <c=marko>]

Just mention luca if you have a question. He's very happy about our little driver project here and typically responds quickly.

velo commented 8 years ago

Yes, it does... but on my machine it fails as well.... odd!

@lvca so, should we just give orient 2.2 more memory or this is something we should investigate a little big further?

velo commented 8 years ago

I just realized that we give tests 2Gb to run

https://github.com/velo/orientdb-gremlin/blob/c93dbf70db07f2f636a9a865234235c4c9b10b7d/driver/pom.xml#L162

wildroco commented 8 years ago

I don't know it is helpful, but I ran 'mvn install' and got same result with mpollmeier.


Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.928 sec <<< FAILURE! searchMatching(org.apache.tinkerpop.gremlin.orientdb.MatcherStepTest) Time elapsed: 0.573 sec <<< FAILURE! java.lang.AssertionError: Expected: (map containing ["a"->"josh"] and map containing ["c"->"marko"]) but: map containing ["a"->"josh"] map was [<a=peter>, <c=marko>] at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20) ...

velo commented 8 years ago

Ya, same OOM issue

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-7.2%) to 73.909% when pulling 2454a3f5873da1fbed88e3a9db75323e043b040b on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-9.6%) to 71.528% when pulling 1e3158d22a87422d2dea532fd1233dfc1b86bde3 on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-9.6%) to 71.528% when pulling 827ea21a7ba2e303aa8131e1183f43a0a4f8e4aa on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

lvca commented 8 years ago

Sorry for the delay, I'm on it.

lvca commented 8 years ago

I've added in the execution setting this new one that is needed from OrientDB v2.2: -XX:MaxDirectMemorySize=512g, otherwise the JVM wont allows to use big space in direct memory.

velo commented 8 years ago

Hrmmm, i don't think I have a machine capable of running it :/

lvca commented 8 years ago

512g is only an upper limit, I think 2G for the test is enough.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-5.5%) to 75.644% when pulling 11733b7907c757d67b7f625636e52251bf617117 on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

velo commented 8 years ago

@lvca no sucess =/

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-5.5%) to 75.641% when pulling d0dd7313b24bda28ced7c03460e20d7d508fde57 on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-5.7%) to 75.444% when pulling 0f863090b60ac6e270217c9dc30f0110d49d958e on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-6.003%) to 75.148% when pulling 0f863090b60ac6e270217c9dc30f0110d49d958e on velo:orient-2.2 into 36d80f6a609cd3285a560171b9ad77a681a0f402 on mpollmeier:master.

wildroco commented 8 years ago

@lvca Could you please help this work? I really really want to see this driver bumped to OrientDB 2.2!

lvca commented 8 years ago

Sorry guys, I'm clearly the bottleneck on this. I'm forwarding this to somebody else of the OrientDB team.

velo commented 8 years ago

that would be nice @lvca

I ran out of ideas.... no matter what I do there is not enough memory!

lvca commented 8 years ago

@tglman and @maggiolo00 from OrientDB Team are looking into it.

wolf4ood commented 8 years ago

Hi @velo

let me try to reproduce it

wolf4ood commented 8 years ago

@velo

Do you run the tests on a Windows machine?

velo commented 8 years ago

Yes, but the CI is Linux and also fails

On Tue, 6 Sep 2016, 22:01 Enrico Risa notifications@github.com wrote:

@velo https://github.com/velo

Do you run the tests on Windows machine?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mpollmeier/orientdb-gremlin/pull/74#issuecomment-244905590, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIVjksxzkmur7qgJNCKeAed4PUFSAkUks5qnTnlgaJpZM4Ik0Nr .

wolf4ood commented 8 years ago

Now seems stuck

let me check

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-9.07%) to 75.148% when pulling fcde1801b8b83306ef490611ed72faad26b431a9 on velo:orient-2.2 into e20a525b64f7b916492875643730aaaa5e6594ea on mpollmeier:master.

wolf4ood commented 8 years ago

@velo

i did try with 512g for Max memory and worked for me with 1 test fail but without OOM

XX:MaxDirectMemorySize=512g

can you try?

velo commented 8 years ago

Wow, I don't think I have access to this kind of hardware.

Also, this seems to be a serious memory leak on 2.2 series. I can run 2.1 tests w/o 512g of memory

On Wed, 7 Sep 2016, 02:11 Enrico Risa notifications@github.com wrote:

@velo https://github.com/velo

i did try with 512g for Max memory and worked for me with 1 test fail but without OOM

XX:MaxDirectMemorySize=512g

can you try?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mpollmeier/orientdb-gremlin/pull/74#issuecomment-244962281, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIVjn0awgQXQG236nL9O4ZMlBnX-Nzbks5qnXSMgaJpZM4Ik0Nr .

wolf4ood commented 8 years ago

@velo

as @lvca explain, this is only an upper limit. OrientDB does not require that in order to run I did run the tests on my mac with such config

mpollmeier commented 8 years ago

we can try it for debugging purposes (@velo please go ahead)

but I agree with @velo that it's not a good idea to actually set the limit so high. it does look like a memory leak, and if orient actually runs anywhere near that value it will slow down or even blow up. if it actually needs that much in our tests you should really take a heap dump and fix the problem.

wolf4ood commented 8 years ago

@mpollmeier

i agree 2G should be fine, i think i may have found something, i will keep you updated

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-8.9%) to 75.345% when pulling 237417eba90f213ca11e5da98b39a9437cf0cacf on velo:orient-2.2 into e20a525b64f7b916492875643730aaaa5e6594ea on mpollmeier:master.

velo commented 8 years ago

Tests passed on my computer using this humongous amount of memory... It did allocated 10Gb (o.0)

Still that failed on CI

wolf4ood commented 8 years ago

@velo

I've made this PR that implements the graph.drop.

I've used it in the Test Suite so the storages used in tests are removed in teardown. You can set it back the -XX:MaxDirectMemorySize=2g paramenter

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 84.071% when pulling 9c86fb754bf607cd8711d8402d34ef46d0b5ef3f on velo:orient-2.2 into 0c81184bc86a1527be799e379274e5b467937f1e on mpollmeier:master.

velo commented 8 years ago

@mpollmeier any idea why the reconnect test is fail on linux but passing on windows?!

velo commented 8 years ago

NVM, fails everywhere =D

wolf4ood commented 8 years ago

@velo

you did remove entirely the -XX:MaxDirectMemorySize JVM settings.

Use it with 2g -XX:MaxDirectMemorySize=2g

velo commented 8 years ago

There is no need, tests are passing now, thanks for the assistance!

On Thu, 8 Sep 2016, 19:44 Enrico Risa notifications@github.com wrote:

@velo https://github.com/velo

you did remove entirely the -XX:MaxDirectMemorySize JVM settings.

Use it with 2g -XX:MaxDirectMemorySize=2g

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mpollmeier/orientdb-gremlin/pull/74#issuecomment-245519122, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIVjq9BdcfbE61AovAmVbOS949lTAzzks5qn7zBgaJpZM4Ik0Nr .

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 84.071% when pulling b16f2e439f9c088fd6125d6ce23da1df6da9eabd on velo:orient-2.2 into dfff9a94004d6fed17d8d0f61006b8c296f2ddb5 on mpollmeier:master.

mpollmeier commented 8 years ago

OrientReconnect test is failing because the graph instance behaviour changed - it's returning cached results instead of failing when the underlying connection drops. Will provide a fix shortly and merge everything to master.

mpollmeier commented 8 years ago

just released 3.2.1.1 which upgrades to OrientDB 2.2 :)