Skip to content

[branch-2.9][cherry-pick] Close TransactionBuffer when MessageDeduplication#checkStatus failed#19287

Merged
codelipenghui merged 8 commits intoapache:branch-2.9from
tjiuming:fix/chk_dedup_failed
Feb 3, 2023
Merged

[branch-2.9][cherry-pick] Close TransactionBuffer when MessageDeduplication#checkStatus failed#19287
codelipenghui merged 8 commits intoapache:branch-2.9from
tjiuming:fix/chk_dedup_failed

Conversation

@tjiuming
Copy link
Contributor

Motivation

Cherry-pick #19157 into branch-2.9

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@codelipenghui
Copy link
Contributor

@tjiuming Could you please help check the failed test? It looks like introduced by this PR.

@tjiuming
Copy link
Contributor Author

/pulsarbot run-failure-checks

}

@VisibleForTesting
public void createPersistentTopic0(final String topic, boolean createIfMissing,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why don't use BrokerService#getTopic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes we can use the method, but when I wrote the test, I chose this method

"Replication or dedup check failed."
+ " Removing topic from topics list {}, {}",
topic, ex);
persistentTopic.getTransactionBuffer()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add this similar logic to line 1395?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!
it makes sense, in line 1395, it seems like checking create topic timeout, it's also need to close transaction buffer.
I'll create another PR to add the logic

@codelipenghui codelipenghui merged commit d4f8c40 into apache:branch-2.9 Feb 3, 2023
@tjiuming tjiuming deleted the fix/chk_dedup_failed branch February 3, 2023 02:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants