Fix schedule task memory leak in ManagedLedgerFactoryImpl

[hezhangjian]

Motivation

The memory is leak due to the ManagerLedgerImpl can't be garbage collected.

Modifications

Before remove the managedLedgerImpl from map, call close() to cacel the schdule job first.

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

• no-need-doc

Fix a memory leak bug

[hezhangjian]

/pulsarbot run-failure-checks

[lhotari]

The memory is leak due to the ManagerLedgerImpl can't be garbage collected.

Can you share more details about the symptoms? Which Pulsar versions are impacted?

[hezhangjian]

@lhotari
The managedledgerImpl were removed from map, but the schedule tasks didn't stop, So their memory can't be released, leading to a memory leak.

In current branch we maintained, version 2.8 and version 2.9 are impacted.

[hezhangjian]

@codelipenghui @hangc0276 @eolivelli PTAL

[lhotari]

LGTM

[lhotari]

Good catch @Shoothzj

[hangc0276]

Actually, if the managedLedger in Fenced or Closed state, call asyncClose just

pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java

Lines 1316 to 1328 in ee90406

	public synchronized void asyncClose(final CloseCallback callback, final Object ctx) {
	State state = STATE_UPDATER.get(this);
	if (state == State.Fenced) {
	factory.close(this);
	callback.closeFailed(new ManagedLedgerFencedException(), ctx);
	return;
	} else if (state == State.Closed) {
	if (log.isDebugEnabled()) {
	log.debug("[{}] Ignoring request to close a closed managed ledger", name);
	}
	callback.closeComplete(ctx);
	return;
	}

So, should we just call ManagedLedgerFactoryImpl::close ?

[lhotari]

Good point. I guess calling asyncClose is sufficient. However there seems to be a bug in the asyncClose method when the ledger is fenced. The method call to cancel the scheduled tasks is missing.

[lhotari]

+1

[lhotari]

+1

[lhotari]

It seems that similar addition would be needed to asyncClose when the ledger is fenced

[hezhangjian]

Yes, yesterday，I found It has been fixed in #12565, it's better to cancel the schedule task when the ledger is fenced. I will test if it fix the memomry leak.