This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 25cd6e3 Fix the flakiness of MultiNodesOfflineClusterIntegrationTest
(#8367)
25cd6e3 is described below
commit 25cd6e34b7a6b1b013e7181bfe466677cdc0a61d
Author: Xiaotian (Jackie) Jiang <17555551+jackie-ji...@users.noreply.github.com>
AuthorDate: Fri Mar 18 01:27:55 2022 -0700
Fix the flakiness of MultiNodesOfflineClusterIntegrationTest (#8367)
---
.../resources/PinotInstanceRestletResource.java | 18 ++++----
.../MultiNodesOfflineClusterIntegrationTest.java | 49 +++++++++++++++++-----
2 files changed, 50 insertions(+), 17 deletions(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
index 0c93eba..6b4b10b 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotInstanceRestletResource.java
@@ -241,17 +241,21 @@ public class PinotInstanceRestletResource {
public SuccessResponse dropInstance(
@ApiParam(value = "Instance name", required = true, example =
"Server_a.b.com_20000 | Broker_my.broker.com_30000")
@PathParam("instanceName") String instanceName) {
- if (!_pinotHelixResourceManager.instanceExists(instanceName)) {
- throw new ControllerApplicationException(LOGGER, "Instance " +
instanceName + " not found",
- Response.Status.NOT_FOUND);
- }
-
+ boolean instanceExists =
_pinotHelixResourceManager.instanceExists(instanceName);
+ // NOTE: Even if instance config does not exist, still try to delete
remaining instance ZK nodes in case some nodes
+ // are created again due to race condition (state transition
messages added after instance is dropped).
PinotResourceManagerResponse response =
_pinotHelixResourceManager.dropInstance(instanceName);
- if (!response.isSuccessful()) {
+ if (response.isSuccessful()) {
+ if (instanceExists) {
+ return new SuccessResponse("Successfully dropped instance");
+ } else {
+ throw new ControllerApplicationException(LOGGER, "Instance " +
instanceName + " not found",
+ Response.Status.NOT_FOUND);
+ }
+ } else {
throw new ControllerApplicationException(LOGGER,
"Failed to drop instance " + instanceName + " - " +
response.getMessage(), Response.Status.CONFLICT);
}
- return new SuccessResponse("Successfully dropped instance");
}
@PUT
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java
index 947db73..fb59a38 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java
@@ -20,13 +20,14 @@ package org.apache.pinot.integration.tests;
import java.util.Collections;
import java.util.Map;
+import org.apache.helix.model.ExternalView;
import org.apache.helix.model.IdealState;
import org.apache.pinot.broker.broker.helix.HelixBrokerStarter;
-import org.apache.pinot.common.utils.helix.HelixHelper;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants;
import
org.apache.pinot.spi.utils.CommonConstants.Helix.StateModel.BrokerResourceStateModel;
import org.apache.pinot.spi.utils.NetUtils;
+import org.apache.pinot.util.TestUtils;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
@@ -61,42 +62,70 @@ public class MultiNodesOfflineClusterIntegrationTest
extends OfflineClusterInteg
throws Exception {
// Add a new broker to the cluster
Map<String, Object> properties = getDefaultBrokerConfiguration().toMap();
- properties.put(CommonConstants.Helix.CONFIG_OF_CLUSTER_NAME,
getHelixClusterName());
+ String clusterName = getHelixClusterName();
+ properties.put(CommonConstants.Helix.CONFIG_OF_CLUSTER_NAME, clusterName);
properties.put(CommonConstants.Helix.CONFIG_OF_ZOOKEEPR_SERVER,
getZkUrl());
int port = NetUtils.findOpenPort(DEFAULT_BROKER_PORT);
properties.put(CommonConstants.Helix.KEY_OF_BROKER_QUERY_PORT, port);
properties.put(CommonConstants.Broker.CONFIG_OF_DELAY_SHUTDOWN_TIME_MS, 0);
-
HelixBrokerStarter brokerStarter = new HelixBrokerStarter();
brokerStarter.init(new PinotConfiguration(properties));
brokerStarter.start();
// Check if broker is added to all the tables in broker resource
String brokerId = brokerStarter.getInstanceId();
- IdealState brokerResource = HelixHelper.getBrokerIdealStates(_helixAdmin,
getHelixClusterName());
- for (Map<String, String> brokerAssignment :
brokerResource.getRecord().getMapFields().values()) {
+ IdealState brokerResourceIdealState =
+ _helixAdmin.getResourceIdealState(clusterName,
CommonConstants.Helix.BROKER_RESOURCE_INSTANCE);
+ for (Map<String, String> brokerAssignment :
brokerResourceIdealState.getRecord().getMapFields().values()) {
assertEquals(brokerAssignment.get(brokerId),
BrokerResourceStateModel.ONLINE);
}
+ TestUtils.waitForCondition(aVoid -> {
+ ExternalView brokerResourceExternalView =
+ _helixAdmin.getResourceExternalView(clusterName,
CommonConstants.Helix.BROKER_RESOURCE_INSTANCE);
+ for (Map<String, String> brokerAssignment :
brokerResourceExternalView.getRecord().getMapFields().values()) {
+ if (!brokerAssignment.containsKey(brokerId)) {
+ return false;
+ }
+ }
+ return true;
+ }, 60_000L, "Failed to find broker in broker resource ExternalView");
- // Stop and drop the broker
+ // Stop the broker
brokerStarter.stop();
+
+ // Dropping the broker should fail because it is still in the broker
resource
try {
sendDeleteRequest(_controllerRequestURLBuilder.forInstance(brokerId));
fail("Dropping instance should fail because it is still in the broker
resource");
} catch (Exception e) {
// Expected
}
+
// Untag the broker and update the broker resource so that it is removed
from the broker resource
sendPutRequest(_controllerRequestURLBuilder.forInstanceUpdateTags(brokerId,
Collections.emptyList(), true));
+
// Check if broker is removed from all the tables in broker resource
- brokerResource = HelixHelper.getBrokerIdealStates(_helixAdmin,
getHelixClusterName());
- for (Map<String, String> brokerAssignment :
brokerResource.getRecord().getMapFields().values()) {
+ brokerResourceIdealState =
+ _helixAdmin.getResourceIdealState(clusterName,
CommonConstants.Helix.BROKER_RESOURCE_INSTANCE);
+ for (Map<String, String> brokerAssignment :
brokerResourceIdealState.getRecord().getMapFields().values()) {
assertFalse(brokerAssignment.containsKey(brokerId));
}
- // Dropping instance should success
+ TestUtils.waitForCondition(aVoid -> {
+ ExternalView brokerResourceExternalView =
+ _helixAdmin.getResourceExternalView(clusterName,
CommonConstants.Helix.BROKER_RESOURCE_INSTANCE);
+ for (Map<String, String> brokerAssignment :
brokerResourceExternalView.getRecord().getMapFields().values()) {
+ if (brokerAssignment.containsKey(brokerId)) {
+ return false;
+ }
+ }
+ return true;
+ }, 60_000L, "Failed to remove broker from broker resource ExternalView");
+
+ // Dropping the broker should success now
sendDeleteRequest(_controllerRequestURLBuilder.forInstance(brokerId));
+
// Check if broker is dropped from the cluster
-
assertFalse(_helixAdmin.getInstancesInCluster(getHelixClusterName()).contains(brokerId));
+
assertFalse(_helixAdmin.getInstancesInCluster(clusterName).contains(brokerId));
}
@Test(enabled = false)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
