This is an automated email from the ASF dual-hosted git repository.
jlli pushed a commit to branch improve-segment-validator
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/improve-segment-validator by
this push:
new b224201 Address PR comments
b224201 is described below
commit b22420111992f6f2ed6c59c50d32017ab06b3994
Author: jackjlli <[email protected]>
AuthorDate: Wed Feb 20 22:54:48 2019 -0800
Address PR comments
---
.../helix/core/PinotHelixResourceManager.java | 11 +++----
.../tests/PinotURIUploadIntegrationTest.java | 34 +++++++++++++++++++++-
2 files changed, 37 insertions(+), 8 deletions(-)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
index 7382615..18100ed 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
@@ -1461,11 +1461,12 @@ public class PinotHelixResourceManager {
}
public void addNewSegment(@Nonnull SegmentMetadata segmentMetadata, @Nonnull
String downloadUrl) {
- addNewSegment(segmentMetadata, downloadUrl, null, null);
+ List<String> assignedInstances =
getAssignedInstancesForSegment(segmentMetadata);
+ addNewSegment(segmentMetadata, downloadUrl, null, assignedInstances);
}
public void addNewSegment(@Nonnull SegmentMetadata segmentMetadata, @Nonnull
String downloadUrl, String crypter,
- List<String> assignedInstances) {
+ @Nonnull List<String> assignedInstances) {
String segmentName = segmentMetadata.getName();
String offlineTableName =
TableNameBuilder.OFFLINE.tableNameWithType(segmentMetadata.getTableName());
@@ -1688,8 +1689,7 @@ public class PinotHelixResourceManager {
}
/**
- * Gets assigned instances for uploading new segment. This method can be
used to detect
- * whether table config is misconfigured when validating segment.
+ * Gets assigned instances for uploading new segment.
* @param segmentMetadata segment metadata
* @return a list of assigned instances.
*/
@@ -1722,9 +1722,6 @@ public class PinotHelixResourceManager {
String segmentName = segmentMetadata.getName();
// Assign new segment to instances
- if (assignedInstances == null) {
- assignedInstances = getAssignedInstancesForSegment(segmentMetadata);
- }
HelixHelper.addSegmentToIdealState(_helixZkManager, offlineTableName,
segmentName, assignedInstances);
}
diff --git
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
index fb86af2..bfa284f 100644
---
a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
+++
b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PinotURIUploadIntegrationTest.java
@@ -48,6 +48,7 @@ import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.DefaultHttpClient;
import org.apache.http.util.EntityUtils;
+import org.apache.pinot.common.utils.CommonConstants;
import org.apache.pinot.common.utils.FileUploadDownloadClient;
import org.apache.pinot.common.utils.JsonUtils;
import org.apache.pinot.common.utils.TarGzCompressionUtils;
@@ -67,7 +68,7 @@ import org.testng.annotations.Test;
/**
* Tests the URI upload path through a local file uri.
*/
-public class PinotURIUploadIntegrationTest extends BaseClusterIntegrationTest {
+public class PinotURIUploadIntegrationTest extends
BaseClusterIntegrationTestSet {
private static final Logger LOGGER =
LoggerFactory.getLogger(PinotURIUploadIntegrationTest.class);
private String _tableName;
private File _metadataDir = new File(_segmentDir, "tmpMeta");
@@ -85,6 +86,9 @@ public class PinotURIUploadIntegrationTest extends
BaseClusterIntegrationTest {
FileUtils.deleteQuietly(new File(_metadataDir.getAbsolutePath() +
TarGzCompressionUtils.TAR_GZ_FILE_EXTENSION));
// Start an empty Pinot cluster
startZk();
+// ControllerConf controllerConf = getDefaultControllerConfiguration();
+// controllerConf.setTenantIsolationEnabled(false);
+// startController(controllerConf);
startController();
startBroker();
startServer();
@@ -186,6 +190,34 @@ public class PinotURIUploadIntegrationTest extends
BaseClusterIntegrationTest {
Assert.fail("Failed to get from " + currentNrows + " to " + finalNrows);
}
+ @Test(dataProvider = "configProvider")
+ public void testSegmentValidator(String tableName, SegmentVersion version) {
+ completeTableConfiguration();
+ String serverInstanceId = "Server_localhost_" +
CommonConstants.Helix.DEFAULT_SERVER_NETTY_PORT;
+
+ // Disable server instance.
+ _helixAdmin.enableInstance(getHelixClusterName(), serverInstanceId, false);
+
+ final String segment6 = "segmentToBeRefreshed_6";
+ final int nRows1 = 69;
+ try {
+ generateAndUploadRandomSegment(segment6, nRows1);
+ Assert.fail("Uploading segments should fail.");
+ } catch (Exception e) {
+ Assert.assertNotNull(e);
+ Assert.assertTrue(e.getMessage().contains("No assigned Instances for
Segment"));
+ }
+
+ // Re-enable the server instance.
+ _helixAdmin.enableInstance(getHelixClusterName(), serverInstanceId, true);
+
+ try {
+ generateAndUploadRandomSegment(segment6, nRows1);
+ } catch (Exception e) {
+ Assert.fail("Uploading segments should succeed.");
+ }
+ }
+
@AfterClass
public void tearDown() {
stopServer();
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
