failed update partition of topic by gaozhangmin · Pull Request #11683 · apache/pulsar

gaozhangmin · 2021-08-17T10:05:19Z

Motivation

1、tryCreatePartitionsAsync would create znode under managed-ledgers/public/default/persistent/ after success invoke

2、there are chance, updatePartitionedTopic would failed, and managed-ledgers znode would stay unclean. If We tried update partition once again , We would fail, because of managed-ledgers znode exists

pulsar/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java

Lines 473 to 485 in 80171a7

    
           if (numPartitions <= 0) { 
        
               throw new RestException(Status.NOT_ACCEPTABLE, "Number of partitions should be more than 0"); 
        
           } 
        
           try { 
        
               tryCreatePartitionsAsync(numPartitions).get(DEFAULT_OPERATION_TIMEOUT_SEC, TimeUnit.SECONDS); 
        
               updatePartitionedTopic(topicName, numPartitions).get(DEFAULT_OPERATION_TIMEOUT_SEC, TimeUnit.SECONDS); 
        
           } catch (Exception e) { 
        
               if (e.getCause() instanceof RestException) { 
        
                   throw (RestException) e.getCause(); 
        
               } 
        
               log.error("[{}] Failed to update partitioned topic {}", clientAppId(), topicName, e); 
        
               throw new RestException(e); 
        
           }

Modifications

clean up managed-ledger znode after we failed update partition.

Anonymitaet · 2021-08-18T01:19:21Z

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)

gaozhangmin · 2021-08-18T02:01:41Z

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)

no need doc.

BewareMyPower · 2021-08-18T07:27:45Z

Please fix the code style.

Error:  src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:[3641,21] (whitespace) WhitespaceAround: 'for' is not followed by whitespace.
Error:  src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:[3642] (sizes) LineLength: Line is longer than 120 characters (found 147).

gaozhangmin · 2021-08-18T10:07:38Z

@eolivelli @lhotari PTAL

gaozhangmin · 2021-08-27T09:19:01Z

@codelipenghui PTAL

Fixes #11682 ### Motivation 1、`tryCreatePartitionsAsync` would create znode under `managed-ledgers/public/default/persistent/` after success invoke 2、there are chance, `updatePartitionedTopic` would failed, and managed-ledgers znode would stay unclean. If We tried update partition once again , We would fail, because of managed-ledgers znode exists https://github.com/apache/pulsar/blob/80171a733ab4799f912a8935f03c19554b9ca3b1/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L473-L485 ### Modifications clean up managed-ledger znode after we failed update partition. (cherry picked from commit 90c7db4)

Fixes apache#11682 ### Motivation 1、`tryCreatePartitionsAsync` would create znode under `managed-ledgers/public/default/persistent/` after success invoke 2、there are chance, `updatePartitionedTopic` would failed, and managed-ledgers znode would stay unclean. If We tried update partition once again , We would fail, because of managed-ledgers znode exists https://github.com/apache/pulsar/blob/80171a733ab4799f912a8935f03c19554b9ca3b1/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L473-L485 ### Modifications clean up managed-ledger znode after we failed update partition. (cherry picked from commit 90c7db4) (cherry picked from commit f96f967)

Fixes apache#11682 ### Motivation 1、`tryCreatePartitionsAsync` would create znode under `managed-ledgers/public/default/persistent/` after success invoke 2、there are chance, `updatePartitionedTopic` would failed, and managed-ledgers znode would stay unclean. If We tried update partition once again , We would fail, because of managed-ledgers znode exists https://github.com/apache/pulsar/blob/80171a733ab4799f912a8935f03c19554b9ca3b1/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L473-L485 ### Modifications clean up managed-ledger znode after we failed update partition.

clean zk managed-ledgers after failed update partition

04e2a1e

BewareMyPower requested review from codelipenghui, eolivelli and lhotari and removed request for codelipenghui and eolivelli August 17, 2021 10:16

BewareMyPower assigned gaozhangmin Aug 17, 2021

BewareMyPower requested changes Aug 17, 2021

View reviewed changes

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated

BewareMyPower added area/broker area/admin labels Aug 17, 2021

BewareMyPower reviewed Aug 17, 2021

View reviewed changes

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated

BewareMyPower reviewed Aug 17, 2021

View reviewed changes

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated

BewareMyPower requested review from hangc0276 and merlimat August 17, 2021 10:30

gaozhangmin force-pushed the fix-updatePartitionedTopic-bug branch from 58e6675 to 49c6cdd Compare August 18, 2021 05:35

support async method

bc771fe

gaozhangmin force-pushed the fix-updatePartitionedTopic-bug branch from 49c6cdd to bc771fe Compare August 18, 2021 05:36

gaozhangmin requested a review from BewareMyPower August 18, 2021 05:39

BewareMyPower reviewed Aug 18, 2021

View reviewed changes

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated

fix code style

8dc16b7

BewareMyPower requested changes Aug 18, 2021

View reviewed changes

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated

gaozhangmin force-pushed the fix-updatePartitionedTopic-bug branch from 0fa1c93 to 8dc16b7 Compare August 18, 2021 07:47

gavingaozhangmin added 2 commits August 18, 2021 15:59

add error log delete when failed to znode

38c9ddd

fix code style

ddee12f

gaozhangmin requested a review from BewareMyPower August 18, 2021 08:18

BewareMyPower approved these changes Aug 18, 2021

View reviewed changes

Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Aug 19, 2021

codelipenghui requested changes Aug 19, 2021

View reviewed changes

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated

Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java Outdated

codelipenghui added this to the 2.9.0 milestone Aug 19, 2021

codelipenghui added the release/2.8.2 label Aug 19, 2021

fix log level to warn and delete repeated complete future

16f55e8

gaozhangmin requested a review from codelipenghui August 19, 2021 05:52

gaozhangmin requested a review from BewareMyPower August 27, 2021 09:16

BewareMyPower approved these changes Aug 30, 2021

View reviewed changes

codelipenghui approved these changes Aug 30, 2021

View reviewed changes

codelipenghui merged commit 90c7db4 into apache:master Aug 30, 2021

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

TakaHiR07 mentioned this pull request Apr 19, 2022

[fix][broker] Fix update topic partitions failed #15217

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

failed update partition of topic - #11683

failed update partition of topic#11683
codelipenghui merged 6 commits into
apache:masterfrom
gaozhangmin:fix-updatePartitionedTopic-bug

gaozhangmin commented Aug 17, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Anonymitaet commented Aug 18, 2021

Uh oh!

gaozhangmin commented Aug 18, 2021

Uh oh!

BewareMyPower commented Aug 18, 2021

Uh oh!

Uh oh!

Uh oh!

gaozhangmin commented Aug 18, 2021

Uh oh!

Uh oh!

Uh oh!

gaozhangmin commented Aug 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	if (numPartitions <= 0) {
	throw new RestException(Status.NOT_ACCEPTABLE, "Number of partitions should be more than 0");
	}
	try {
	tryCreatePartitionsAsync(numPartitions).get(DEFAULT_OPERATION_TIMEOUT_SEC, TimeUnit.SECONDS);
	updatePartitionedTopic(topicName, numPartitions).get(DEFAULT_OPERATION_TIMEOUT_SEC, TimeUnit.SECONDS);
	} catch (Exception e) {
	if (e.getCause() instanceof RestException) {
	throw (RestException) e.getCause();
	}
	log.error("[{}] Failed to update partitioned topic {}", clientAppId(), topicName, e);
	throw new RestException(e);
	}

Uh oh!

Conversation

gaozhangmin commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Anonymitaet commented Aug 18, 2021

Uh oh!

gaozhangmin commented Aug 18, 2021

Uh oh!

BewareMyPower commented Aug 18, 2021

Uh oh!

Uh oh!

Uh oh!

gaozhangmin commented Aug 18, 2021

Uh oh!

Uh oh!

Uh oh!

gaozhangmin commented Aug 27, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gaozhangmin commented Aug 17, 2021 •

edited

Loading