Lazy relations test [cql-tests][tp-tests]

[ntisseyre]

Making Lazy Load relations by default TRUE to validate the build.

This PR is opened only for testing purposes of the #4343 PR. Do not merge this PR.

[li-boxuan]

Great, I can see the ES tests capture an issue.

[porunov]

Really great that you executed this feature across all the test cases and some bugs were discovered.
I see currently some tests failing due to:

CQLGraphTest>JanusGraphTest.testPropertyIdAccessInDifferentTransaction:2647 » IllegalState Any lazy load operation is not supported when transaction is already closed.

I guess it would make sense to execute also tp-tests and extended cql-tests for this feature just to ensure that it works as expected. I'm adding [cql-tests][tp-tests] to the name of the PR, but you will also need to add the same tags to the end of your commit message to trigger full test cases suit for the next build. Notice, usually it takes 4-5 hours to execute all the TP tests using GitHub Actions.

[ntisseyre]

Really great that you executed this feature across all the test cases and some bugs were discovered. I see currently some tests failing due to:

CQLGraphTest>JanusGraphTest.testPropertyIdAccessInDifferentTransaction:2647 » IllegalState Any lazy load operation is not supported when transaction is already closed.

I guess it would make sense to execute also tp-tests and extended cql-tests for this feature just to ensure that it works as expected. I'm adding [cql-tests][tp-tests] to the name of the PR, but you will also need to add the same tags to the end of your commit message to trigger full test cases suit for the next build. Notice, usually it takes 4-5 hours to execute all the TP tests using GitHub Actions.

This error IllegalState Any lazy load operation is not supported when transaction is already closed. is expected. In the dedicated test, i have overwritten the test:
https://github.com/JanusGraph/janusgraph/pull/4343/files#diff-740376f307b1e8385b2e38e7f1ebc4db4fa235dfb838679209a37ca7b3b7d3f6R41

The idea is that you can't access even the relation ID if it was a lazy load and the transaction is closed because, in some cases, to get the relation id, you also need to deserialize the value before.

[porunov]

Really great that you executed this feature across all the test cases and some bugs were discovered. I see currently some tests failing due to:

CQLGraphTest>JanusGraphTest.testPropertyIdAccessInDifferentTransaction:2647 » IllegalState Any lazy load operation is not supported when transaction is already closed.

I guess it would make sense to execute also tp-tests and extended cql-tests for this feature just to ensure that it works as expected. I'm adding [cql-tests][tp-tests] to the name of the PR, but you will also need to add the same tags to the end of your commit message to trigger full test cases suit for the next build. Notice, usually it takes 4-5 hours to execute all the TP tests using GitHub Actions.

This error IllegalState Any lazy load operation is not supported when transaction is already closed. is expected. In the dedicated test, i have overwritten the test: https://github.com/JanusGraph/janusgraph/pull/4343/files#diff-740376f307b1e8385b2e38e7f1ebc4db4fa235dfb838679209a37ca7b3b7d3f6R41

The idea is that you can't access even the relation ID if it was a lazy load and the transaction is closed because, in some cases, to get the relation id, you also need to deserialize the value before.

Got it. Thank you for explanation. Make sense.

[ntisseyre]

Great, I can see the ES tests capture an issue.

@li-boxuan @porunov The tests are now passing, and I was able to catch 2 errors. Thanks!

However, I had to modify 3 tests because the behavior is slightly different now:

1. The missing counts are less because lazy load doesn't do them:
   241949e#diff-b55e47970f42d4d909d653e0e3c978517d6f9830f0a00179a80259ddf21a96c3L321

2. 241949e#diff-d27c4afe468043de57ec91fb1fd1f5ef2ee0acc38fd8689bf1744ff365a2577aL2647
   Now, when accessing even the id of the relation, it throws an exception if the transaction is closed.

3. 241949e#diff-d27c4afe468043de57ec91fb1fd1f5ef2ee0acc38fd8689bf1744ff365a2577aR6291
   In this test scenario it was also closing transaction before doing assertEquals, i forced-loaded values before closing transactoin.

Let me know what are your thoughts around these changes

[porunov]

Closing this PR as the relative PR ( #4343 ) is now merged.