[HUDI-5993] Connection leak for lock provider by xicm · Pull Request #8304 · apache/hudi

xicm · 2023-03-28T02:37:30Z

Change Logs

Enable occ with ZookeeperBasedLockProvider, when the job runs for a while, it throws a CuratorConnectionLossException. And I find Too many connections in zk log.

23/03/27 11:23:54 ERROR ConnectionState: Connection timed out for connection string (10.19.36.89) and timeout (15000) / elapsed (33970)
org.apache.curator.CuratorConnectionLossException: KeeperErrorCode = ConnectionLoss
	at org.apache.curator.ConnectionState.checkTimeouts(ConnectionState.java:225)
	at org.apache.curator.ConnectionState.getZooKeeper(ConnectionState.java:94)
	at org.apache.curator.CuratorZookeeperClient.getZooKeeper(CuratorZookeeperClient.java:117)
	at org.apache.curator.framework.imps.CuratorFrameworkImpl.getZooKeeper(CuratorFrameworkImpl.java:489)
	at org.apache.curator.framework.imps.CreateBuilderImpl$12.call(CreateBuilderImpl.java:786)
	at org.apache.curator.framework.imps.CreateBuilderImpl$12.call(CreateBuilderImpl.java:778)
	at org.apache.curator.RetryLoop.callWithRetry(RetryLoop.java:109)
	at org.apache.curator.framework.imps.CreateBuilderImpl.findProtectedNodeInForeground(CreateBuilderImpl.java:775)
	at org.apache.curator.framework.imps.CreateBuilderImpl.access$1200(CreateBuilderImpl.java:44)
	at org.apache.curator.framework.imps.CreateBuilderImpl$11.call(CreateBuilderImpl.java:733)
	at org.apache.curator.framework.imps.CreateBuilderImpl$11.call(CreateBuilderImpl.java:723)
	at org.apache.curator.RetryLoop.callWithRetry(RetryLoop.java:109)
	at org.apache.curator.framework.imps.CreateBuilderImpl.pathInForeground(CreateBuilderImpl.java:720)
	at org.apache.curator.framework.imps.CreateBuilderImpl.protectedPathInForeground(CreateBuilderImpl.java:484)
	at org.apache.curator.framework.imps.CreateBuilderImpl.forPath(CreateBuilderImpl.java:474)
	at org.apache.curator.framework.imps.CreateBuilderImpl.forPath(CreateBuilderImpl.java:454)
	at org.apache.curator.framework.imps.CreateBuilderImpl.forPath(CreateBuilderImpl.java:44)
	at org.apache.curator.framework.recipes.locks.StandardLockInternalsDriver.createsTheLock(StandardLockInternalsDriver.java:54)
	at org.apache.curator.framework.recipes.locks.LockInternals.attemptLock(LockInternals.java:217)
	at org.apache.curator.framework.recipes.locks.InterProcessMutex.internalLock(InterProcessMutex.java:232)
	at org.apache.curator.framework.recipes.locks.InterProcessMutex.acquire(InterProcessMutex.java:108)
	at org.apache.hudi.client.transaction.lock.ZookeeperBasedLockProvider.acquireLock(ZookeeperBasedLockProvider.java:144)
	at org.apache.hudi.client.transaction.lock.ZookeeperBasedLockProvider.tryLock(ZookeeperBasedLockProvider.java:96)
	at org.apache.hudi.client.transaction.lock.LockManager.lock(LockManager.java:78)
	at org.apache.hudi.client.transaction.TransactionManager.beginTransaction(TransactionManager.java:59)
	at org.apache.hudi.client.HoodieTimelineArchiver.archiveIfRequired(HoodieTimelineArchiver.java:168)
	at org.apache.hudi.client.BaseHoodieTableServiceClient.archive(BaseHoodieTableServiceClient.java:577)
	at org.apache.hudi.client.BaseHoodieWriteClient.archive(BaseHoodieWriteClient.java:784)
	at org.apache.hudi.client.BaseHoodieWriteClient.autoArchiveOnCommit(BaseHoodieWriteClient.java:574)
	at org.apache.hudi.client.BaseHoodieWriteClient.mayBeCleanAndArchive(BaseHoodieWriteClient.java:540)
	at org.apache.hudi.client.BaseHoodieWriteClient.commitStats(BaseHoodieWriteClient.java:247)
	at org.apache.hudi.client.SparkRDDWriteClient.commit(SparkRDDWriteClient.java:103)
	at org.apache.hudi.HoodieSparkSqlWriter$.commitAndPerformPostOperations(HoodieSparkSqlWriter.scala:960)
	at org.apache.hudi.HoodieSparkSqlWriter$.write(HoodieSparkSqlWriter.scala:377)
	at org.apache.hudi.HoodieStreamingSink.$anonfun$addBatch$2(HoodieStreamingSink.scala:122)
	at scala.util.Try$.apply(Try.scala:213)
	at org.apache.hudi.HoodieStreamingSink.$anonfun$addBatch$1(HoodieStreamingSink.scala:120)
	at org.apache.hudi.HoodieStreamingSink.retry(HoodieStreamingSink.scala:244)
	at org.apache.hudi.HoodieStreamingSink.addBatch(HoodieStreamingSink.scala:119)

This pr calls TransactionManager.close after endTransaction

Impact

none

Risk level (write none, low medium or high below)

low

Documentation Update

Describe any necessary documentation update if there is any new feature, config, or user-facing change

The config description must be updated if new configs are added or the default value of the configs are changed
Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the instruction to make
changes to the website.

Contributor's checklist

Read through contributor's guide
Change Logs and Impact were stated clearly
Adequate tests were added if applicable
CI passed

danny0405 · 2023-03-28T05:11:50Z

...nt/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/LockManager.java

    if (writeConfig.getWriteConcurrencyMode().supportsOptimisticConcurrencyControl()) {
      getLockProvider().unlock();
      metrics.updateLockHeldTimerMetrics();
+      close();


Nice catch, can we write a UT to guard this logic?

Is this because we reuse the client ?

We new many txn managers for each fresh new txn, like in all kinds of action executors, we execute a beginTransaction and endTransaction for each of the txn but missing to call the #close explicitly.

Of course, it depends on whether we want to keep the long live connection or short live connection, if we fix the code like this patch does, the connection becomes short lived.

A long lived connection is more risky for connection leaks, we need to care about the life cycle of the connection, we need to consider resuing the existing connections, which makes the things more complicated and hard to keep correctness.

@vinothchandar : yes you are right. for streaming ingestion, we keep re-using the same write client and hence.

danny0405 · 2023-03-29T03:08:05Z

...ent/hudi-client-common/src/test/java/org/apache/hudi/client/transaction/TestLockManager.java

+    assertNotNull(lockProvider.get(lockManager));
+
+    assertDoesNotThrow(() -> {
+      lockManager.unlock();


We can use the mockito to check the #close method invoke instead, take TestCloudWatchMetricsReporter for an example.

nsivabalan · 2023-03-29T06:00:56Z

LGTM.

hudi-bot · 2023-03-29T10:21:09Z

CI report:

c3eb7ee UNKNOWN
0f86c1f Azure: SUCCESS

Bot commands

@hudi-bot supports the following commands:

@hudi-bot run azure re-run the last Azure build

* close lockProvider when we do unlock

danny0405 changed the title ~~[minor]call TransactionManager.close after endTransaction~~ [HUDI-5993] Connection leak for lock provider Mar 28, 2023

close lockProvider when we do unlock

c2efad7

xicm force-pushed the xicm_transaction branch from c3eb7ee to c2efad7 Compare March 28, 2023 03:19

danny0405 self-assigned this Mar 28, 2023

danny0405 added concurrency-control labels Mar 28, 2023

danny0405 reviewed Mar 28, 2023

View reviewed changes

add ut

5879764

danny0405 reviewed Mar 29, 2023

View reviewed changes

fix ut

0f86c1f

danny0405 approved these changes Mar 30, 2023

View reviewed changes

danny0405 merged commit f076c0a into apache:master Mar 30, 2023

nsivabalan pushed a commit to nsivabalan/hudi that referenced this pull request Mar 30, 2023

[HUDI-5993] Connection leak for lock provider (apache#8304)

3f43299

* close lockProvider when we do unlock

nsivabalan pushed a commit to nsivabalan/hudi that referenced this pull request Mar 31, 2023

[HUDI-5993] Connection leak for lock provider (apache#8304)

2f2c0f9

* close lockProvider when we do unlock

fengjian428 pushed a commit to fengjian428/hudi that referenced this pull request Apr 5, 2023

[HUDI-5993] Connection leak for lock provider (apache#8304)

fad33fb

* close lockProvider when we do unlock

stayrascal pushed a commit to stayrascal/hudi that referenced this pull request Apr 20, 2023

[HUDI-5993] Connection leak for lock provider (apache#8304)

3da504f

* close lockProvider when we do unlock

KnightChess pushed a commit to KnightChess/hudi that referenced this pull request Jan 2, 2024

[HUDI-5993] Connection leak for lock provider (apache#8304)

ae64b5e

* close lockProvider when we do unlock

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

[HUDI-5993] Connection leak for lock provider#8304

[HUDI-5993] Connection leak for lock provider#8304
danny0405 merged 3 commits intoapache:masterfrom
xicm:xicm_transaction

xicm commented Mar 28, 2023 •

edited

Loading

Uh oh!

danny0405 Mar 28, 2023

Uh oh!

vinothchandar Mar 28, 2023

Uh oh!

danny0405 Mar 28, 2023

Uh oh!

danny0405 Mar 28, 2023 •

edited

Loading

Uh oh!

nsivabalan Mar 29, 2023

Uh oh!

danny0405 Mar 29, 2023 •

edited

Loading

Uh oh!

nsivabalan Mar 29, 2023

Uh oh!

nsivabalan commented Mar 29, 2023

Uh oh!

hudi-bot commented Mar 29, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Comments

Conversation

xicm commented Mar 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Logs

Impact

Risk level (write none, low medium or high below)

Documentation Update

Contributor's checklist

Uh oh!

danny0405 Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

vinothchandar Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

danny0405 Mar 28, 2023

Choose a reason for hiding this comment

Uh oh!

danny0405 Mar 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nsivabalan Mar 29, 2023

Choose a reason for hiding this comment

Uh oh!

danny0405 Mar 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nsivabalan Mar 29, 2023

Choose a reason for hiding this comment

Uh oh!

nsivabalan commented Mar 29, 2023

Uh oh!

hudi-bot commented Mar 29, 2023

CI report:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xicm commented Mar 28, 2023 •

edited

Loading

danny0405 Mar 28, 2023 •

edited

Loading

danny0405 Mar 29, 2023 •

edited

Loading