Skip to content

Commit

Permalink
[Broker] Fix create the dynamic configuration resource if not exist (a…
Browse files Browse the repository at this point in the history
…pache#13420)

### Motivation

When request the `DELETE: /admin/brokers/configuration/dispatcherMinReadBatchSize`, which return the`org.apache.pulsar.metadata.api.MetadataStoreException$NotFoundException`.  The Pulsar does not create the dynamic configuration resource path on metadata if it does not exist, this behavior is different with Pulsar 2.8.
  • Loading branch information
nodece authored Dec 23, 2021
1 parent b9478f8 commit 3a1e8da
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public CompletableFuture<Optional<Map<String, String>>> getDynamicConfigurationA
return getAsync(BROKER_SERVICE_CONFIGURATION_PATH);
}

public Map<String, String> getDynamicConfiguration() throws MetadataStoreException {
return get(BROKER_SERVICE_CONFIGURATION_PATH).orElse(Collections.emptyMap());
public Optional<Map<String, String>> getDynamicConfiguration() throws MetadataStoreException {
return get(BROKER_SERVICE_CONFIGURATION_PATH);
}

public void setDynamicConfigurationWithCreate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import io.swagger.annotations.ApiResponse;
import io.swagger.annotations.ApiResponses;
import java.time.Duration;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -181,7 +182,7 @@ public void deleteDynamicConfiguration(@PathParam("configName") String configNam
public Map<String, String> getAllDynamicConfigurations() throws Exception {
validateSuperUserAccess();
try {
return dynamicConfigurationResources().getDynamicConfiguration();
return dynamicConfigurationResources().getDynamicConfiguration().orElseGet(Collections::emptyMap);
} catch (RestException e) {
LOG.error("[{}] couldn't find any configuration in zk {}", clientAppId(), e.getMessage(), e);
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2296,9 +2296,16 @@ private void validateConfigKey(String key) {
*/
private void updateDynamicServiceConfiguration() {
Optional<Map<String, String>> configCache = Optional.empty();

try {
configCache =
Optional.of(pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfiguration());
configCache =
pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfiguration();

// create dynamic-config if not exist.
if (!configCache.isPresent()) {
pulsar().getPulsarResources().getDynamicConfigResources()
.setDynamicConfigurationWithCreate(n -> Maps.newHashMap());
}
} catch (Exception e) {
log.warn("Failed to read dynamic broker configuration", e);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.pulsar.broker.admin;

import static org.junit.Assert.fail;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertNotNull;
import java.util.Map;
import javax.ws.rs.core.Response;
import lombok.extern.slf4j.Slf4j;
import org.apache.pulsar.broker.auth.MockedPulsarServiceBaseTest;
import org.apache.pulsar.client.admin.PulsarAdminException;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

@Slf4j
@Test(groups = "broker")
public class AdminApiDynamicConfigurationsTest extends MockedPulsarServiceBaseTest {
@BeforeMethod
@Override
public void setup() throws Exception {
super.internalSetup();
}

@AfterMethod(alwaysRun = true)
@Override
public void cleanup() throws Exception {
super.internalCleanup();
}

@Test
public void TestGetAllDynamicConfigurations() throws Exception {
Map<String,String> configs = admin.brokers().getAllDynamicConfigurations();
assertNotNull(configs);
}

@Test
public void TestDeleteDynamicConfiguration() throws Exception {
admin.brokers().deleteDynamicConfiguration("dispatcherMinReadBatchSize");
}

@Test
public void TestDeleteInvalidDynamicConfiguration() {
try {
admin.brokers().deleteDynamicConfiguration("errorName");
fail("exception should be thrown");
} catch (Exception e) {
if (e instanceof PulsarAdminException) {
assertEquals(((PulsarAdminException) e).getStatusCode(), Response.Status.PRECONDITION_FAILED.getStatusCode());
} else {
fail("PulsarAdminException should be thrown");
}
}
}
}

0 comments on commit 3a1e8da

Please sign in to comment.