Fix update ledger list to znode version mismatch failed, ledger not delete #12015

hangc0276 · 2021-09-12T04:49:33Z

Motivation

When Zookeeper throws Failed to update ledger list. z-node version mismatch. Closing managed ledger exception when update ZNode list, it will not clean up the created ledger, which will lead to the new created ledger not be indexed to the topic managedLedger list, and can't be cleanup as topic retention. What's more, it will cause ZNode number increase in Zookeeper if the z-node version mismatch exception keeping throw out. The exception list as follow:

10:44:29.017 [main-EventThread] INFO  org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Created new ledger 67311140
10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] ERROR org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Failed to update ledger list. z-node version mismatch. Closing managed ledger
10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] INFO  org.apache.pulsar.broker.service.Producer - Disconnecting producer: Producer{topic=PersistentTopic{topic=persistent://test/test/test_v1-partition-4}, client=/10.1.2.3:38938, producerName=pulsar-101-1123, producerId=20}

Modification

When updating ZNode list failed, delete the created ledger from broker cache and BookKeeper, regardless of whether the exception type is BadVersionException or not.

eolivelli · 2021-09-12T08:07:24Z

Good catch

I am not sure that this applies to2.7.x

gaoran10 · 2021-09-12T14:50:49Z

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

+                            return;
+                        }
+                    }
+
                    synchronized (ManagedLedgerImpl.this) {


Could we add the exception check here? We could record the lastLedgerCreationFailureTimestamp.

@gaoran10 I have add record the lastLedgerCreationFailureTimestamp into the exception block. We couldn't merge the exception block here due to the metadataMutex unlock.

315157973 · 2021-09-12T15:54:21Z

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

@@ -1466,6 +1452,20 @@ public void operationFailed(MetaStoreException e) {

                    metadataMutex.unlock();

+                    if (e instanceof BadVersionException) {


Do we need to put this logic before releasing the lock? Now we changed the previous behavior. I'm not sure if it will cause other thread safety issues

Anonymitaet · 2021-09-13T09:10:29Z

@hangc0276 Thanks for your contribution. For this PR, do we need to update docs?

(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

eolivelli · 2021-09-14T06:58:21Z

@Anonymitaet I added "no-need-docs", thanks

eolivelli · 2021-09-20T12:21:42Z

@hangc0276 can you please answer the comments ?
it would be good to see this patch into 2.9.0

…elete

…elete (#12015) ### Motivation When Zookeeper throws `Failed to update ledger list. z-node version mismatch. Closing managed ledger` exception when update ZNode list, it will not clean up the created ledger, which will lead to the new created ledger not be indexed to the topic managedLedger list, and can't be cleanup as topic retention. What's more, it will cause ZNode number increase in Zookeeper if the `z-node version mismatch` exception keeping throw out. The exception list as follow: ``` 10:44:29.017 [main-EventThread] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Created new ledger 67311140 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] ERROR org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Failed to update ledger list. z-node version mismatch. Closing managed ledger 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] INFO org.apache.pulsar.broker.service.Producer - Disconnecting producer: Producer{topic=PersistentTopic{topic=persistent://test/test/test_v1-partition-4}, client=/10.1.2.3:38938, producerName=pulsar-101-1123, producerId=20} ``` ### Modification 1. When updating ZNode list failed, delete the created ledger from broker cache and BookKeeper, regardless of whether the exception type is BadVersionException or not. (cherry picked from commit e7b0e3d)

…elete (apache#12015) ### Motivation When Zookeeper throws `Failed to update ledger list. z-node version mismatch. Closing managed ledger` exception when update ZNode list, it will not clean up the created ledger, which will lead to the new created ledger not be indexed to the topic managedLedger list, and can't be cleanup as topic retention. What's more, it will cause ZNode number increase in Zookeeper if the `z-node version mismatch` exception keeping throw out. The exception list as follow: ``` 10:44:29.017 [main-EventThread] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Created new ledger 67311140 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] ERROR org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Failed to update ledger list. z-node version mismatch. Closing managed ledger 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] INFO org.apache.pulsar.broker.service.Producer - Disconnecting producer: Producer{topic=PersistentTopic{topic=persistent://test/test/test_v1-partition-4}, client=/10.1.2.3:38938, producerName=pulsar-101-1123, producerId=20} ``` ### Modification 1. When updating ZNode list failed, delete the created ledger from broker cache and BookKeeper, regardless of whether the exception type is BadVersionException or not.

…elete (#12015) ### Motivation When Zookeeper throws `Failed to update ledger list. z-node version mismatch. Closing managed ledger` exception when update ZNode list, it will not clean up the created ledger, which will lead to the new created ledger not be indexed to the topic managedLedger list, and can't be cleanup as topic retention. What's more, it will cause ZNode number increase in Zookeeper if the `z-node version mismatch` exception keeping throw out. The exception list as follow: ``` 10:44:29.017 [main-EventThread] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Created new ledger 67311140 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] ERROR org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Failed to update ledger list. z-node version mismatch. Closing managed ledger 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] INFO org.apache.pulsar.broker.service.Producer - Disconnecting producer: Producer{topic=PersistentTopic{topic=persistent://test/test/test_v1-partition-4}, client=/10.1.2.3:38938, producerName=pulsar-101-1123, producerId=20} ``` ### Modification 1. When updating ZNode list failed, delete the created ledger from broker cache and BookKeeper, regardless of whether the exception type is BadVersionException or not. (cherry picked from commit e7b0e3d)

…elete (apache#12015) ### Motivation When Zookeeper throws `Failed to update ledger list. z-node version mismatch. Closing managed ledger` exception when update ZNode list, it will not clean up the created ledger, which will lead to the new created ledger not be indexed to the topic managedLedger list, and can't be cleanup as topic retention. What's more, it will cause ZNode number increase in Zookeeper if the `z-node version mismatch` exception keeping throw out. The exception list as follow: ``` 10:44:29.017 [main-EventThread] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Created new ledger 67311140 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] ERROR org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Failed to update ledger list. z-node version mismatch. Closing managed ledger 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] INFO org.apache.pulsar.broker.service.Producer - Disconnecting producer: Producer{topic=PersistentTopic{topic=persistent://test/test/test_v1-partition-4}, client=/10.1.2.3:38938, producerName=pulsar-101-1123, producerId=20} ``` ### Modification 1. When updating ZNode list failed, delete the created ledger from broker cache and BookKeeper, regardless of whether the exception type is BadVersionException or not.

…elete (#12015) ### Motivation When Zookeeper throws `Failed to update ledger list. z-node version mismatch. Closing managed ledger` exception when update ZNode list, it will not clean up the created ledger, which will lead to the new created ledger not be indexed to the topic managedLedger list, and can't be cleanup as topic retention. What's more, it will cause ZNode number increase in Zookeeper if the `z-node version mismatch` exception keeping throw out. The exception list as follow: ``` 10:44:29.017 [main-EventThread] INFO org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Created new ledger 67311140 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] ERROR org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl - [test/test/persistent/test_v1-partition-4] Failed to update ledger list. z-node version mismatch. Closing managed ledger 10:44:29.018 [bookkeeper-ml-workers-OrderedExecutor-2-0] INFO org.apache.pulsar.broker.service.Producer - Disconnecting producer: Producer{topic=PersistentTopic{topic=persistent://test/test/test_v1-partition-4}, client=/10.1.2.3:38938, producerName=pulsar-101-1123, producerId=20} ``` ### Modification 1. When updating ZNode list failed, delete the created ledger from broker cache and BookKeeper, regardless of whether the exception type is BadVersionException or not. (cherry picked from commit e7b0e3d)

hangc0276 requested review from codelipenghui, jerrypeng, rdhabalia, 315157973, eolivelli and merlimat September 12, 2021 04:49

hangc0276 self-assigned this Sep 12, 2021

hangc0276 added area/broker type/bug The PR fixed a bug or issue reported a bug release/2.7.4 release/2.8.2 labels Sep 12, 2021

hangc0276 added this to the 2.9.0 milestone Sep 12, 2021

gaoran10 reviewed Sep 12, 2021

View reviewed changes

merlimat approved these changes Sep 12, 2021

View reviewed changes

315157973 reviewed Sep 12, 2021

View reviewed changes

eolivelli added the doc-not-needed Your PR changes do not impact docs label Sep 14, 2021

eolivelli modified the milestones: 2.9.0, 2.10.0 Oct 6, 2021

codelipenghui added the release/2.9.1 label Nov 10, 2021

codelipenghui added release/2.8.3 and removed release/2.8.2 labels Nov 21, 2021

hangc0276 added 3 commits November 21, 2021 17:01

Fix update ledger list to znode version mismatch failed, ledger not d…

68b1caf

…elete

format code

353a56e

address comments

bd74c58

hangc0276 force-pushed the chenhang/fix_znode_version_mismatch_ledger_not_delete_bug branch from 2a9b2a1 to bd74c58 Compare November 21, 2021 09:29

hangc0276 requested a review from 315157973 November 21, 2021 09:39

hangc0276 requested a review from gaoran10 November 21, 2021 09:47

315157973 approved these changes Nov 24, 2021

View reviewed changes

hangc0276 added release/2.8.2 and removed release/2.8.3 labels Nov 29, 2021

codelipenghui approved these changes Nov 29, 2021

View reviewed changes

codelipenghui merged commit e7b0e3d into apache:master Nov 29, 2021

codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Nov 29, 2021

codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Dec 11, 2021

eolivelli added release/2.9.2 and removed release/2.9.1 labels Dec 15, 2021

codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix update ledger list to znode version mismatch failed, ledger not delete #12015

Fix update ledger list to znode version mismatch failed, ledger not delete #12015

hangc0276 commented Sep 12, 2021

eolivelli commented Sep 12, 2021

gaoran10 Sep 12, 2021

hangc0276 Nov 21, 2021

315157973 Sep 12, 2021

codelipenghui Nov 10, 2021

hangc0276 Nov 21, 2021

Anonymitaet commented Sep 13, 2021

eolivelli commented Sep 14, 2021

eolivelli commented Sep 20, 2021

		@@ -1466,6 +1452,20 @@ public void operationFailed(MetaStoreException e) {

		metadataMutex.unlock();

		if (e instanceof BadVersionException) {

Fix update ledger list to znode version mismatch failed, ledger not delete #12015

Fix update ledger list to znode version mismatch failed, ledger not delete #12015

Conversation

hangc0276 commented Sep 12, 2021

Motivation

Modification

eolivelli commented Sep 12, 2021

gaoran10 Sep 12, 2021

Choose a reason for hiding this comment

hangc0276 Nov 21, 2021

Choose a reason for hiding this comment

315157973 Sep 12, 2021

Choose a reason for hiding this comment

codelipenghui Nov 10, 2021

Choose a reason for hiding this comment

hangc0276 Nov 21, 2021

Choose a reason for hiding this comment

Anonymitaet commented Sep 13, 2021

eolivelli commented Sep 14, 2021

eolivelli commented Sep 20, 2021