[fix][flaky-test]AdminApi2Test.testDeleteTenant

[poorbarcode]

Fixes #17050

Master Issue:

• Flaky-test: AdminApi2Test.testDeleteTenant #17050

Motivation

If enabled system topic, when a topic is deleted, these events are triggered serially:

• delete topic
• check topic ownership
• load bundle
• trigger event: NamespaceBundleOwnershipListener.onLoad()
• init system topic client
• subscribe system topic
• (High loght)create topic: __change_events

Then the exception "Directory not empty" occurs when deleting the namespace.

pulsar/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApi2Test.java

Lines 1367 to 1371 in ee0ea3a

	admin.topics().deletePartitionedTopic(topic);
	assertTrue(admin.topics().getList(namespace).isEmpty());
	// delete namespace
	admin.namespaces().deleteNamespace(namespace, false);

Even if will try to delete topics that type system when deleting namespace, there is still the problem of multi-thread concurrency.

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

Lines 311 to 315 in ee0ea3a

	if (!topics.isEmpty()) {
	for (String topic : topics) {
	try {
	futures.add(pulsar().getAdminClient().topics().deleteAsync(topic, true, true));
	} catch (Exception ex) {

(High light) I think this is also a bug

when the user manually creates the topic but does not use it, and then quickly deletes the namespace, there is a small probability left topic __change_events

I try to fix the flaky test this way: make delete namespace after __change_events is successfully created, but there still has another race condition: the __change_events/__compaction async creates and __change_events delete by namespace delete. Therefore, I will disabled systemTopic in method testDeleteTenant to solve this flaky test.

Modifications

• This PR only fixed the flaky test: disabled systemTopic in method testDeleteTenant.

• As for how to solve the bug, I think we need to discuss

Documentation

• doc-required
  (Your PR needs to update docs and you will update later)

• doc-not-needed
  (Please explain why)

• doc
  (Your PR contains doc changes)

• doc-complete
  (Docs have been already added)

[Technoboy-]

I think query getListOfPersistentTopics is enough, because __change_events is persistent.

[codelipenghui]

And we don't need to check partitioned topics? Because we will eventually create the partitons.

[poorbarcode]

Hi @Technoboy-

I think query getListOfPersistentTopics is enough, because __change_events is persistent.

already remove the logic that queries non-persistent topic

Hi @codelipenghui

And we don't need to check partitioned topics? Because we will eventually create the partitons.

already add the partitioned topics check.

[codelipenghui]

when the user manually creates the topic but does not use it, and then quickly deletes the namespace, there is a small probability left topic __change_events

Here looks like a race condition? The system topic created after the namespace has been deleted?

[poorbarcode]

Here looks like a race condition? The system topic created after the namespace has been deleted?

both correct.

[Technoboy-]

when the user manually creates the topic but does not use it, and then quickly deletes the namespace, there is a small probability left topic __change_events

Here looks like a race condition? The system topic created after the namespace has been deleted?

When deleting the partitioned topics, we have to check the ownership first and find that the bundle is not owned and then try to own the bundle and then trigger the system topic created async.

[poorbarcode]

I try to fix the flaky test this way: make delete namespace after __change_events is successfully created, but there still has another race condition: the __change_events/__compaction async creates and __change_events delete by namespace delete. Therefore, I will disable systemTopic in method testDeleteTenant to solve this flaky test.

@Technoboy- @codelipenghui Could you review this PR again?

[Technoboy-]

when the user manually creates the topic but does not use it, and then quickly deletes the namespace, there is a small probability left topic __change_events

Here looks like a race condition? The system topic created after the namespace has been deleted?

When deleting the partitioned topics, we have to check the ownership first and find that the bundle is not owned and then try to own the bundle and then trigger the system topic created async.

I come up with a new idea for deleting partitioned topics, we can set the LookupOptions.readOnly(true) when validateTopicOwnershipAsync to avoid loading the bundle.