From b23a1eebc87a0180ddfaf749f67356484be03c53 Mon Sep 17 00:00:00 2001 From: fengyubiao Date: Wed, 1 Feb 2023 17:19:48 +0800 Subject: [PATCH 1/3] [fix] [admin] set offload threshold should fail when ns policies is readonly --- .../broker/resources/NamespaceResources.java | 2 +- .../broker/admin/impl/NamespacesBase.java | 2 +- .../broker/admin/AdminApiOffloadTest.java | 19 +++++++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/NamespaceResources.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/NamespaceResources.java index dd1c428380bad..48f8259656729 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/NamespaceResources.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/resources/NamespaceResources.java @@ -50,7 +50,7 @@ public class NamespaceResources extends BaseResources { private final PartitionedTopicResources partitionedTopicResources; private final MetadataStore configurationStore; - private static final String POLICIES_READONLY_FLAG_PATH = "/admin/flags/policies-readonly"; + public static final String POLICIES_READONLY_FLAG_PATH = "/admin/flags/policies-readonly"; private static final String NAMESPACE_BASE_PATH = "/namespace"; private static final String BUNDLE_DATA_BASE_PATH = "/loadbalance/bundle-data"; diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java index 324c84048751f..44e2f46174abf 100644 --- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java +++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java @@ -2046,7 +2046,7 @@ protected CompletableFuture internalSetOffloadThresholdInSecondsAsync(long CompletableFuture f = new CompletableFuture<>(); validateNamespacePolicyOperationAsync(namespaceName, PolicyName.OFFLOAD, PolicyOperation.WRITE) - .thenApply(v -> validatePoliciesReadOnlyAccessAsync()) + .thenCompose(v -> validatePoliciesReadOnlyAccessAsync()) .thenCompose(v -> updatePoliciesAsync(namespaceName, policies -> { if (policies.offload_policies == null) { diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java index 604bc437f1963..3f99205966aa2 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java @@ -47,14 +47,17 @@ import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertNull; import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; import java.util.HashMap; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; import org.apache.bookkeeper.mledger.LedgerOffloader; import org.apache.bookkeeper.mledger.ManagedLedgerInfo; import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest; +import org.apache.pulsar.broker.resources.NamespaceResources; import org.apache.pulsar.broker.service.persistent.PersistentTopic; import org.apache.pulsar.client.admin.LongRunningProcessStatus; import org.apache.pulsar.client.admin.PulsarAdminException.ConflictException; @@ -290,6 +293,22 @@ public void testSetNamespaceOffloadPolicies() throws Exception { assertEquals(admin.namespaces().getOffloadPolicies(myNamespace), policies); } + + @Test + public void testSetNamespaceOffloadPoliciesFailByReadOnly() throws Exception { + pulsar.getConfigurationMetadataStore().put(NamespaceResources.POLICIES_READONLY_FLAG_PATH, "0".getBytes(), + Optional.empty()).join(); + try { + admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300); + fail("set offload threshold should fail when ns policies is readonly"); + } catch (Exception ex){ + // ignore. + } + // cleanup. + pulsar.getConfigurationMetadataStore().delete(NamespaceResources.POLICIES_READONLY_FLAG_PATH, + Optional.empty()).join(); + } + @Test public void testSetTopicOffloadPolicies() throws Exception { conf.setManagedLedgerOffloadThresholdInSeconds(100); From 8defc382e5166553f201508ab371f8100c1002bb Mon Sep 17 00:00:00 2001 From: fengyubiao Date: Wed, 1 Feb 2023 17:27:20 +0800 Subject: [PATCH 2/3] remove empty line --- .../java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java index 3f99205966aa2..edcac245a9d99 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java @@ -293,7 +293,6 @@ public void testSetNamespaceOffloadPolicies() throws Exception { assertEquals(admin.namespaces().getOffloadPolicies(myNamespace), policies); } - @Test public void testSetNamespaceOffloadPoliciesFailByReadOnly() throws Exception { pulsar.getConfigurationMetadataStore().put(NamespaceResources.POLICIES_READONLY_FLAG_PATH, "0".getBytes(), From 546c0490d2139c584272bdaaa259e9ad8b954ca6 Mon Sep 17 00:00:00 2001 From: fengyubiao Date: Thu, 2 Feb 2023 00:14:33 +0800 Subject: [PATCH 3/3] mv cleanup to finally block --- .../pulsar/broker/admin/AdminApiOffloadTest.java | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java index edcac245a9d99..c3265897b8767 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/admin/AdminApiOffloadTest.java @@ -295,17 +295,22 @@ public void testSetNamespaceOffloadPolicies() throws Exception { @Test public void testSetNamespaceOffloadPoliciesFailByReadOnly() throws Exception { - pulsar.getConfigurationMetadataStore().put(NamespaceResources.POLICIES_READONLY_FLAG_PATH, "0".getBytes(), - Optional.empty()).join(); + boolean setNsPolicyReadOnlySuccess = false; try { + pulsar.getConfigurationMetadataStore().put(NamespaceResources.POLICIES_READONLY_FLAG_PATH, "0".getBytes(), + Optional.empty()).join(); + setNsPolicyReadOnlySuccess = true; admin.namespaces().setOffloadThresholdInSeconds(myNamespace, 300); fail("set offload threshold should fail when ns policies is readonly"); } catch (Exception ex){ // ignore. + } finally { + // cleanup. + if (setNsPolicyReadOnlySuccess) { + pulsar.getConfigurationMetadataStore().delete(NamespaceResources.POLICIES_READONLY_FLAG_PATH, + Optional.empty()).join(); + } } - // cleanup. - pulsar.getConfigurationMetadataStore().delete(NamespaceResources.POLICIES_READONLY_FLAG_PATH, - Optional.empty()).join(); } @Test