From 6f792e51703d78b87052658bb1b963fb768eef2f Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Wed, 6 May 2026 00:12:17 +0300 Subject: [PATCH] [fix][test] Fix flaky BacklogQuotaManagerTest.createNamespaces (HTTP 409 cascade) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two `BacklogQuotaManagerTest` cases consistently fail together: an `@AfterMethod` `clearNamespaces` times out at 10s while force-deleting `prop/ns-quota`, leaving the namespace in metadata; the next test's `@BeforeMethod` `createNamespaces` then hits HTTP 409 ("Namespace already exists") and the cascade continues for every subsequent test. Root cause: PR #25624 added two pending-acks tests that leave Key_Shared consumers with unacked messages on `prop/ns-quota` and an active backlog-quota check loop running every 2s. Force-deleting that state can exceed the 10s default `Awaitility` budget used by the shared `MockedPulsarServiceBaseTest.deleteNamespaceWithRetry`. The afterMethod times out, leftover metadata remains, and the next beforeMethod hits 409. Fix: 1. `MockedPulsarServiceBaseTest.deleteNamespaceWithRetry` now uses `atMost(20, SECONDS)` instead of relying on the Awaitility 10s default. 2× headroom is enough for the heaviest cleanups in this class while staying conservative for the many tests that share this helper. 2. `BacklogQuotaManagerTest.createNamespaces` is split into a helper that catches `ConflictException` and recovers by force-deleting the leftover and recreating, so a slow prior cleanup cannot cascade as a 409 — defense in depth on top of (1). Verified locally with `@Test(invocationCount = 10)` on the two new pending-acks tests: 20/20 passes, all `clearNamespaces` cycles within the new 20s budget. --- .../auth/MockedPulsarServiceBaseTest.java | 1 + .../service/BacklogQuotaManagerTest.java | 28 ++++++++++++++----- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java index 550bc51390269..c1629c723a33e 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/MockedPulsarServiceBaseTest.java @@ -733,6 +733,7 @@ protected void deleteNamespaceWithRetry(String ns, boolean force) */ public static void deleteNamespaceWithRetry(String ns, boolean force, PulsarAdmin admin) throws Exception { Awaitility.await() + .atMost(20, TimeUnit.SECONDS) .pollDelay(500, TimeUnit.MILLISECONDS) .until(() -> { try { diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java index 6ac258d0ba83a..9a85b65b1e74b 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/service/BacklogQuotaManagerTest.java @@ -201,14 +201,28 @@ void shutdown() throws Exception { } @BeforeMethod(alwaysRun = true) - void createNamespaces() throws PulsarAdminException { + void createNamespaces() throws Exception { config.setPreciseTimeBasedBacklogQuotaCheck(false); - admin.namespaces().createNamespace("prop/ns-quota"); - admin.namespaces().setNamespaceReplicationClusters("prop/ns-quota", Sets.newHashSet("usc"), false); - admin.namespaces().createNamespace("prop/quotahold"); - admin.namespaces().setNamespaceReplicationClusters("prop/quotahold", Sets.newHashSet("usc"), false); - admin.namespaces().createNamespace("prop/quotaholdasync"); - admin.namespaces().setNamespaceReplicationClusters("prop/quotaholdasync", Sets.newHashSet("usc"), false); + createNamespaceForTest("prop/ns-quota"); + createNamespaceForTest("prop/quotahold"); + createNamespaceForTest("prop/quotaholdasync"); + } + + /** + * If a previous test's @AfterMethod timed out before the namespace was fully removed, the + * leftover would otherwise cascade as HTTP 409 here and fail every subsequent test. + * Force-delete and retry so each test starts with clean namespace state. + */ + private void createNamespaceForTest(String ns) throws Exception { + try { + admin.namespaces().createNamespace(ns); + } catch (PulsarAdminException.ConflictException e) { + log.warn().attr("namespace", ns) + .log("Namespace already exists from previous test — force-deleting and recreating"); + deleteNamespaceWithRetry(ns, true); + admin.namespaces().createNamespace(ns); + } + admin.namespaces().setNamespaceReplicationClusters(ns, Sets.newHashSet("usc"), false); } @AfterMethod(alwaysRun = true)