OAK-9668 Update H2DB dependency #466

thomasmueller · 2022-01-17T14:31:30Z

binarySearch: This is a bit ugly: I copied that from the H2 base class. An option would be to use the base class directly, but this would likely require a bigger change.
Changing maxBlobSize from 2 MB to 1 MB: this is a change in H2, to avoid running out of memory I think. The reason is that PreparedStatement.setBytes is used with the byte array.
Cache test: It looks like there was a change in the behavior after Thread.interrupt. I don't think this test is needed any longer, as I don't think we have custom code calling Thread.interrupt any longer.

reschke · 2022-01-17T14:38:19Z

...document/src/test/java/org/apache/jackrabbit/oak/plugins/document/blob/RDBBlobStoreTest.java

@@ -146,7 +146,7 @@ public void testBigBlob() throws Exception {

        LOG.info("max blob length for " + blobStoreName + " was " + test);

-        int expected = Math.max(blobStore.getBlockSize(), 2 * 1024 * 1024);
+        int expected = Math.max(blobStore.getBlockSize(), 1 * 1024 * 1024);


I don't recall whether RDBBlobStore requires bigger blobs. In which case, changing the test here would hide regressions with other databases. It might be better to disable this test case just for h2db.

I agree with @reschke. This seems to be a limitation of the newer H2 version. The default block size of AbstractBlobStore is 2MB. I assume that's why we have this test.

jackrabbit-oak/oak-blob/src/main/java/org/apache/jackrabbit/oak/spi/blob/AbstractBlobStore.java

Line 120 in c6ddcc5

private int blockSize = 2 * 1024 * 1024;

Yes it's a limitation of a new version of H2... An in-memory object contains the array of bytes, and the size of such objects is now limited to 1 MB (I didn't know btw... just read that in the source code)

Why does the test consider blobStore.getBlockSize() when it is set to 128 in setUp()?

I reverted this change in 4f76f50. Instead the test is skipped when it runs on H2.

I honestly do not recall.

mreutegg · 2022-01-17T15:33:16Z

The PR currently does not update the H2 dependency. I'll do that in a minute...

Update dependency to 2.0.206

mreutegg · 2022-01-17T16:10:02Z

oak-pojosr has a test failure with:

Caused by: org.h2.jdbc.JdbcSQLDataException: Value too long for column "BINARY VARYING": "3b4e0d35d7bda3579118fed83f2c7ba06fbd1c606a6dede074c9b2a980fa84d78613cdc1774af692... (2097152)" [22001-206]
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:525)
	at org.h2.message.DbException.getJdbcSQLException(DbException.java:496)
	at org.h2.message.DbException.get(DbException.java:227)
	at org.h2.message.DbException.getValueTooLongException(DbException.java:341)
	at org.h2.value.ValueVarbinary.<init>(ValueVarbinary.java:34)
	at org.h2.value.ValueVarbinary.getNoCopy(ValueVarbinary.java:65)
	at org.h2.value.ValueVarbinary.get(ValueVarbinary.java:51)
	at org.h2.jdbc.JdbcPreparedStatement.setBytes(JdbcPreparedStatement.java:1001)
	at org.apache.jackrabbit.oak.plugins.document.rdb.RDBBlobStore.storeBlockInDatabase(RDBBlobStore.java:334)

Looks like 1MB is the limit for BINARY VARYING: https://h2database.com/html/advanced.html#limits_limitations

stefan-egli · 2022-01-17T16:25:20Z

binarySearch: This is a bit ugly:

would it make sense to move cast and binarySearch to statics? Would at least remove the duplication..

I've done that now in d08bfc0 - but feel free to revert that

Use 1MB block size for test with RDBBlobStore on H2

mreutegg · 2022-01-17T16:56:44Z

oak-pojosr has a test failure with:

This should be fixed with b015851. The test sets the block size to 1MB.

thomasmueller

Look good to me now.

Revert change of expected size. Disable test instead on H2

stefan-egli · 2022-01-18T12:57:37Z

PS: trying to reproduce the test locally but can't so far. rerunning the test now to see if it is flaky

stefan-egli · 2022-01-18T14:01:46Z

run 6 succeeded - so it could be a potential flaky test.
started a run 7 to get another data point

stefan-egli · 2022-01-18T14:55:08Z

oak-store-document that failed in run 5 succeeded in run 6 and 7 - so it does look like a flaky test. the failure is:

[ERROR] invalidateAfterSelfRecovery[RDBFixture: RDB-H2(file)](org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreSweepIT)  Time elapsed: 0.063 s  <<< ERROR!
org.apache.jackrabbit.oak.plugins.document.DocumentStoreException: Configured cluster node id 1 already in use: needs recovery and machineId/instanceId do not match: mac:0ad0cf3a6cc0//home/jenkins/jenkins-agent/workspace/Jackrabbit_oak-trunk-pr_PR-466/oak-store-document != mac:2a990ba80ef4//home/jenkins/jenkins-agent/workspace/Jackrabbit_oak-trunk-pr_PR-466/oak-store-document
	at org.apache.jackrabbit.oak.plugins.document.ClusterNodeInfo.createInstance(ClusterNodeInfo.java:629)
	at org.apache.jackrabbit.oak.plugins.document.ClusterNodeInfo.getInstance(ClusterNodeInfo.java:471)
	at org.apache.jackrabbit.oak.plugins.document.DocumentNodeStore.<init>(DocumentNodeStore.java:582)
	at org.apache.jackrabbit.oak.plugins.document.DocumentNodeStoreBuilder.build(DocumentNodeStoreBuilder.java:168)
	at org.apache.jackrabbit.oak.plugins.document.DocumentMK$Builder.getNodeStore(DocumentMK.java:478)
	at org.apache.jackrabbit.oak.plugins.document.AbstractTwoNodeTest.setUp(AbstractTwoNodeTest.java:107)

which looks like a test race condition between the previous test case and invalidateAfterSelfRecovery.

It's not exactly clear to me why it fails though .. but I don't consider this as a blocker for this PR

stefan-egli

+1 from my side - the test failure of run nr. 5 look flaky

stefan-egli · 2022-01-18T15:01:19Z

any other +1?
@thomasmueller , @reschke ?

* OAK-9668 Update H2DB dependency * OAK-9668: Update H2DB dependency Update dependency to 2.0.206 * OAK-9668 : move cast and binarySearch to DataTypeUtil * OAK-9668: Update H2DB dependency Use 1MB block size for test with RDBBlobStore on H2 * OAK-9668: Update H2DB dependency Revert change of expected size. Disable test instead on H2 Co-authored-by: Marcel Reutegger <marcel.reutegger@gmail.com> Co-authored-by: Stefan Egli <stefanegli@apache.org>

stefan-egli · 2022-01-18T17:50:39Z

PS: #468 is the backport of this to 1.22 branch

* OAK-9668 Update H2DB dependency * OAK-9668: Update H2DB dependency Update dependency to 2.0.206 * OAK-9668 : move cast and binarySearch to DataTypeUtil * OAK-9668: Update H2DB dependency Use 1MB block size for test with RDBBlobStore on H2 * OAK-9668: Update H2DB dependency Revert change of expected size. Disable test instead on H2 Co-authored-by: Marcel Reutegger <marcel.reutegger@gmail.com> Co-authored-by: Stefan Egli <stefanegli@apache.org> Co-authored-by: Thomas Mueller <thomasm@apache.org> Co-authored-by: Marcel Reutegger <marcel.reutegger@gmail.com>

* OAK-9668 Update H2DB dependency * OAK-9668: Update H2DB dependency Update dependency to 2.0.206 * OAK-9668 : move cast and binarySearch to DataTypeUtil * OAK-9668: Update H2DB dependency Use 1MB block size for test with RDBBlobStore on H2 * OAK-9668: Update H2DB dependency Revert change of expected size. Disable test instead on H2 Co-authored-by: Marcel Reutegger <marcel.reutegger@gmail.com> Co-authored-by: Stefan Egli <stefanegli@apache.org> (cherry picked from commit 8c1b628)

OAK-9668 Update H2DB dependency (#466)

OAK-9668 Update H2DB dependency

e6efedf

thomasmueller requested review from reschke, mreutegg, nit0906 and stefan-egli January 17, 2022 14:31

reschke reviewed Jan 17, 2022

View reviewed changes

OAK-9668: Update H2DB dependency

6bd482e

Update dependency to 2.0.206

OAK-9668 : move cast and binarySearch to DataTypeUtil

d08bfc0

OAK-9668: Update H2DB dependency

b015851

Use 1MB block size for test with RDBBlobStore on H2

thomasmueller commented Jan 17, 2022

View reviewed changes

OAK-9668: Update H2DB dependency

4f76f50

Revert change of expected size. Disable test instead on H2

stefan-egli approved these changes Jan 18, 2022

View reviewed changes

stefan-egli merged commit 8c1b628 into trunk Jan 18, 2022

mreutegg added a commit that referenced this pull request Jan 27, 2022

Merge pull request #475 from mreutegg/OAK-9668-1.8

8158428

OAK-9668 Update H2DB dependency (#466)

thomasmueller deleted the OAK-9668 branch August 19, 2022 07:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OAK-9668 Update H2DB dependency #466

OAK-9668 Update H2DB dependency #466

thomasmueller commented Jan 17, 2022

reschke Jan 17, 2022

mreutegg Jan 17, 2022

thomasmueller Jan 17, 2022

mreutegg Jan 17, 2022

mreutegg Jan 17, 2022

reschke Jan 17, 2022

mreutegg commented Jan 17, 2022

mreutegg commented Jan 17, 2022

stefan-egli commented Jan 17, 2022

mreutegg commented Jan 17, 2022

thomasmueller left a comment

stefan-egli commented Jan 18, 2022

stefan-egli commented Jan 18, 2022 •

edited

stefan-egli commented Jan 18, 2022

stefan-egli left a comment

stefan-egli commented Jan 18, 2022

stefan-egli commented Jan 18, 2022

OAK-9668 Update H2DB dependency #466

OAK-9668 Update H2DB dependency #466

Conversation

thomasmueller commented Jan 17, 2022

reschke Jan 17, 2022

Choose a reason for hiding this comment

mreutegg Jan 17, 2022

Choose a reason for hiding this comment

thomasmueller Jan 17, 2022

Choose a reason for hiding this comment

mreutegg Jan 17, 2022

Choose a reason for hiding this comment

mreutegg Jan 17, 2022

Choose a reason for hiding this comment

reschke Jan 17, 2022

Choose a reason for hiding this comment

mreutegg commented Jan 17, 2022

mreutegg commented Jan 17, 2022

stefan-egli commented Jan 17, 2022

mreutegg commented Jan 17, 2022

thomasmueller left a comment

Choose a reason for hiding this comment

stefan-egli commented Jan 18, 2022

stefan-egli commented Jan 18, 2022 • edited

stefan-egli commented Jan 18, 2022

stefan-egli left a comment

Choose a reason for hiding this comment

stefan-egli commented Jan 18, 2022

stefan-egli commented Jan 18, 2022

stefan-egli commented Jan 18, 2022 •

edited