HIVE-28839: Connection starvation in HMS if datanucleus value generation fails

[zabetak]

What changes were proposed in this pull request and why?

1. Upgrade to datanucleus-rdbms version 5.2.13 that contains the fix for the connection leak (SQLException during value generation leads to (non transactional) connection leak datanucleus/datanucleus-rdbms#501).
2. Add a test case reproducing the connection leak by introducing random failures on commit using the jdbc-faulty project.

Does this PR introduce any user-facing change?

Yes, fixes the leak.

How was this patch tested?

mvn clean test -Dtest=TestObjectStoreSecondaryPoolLeak -Dtest.groups=

Before the datanucleus upgrade

The test fails with the following error:

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 40.73 s <<< FAILURE! -- in org.apache.hadoop.hive.metastore.TestObjectStoreSecondaryPoolLeak
[ERROR] org.apache.hadoop.hive.metastore.TestObjectStoreSecondaryPoolLeak.testCreateTableWithRandomTimeoutOnCommit -- Time elapsed: 30.23 s <<< FAILURE!
org.opentest4j.AssertionFailedError: Connection leak during creation of table_28
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:42)
	at org.junit.jupiter.api.Assertions.fail(Assertions.java:150)
	at org.apache.hadoop.hive.metastore.TestObjectStoreSecondaryPoolLeak.testCreateTableWithRandomTimeoutOnCommit(TestObjectStoreSecondaryPoolLeak.java:103)
...

After the datanucleus upgrade

The test passes successfully in ~40 seconds.

[INFO] Running org.apache.hadoop.hive.metastore.TestObjectStoreSecondaryPoolLeak
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 39.89 s -- in org.apache.hadoop.hive.metastore.TestObjectStoreSecondaryPoolLeak
[INFO] 
[INFO] Results:
[INFO] 
[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[dengzhhu653]

can we move this test to a separate PR so it only contains upgrading datanucleus

[zabetak]

Whenever we fix a bug we usually want to have a test that proves that the bug is fixed. Why should we separate the them here?

[dengzhhu653]

The main point is we use a private dependency for this test, I'm not sure if this is a good practice for the project

[zabetak]

There is nothing specific to jdbc-faulty that distinguishes it from other similar dependencies. For instance, HikariCP is maintained principally by one person, datanucleus as well, and possibly many others from those that are listed in the project.

I can understand that there is a burden when new dependencies come to the project but in the end it is a question of pros & cons. If you believe that the test does not add enough value then I am fine to remove it.

[zabetak]

There has been an upgrade to a more recent version of datanucleus (which contains the bug fix) in HIVE-26473. The test-case is still relevant but given that there have been concerns about the new test dependency that are necessary for the PR to land I plan to leave things as is for now thus closing this PR.