[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5715: Support alias name to be same as selection
Jackie-Jiang commented on a change in pull request #5715:
URL: https://github.com/apache/incubator-pinot/pull/5715#discussion_r457848395
##
File path:
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
##
@@ -113,7 +113,11 @@ private static void
validateSelectionClause(Map aliasMap
// Sanity check on selection expression shouldn't use alias reference.
Set aliasKeys = new HashSet<>();
for (Identifier identifier : aliasMap.keySet()) {
- aliasKeys.add(identifier.getName().toLowerCase());
+ String aliasName = identifier.getName().toLowerCase();
+ if (aliasKeys.contains(aliasName)) {
+throw new SqlCompilationException("Duplicated alias name found.");
+ }
+ aliasKeys.add(aliasName);
Review comment:
(nit)
```suggestion
if (!aliasKeys.add(aliasName)) {
throw new SqlCompilationException("Duplicated alias name found.");
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5705: Add error message to broker response in case of broker send error
Jackie-Jiang commented on a change in pull request #5705:
URL: https://github.com/apache/incubator-pinot/pull/5705#discussion_r457847609
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##
@@ -125,8 +126,17 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
}
public static ProcessingException getException(ProcessingException
processingException, Exception exception) {
+return getException(processingException,
getTruncatedStackTrace(exception));
+ }
+
+ public static ProcessingException getException(ProcessingException
processingException, String errorMessage) {
String errorType = processingException.getMessage();
ProcessingException copiedProcessingException =
processingException.deepCopy();
+copiedProcessingException.setMessage(errorType + ":\n" + errorMessage);
+return copiedProcessingException;
+ }
+
+ public static String getTruncatedStackTrace(Exception exception) {
Review comment:
@sajjad-moradi Merged the PR. I think you need to be a committer in
order to merge PR. @mayankshriv @mcvsubbu Do we need to start a vote in order
to make Sajjad a committer?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: Add error message to broker response in case of broker send error (#5705)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 897c96c Add error message to broker response in case of broker send
error (#5705)
897c96c is described below
commit 897c96cf45d46ed664883a7c54790454266916f1
Author: Sajjad
AuthorDate: Mon Jul 20 22:34:00 2020 -0700
Add error message to broker response in case of broker send error (#5705)
On Pinot Broker, the incoming request gets written into socket channels of
the target Servers. This happens on `QueryRouter.submitQuery(...)` function. If
any exception occurs during submitQuery for any reason like connection refused
to one of the servers, sending requests to the remaining servers are abandoned
and the partial responses from already successful sent requests are returned in
BrokerResponse with no indication of the exception.
Although partial response is acceptable, there should be an indication of
such problem in broker response to make life easier for ppl debugging this
issue. Recently there was an incident where, for a specific use case, broker
response was empty (no partial response) and there was no exception returned,
while data was available on Pinot Servers. After good amount of time going
through logs, this issue was discovered with exception being connection refused
by only one faulty server.
This PR adds the exception to BrokerReponse for easier debugging.
---
.../SingleConnectionBrokerRequestHandler.java| 8
.../pinot/common/exception/QueryException.java | 20 +++-
.../pinot/core/transport/AsyncQueryResponse.java | 10 ++
.../org/apache/pinot/core/transport/QueryRouter.java | 1 +
4 files changed, 30 insertions(+), 9 deletions(-)
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
index 862dae1..420177a 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/SingleConnectionBrokerRequestHandler.java
@@ -31,12 +31,14 @@ import org.apache.pinot.broker.api.RequestStatistics;
import org.apache.pinot.broker.broker.AccessControlFactory;
import org.apache.pinot.broker.queryquota.QueryQuotaManager;
import org.apache.pinot.broker.routing.RoutingManager;
+import org.apache.pinot.common.exception.QueryException;
import org.apache.pinot.common.metrics.BrokerMeter;
import org.apache.pinot.common.metrics.BrokerMetrics;
import org.apache.pinot.common.metrics.BrokerQueryPhase;
import org.apache.pinot.common.request.BrokerRequest;
import org.apache.pinot.common.response.BrokerResponse;
import org.apache.pinot.common.response.broker.BrokerResponseNative;
+import org.apache.pinot.common.response.broker.QueryProcessingException;
import org.apache.pinot.common.utils.DataTable;
import org.apache.pinot.common.utils.HashUtil;
import org.apache.pinot.core.transport.AsyncQueryResponse;
@@ -114,6 +116,12 @@ public class SingleConnectionBrokerRequestHandler extends
BaseBrokerRequestHandl
brokerResponse.setNumServersQueried(numServersQueried);
brokerResponse.setNumServersResponded(numServersResponded);
+Exception brokerRequestSendException =
asyncQueryResponse.getBrokerRequestSendException();
+if (brokerRequestSendException != null) {
+ String errorMsg =
QueryException.getTruncatedStackTrace(brokerRequestSendException);
+ brokerResponse
+ .addToExceptions(new
QueryProcessingException(QueryException.BROKER_REQUEST_SEND_ERROR_CODE,
errorMsg));
+}
if (brokerResponse.getExceptionsSize() > 0) {
_brokerMetrics.addMeteredTableValue(rawTableName,
BrokerMeter.BROKER_RESPONSES_WITH_PROCESSING_EXCEPTIONS, 1);
}
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
b/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
index b0b9be5..89dda00 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
@@ -53,6 +53,7 @@ public class QueryException {
public static final int BROKER_TIMEOUT_ERROR_CODE = 400;
public static final int BROKER_RESOURCE_MISSING_ERROR_CODE = 410;
public static final int BROKER_INSTANCE_MISSING_ERROR_CODE = 420;
+ public static final int BROKER_REQUEST_SEND_ERROR_CODE = 425;
public static final int TOO_MANY_REQUESTS_ERROR_CODE = 429;
public static final int INTERNAL_ERROR_CODE = 450;
public static final int MERGE_RESPONSE_ERROR_CODE = 500;
@@ -125,8 +126,17 @@ public
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5705: Add error message to broker response in case of broker send error
Jackie-Jiang merged pull request #5705: URL: https://github.com/apache/incubator-pinot/pull/5705 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457845842
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -2164,6 +2168,190 @@ public boolean instanceExists(String instanceName) {
return tableNamesWithType;
}
+ /**
+ * Computes the start batch upload phase
+ *
+ * 1. Generate a batch id
+ * 2. Compute validation on the user inputs
+ * 3. Add the new lineage entry to the segment lineage metadata in the
property store
+ *
+ * Update is done with retry logic along with read-modify-write block for
achieving atomic update of the lineage
+ * metadata.
+ *
+ * @param tableNameWithType Table name with type
+ * @param segmentsFrom a list of segments to be merged
+ * @param segmentsTo a list of merged segments
+ * @return Bath Id
+ *
+ * @throws InvalidConfigException
+ */
+ public String startBatchUpload(String tableNameWithType, List
segmentsFrom, List segmentsTo) {
+// Create a batch id
+String batchId = UUID.randomUUID().toString();
+
+// Check that segmentsTo is not empty.
+if (segmentsTo == null || segmentsTo.isEmpty()) {
Review comment:
I have changed all checks to use `Preconditions`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang opened a new pull request #5722: Introduce IndexContainer in MutableSegmentImpl to reduce map lookups
Jackie-Jiang opened a new pull request #5722: URL: https://github.com/apache/incubator-pinot/pull/5722 ## Description Motivation: Currently within `MutableSegmentImpl` we maintain 10 maps from column to different type of index or column stats, which is very inefficient and requires a lot of redundant map lookups to access or update indexes. Also, it is quite hard to manage 10 maps, and the number of maps will keep growing as we add more types of indexes. Changes: - Introduce `IndexContainer` class inside MutableSegmentImpl to wrap all the index and stats for a column - Add helper method in `IndexContainer` to directly get `DataSource` and close all indexes for a column - Store `dictId` for the latest ingested records in `IndexContainer` to avoid the per-record map from column to dictId Bug fixes: - Do not store min/max value for aggregated metrics - Add document into text index properly This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457845659
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -2164,6 +2168,190 @@ public boolean instanceExists(String instanceName) {
return tableNamesWithType;
}
+ /**
+ * Computes the start batch upload phase
+ *
+ * 1. Generate a batch id
+ * 2. Compute validation on the user inputs
+ * 3. Add the new lineage entry to the segment lineage metadata in the
property store
+ *
+ * Update is done with retry logic along with read-modify-write block for
achieving atomic update of the lineage
+ * metadata.
+ *
+ * @param tableNameWithType Table name with type
+ * @param segmentsFrom a list of segments to be merged
+ * @param segmentsTo a list of merged segments
+ * @return Bath Id
+ *
+ * @throws InvalidConfigException
+ */
+ public String startBatchUpload(String tableNameWithType, List
segmentsFrom, List segmentsTo) {
+// Create a batch id
+String batchId = UUID.randomUUID().toString();
+
+// Check that segmentsTo is not empty.
+if (segmentsTo == null || segmentsTo.isEmpty()) {
+ String errorMsg = String
+ .format("'segmentsTo' cannot be null or empty (tableName = '%s',
segmentsFrom = '%s', segmentsTo = '%s'",
+ tableNameWithType, segmentsFrom, segmentsTo);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+// segmentsFrom can be empty in case of the initial upload
+if (segmentsFrom == null) {
+ segmentsFrom = new ArrayList<>();
+}
+
+// Check that all the segments from 'segmentsFrom' exist in the table
+Set segmentsForTable = new
HashSet<>(getSegmentsFor(tableNameWithType));
+if (!segmentsForTable.containsAll(segmentsFrom)) {
+ String errorMsg = String.format(
+ "Not all segments from 'segmentsFrom' are available in the table.
(tableName = '%s', segmentsFrom = '%s', "
+ + "segmentsTo = '%s', segmentsFromTable = '%s')",
tableNameWithType, segmentsFrom, segmentsTo,
+ segmentsForTable);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+try {
+ final List finalSegmentsFrom = segmentsFrom;
+ DEFAULT_RETRY_POLICY.attempt(() -> {
+// Fetch the segment lineage metadata
+ZNRecord segmentLineageZNRecord =
+
SegmentLineageAccessHelper.getSegmentLineageZNRecord(_propertyStore,
tableNameWithType);
+SegmentLineage segmentLineage;
+int expectedVersion = -1;
+if (segmentLineageZNRecord == null) {
+ segmentLineage = new SegmentLineage(tableNameWithType);
+} else {
+ segmentLineage = SegmentLineage.fromZNRecord(segmentLineageZNRecord);
+ expectedVersion = segmentLineageZNRecord.getVersion();
+}
+
+// Check that the batchId doesn't exists in the segment lineage
+if (segmentLineage.getLineageEntry(batchId) != null) {
+ String errorMsg = String.format("BatchId (%s) already exists in the
segment lineage.", batchId);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+for (String lineageEntryId : segmentLineage.getLineageEntryIds()) {
+ LineageEntry lineageEntry =
segmentLineage.getLineageEntry(lineageEntryId);
+
+ // Check that any segment from 'segmentsFrom' does not appear on the
left side.
+ if
(lineageEntry.getSegmentsFrom().stream().anyMatch(finalSegmentsFrom::contains))
{
Review comment:
changed to `Collections.disjoint`
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457845713
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -2164,6 +2168,190 @@ public boolean instanceExists(String instanceName) {
return tableNamesWithType;
}
+ /**
+ * Computes the start batch upload phase
+ *
+ * 1. Generate a batch id
+ * 2. Compute validation on the user inputs
+ * 3. Add the new lineage entry to the segment lineage metadata in the
property store
+ *
+ * Update is done with retry logic along with read-modify-write block for
achieving atomic update of the lineage
+ * metadata.
+ *
+ * @param tableNameWithType Table name with type
+ * @param segmentsFrom a list of segments to be merged
+ * @param segmentsTo a list of merged segments
+ * @return Bath Id
+ *
+ * @throws InvalidConfigException
+ */
+ public String startBatchUpload(String tableNameWithType, List
segmentsFrom, List segmentsTo) {
+// Create a batch id
+String batchId = UUID.randomUUID().toString();
+
+// Check that segmentsTo is not empty.
+if (segmentsTo == null || segmentsTo.isEmpty()) {
+ String errorMsg = String
+ .format("'segmentsTo' cannot be null or empty (tableName = '%s',
segmentsFrom = '%s', segmentsTo = '%s'",
+ tableNameWithType, segmentsFrom, segmentsTo);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+// segmentsFrom can be empty in case of the initial upload
+if (segmentsFrom == null) {
+ segmentsFrom = new ArrayList<>();
+}
+
+// Check that all the segments from 'segmentsFrom' exist in the table
+Set segmentsForTable = new
HashSet<>(getSegmentsFor(tableNameWithType));
+if (!segmentsForTable.containsAll(segmentsFrom)) {
+ String errorMsg = String.format(
+ "Not all segments from 'segmentsFrom' are available in the table.
(tableName = '%s', segmentsFrom = '%s', "
+ + "segmentsTo = '%s', segmentsFromTable = '%s')",
tableNameWithType, segmentsFrom, segmentsTo,
+ segmentsForTable);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+try {
+ final List finalSegmentsFrom = segmentsFrom;
+ DEFAULT_RETRY_POLICY.attempt(() -> {
+// Fetch the segment lineage metadata
+ZNRecord segmentLineageZNRecord =
+
SegmentLineageAccessHelper.getSegmentLineageZNRecord(_propertyStore,
tableNameWithType);
+SegmentLineage segmentLineage;
+int expectedVersion = -1;
+if (segmentLineageZNRecord == null) {
+ segmentLineage = new SegmentLineage(tableNameWithType);
+} else {
+ segmentLineage = SegmentLineage.fromZNRecord(segmentLineageZNRecord);
+ expectedVersion = segmentLineageZNRecord.getVersion();
+}
+
+// Check that the batchId doesn't exists in the segment lineage
+if (segmentLineage.getLineageEntry(batchId) != null) {
+ String errorMsg = String.format("BatchId (%s) already exists in the
segment lineage.", batchId);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+for (String lineageEntryId : segmentLineage.getLineageEntryIds()) {
+ LineageEntry lineageEntry =
segmentLineage.getLineageEntry(lineageEntryId);
+
+ // Check that any segment from 'segmentsFrom' does not appear on the
left side.
+ if
(lineageEntry.getSegmentsFrom().stream().anyMatch(finalSegmentsFrom::contains))
{
+String errorMsg = String.format(
+"It is not allowed to merge segments that are already merged.
(tableName = %s, segmentsFrom from "
++ "existing lineage entry = %s, requested segmentsFrom =
%s)", tableNameWithType,
+lineageEntry.getSegmentsFrom(), finalSegmentsFrom);
+throw new IllegalArgumentException(errorMsg);
+ }
+
+ // Check that merged segments name cannot be the same for different
lineage entry
+ if
(lineageEntry.getSegmentsTo().stream().anyMatch(segmentsTo::contains)) {
Review comment:
yes, we need to check both.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457843510
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##
@@ -2164,6 +2168,190 @@ public boolean instanceExists(String instanceName) {
return tableNamesWithType;
}
+ /**
+ * Computes the start batch upload phase
+ *
+ * 1. Generate a batch id
+ * 2. Compute validation on the user inputs
+ * 3. Add the new lineage entry to the segment lineage metadata in the
property store
+ *
+ * Update is done with retry logic along with read-modify-write block for
achieving atomic update of the lineage
+ * metadata.
+ *
+ * @param tableNameWithType Table name with type
+ * @param segmentsFrom a list of segments to be merged
+ * @param segmentsTo a list of merged segments
+ * @return Bath Id
+ *
+ * @throws InvalidConfigException
+ */
+ public String startBatchUpload(String tableNameWithType, List
segmentsFrom, List segmentsTo) {
+// Create a batch id
+String batchId = UUID.randomUUID().toString();
+
+// Check that segmentsTo is not empty.
+if (segmentsTo == null || segmentsTo.isEmpty()) {
+ String errorMsg = String
+ .format("'segmentsTo' cannot be null or empty (tableName = '%s',
segmentsFrom = '%s', segmentsTo = '%s'",
+ tableNameWithType, segmentsFrom, segmentsTo);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+// segmentsFrom can be empty in case of the initial upload
+if (segmentsFrom == null) {
+ segmentsFrom = new ArrayList<>();
+}
+
+// Check that all the segments from 'segmentsFrom' exist in the table
+Set segmentsForTable = new
HashSet<>(getSegmentsFor(tableNameWithType));
+if (!segmentsForTable.containsAll(segmentsFrom)) {
+ String errorMsg = String.format(
+ "Not all segments from 'segmentsFrom' are available in the table.
(tableName = '%s', segmentsFrom = '%s', "
+ + "segmentsTo = '%s', segmentsFromTable = '%s')",
tableNameWithType, segmentsFrom, segmentsTo,
+ segmentsForTable);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+try {
+ final List finalSegmentsFrom = segmentsFrom;
+ DEFAULT_RETRY_POLICY.attempt(() -> {
+// Fetch the segment lineage metadata
+ZNRecord segmentLineageZNRecord =
+
SegmentLineageAccessHelper.getSegmentLineageZNRecord(_propertyStore,
tableNameWithType);
+SegmentLineage segmentLineage;
+int expectedVersion = -1;
+if (segmentLineageZNRecord == null) {
+ segmentLineage = new SegmentLineage(tableNameWithType);
+} else {
+ segmentLineage = SegmentLineage.fromZNRecord(segmentLineageZNRecord);
+ expectedVersion = segmentLineageZNRecord.getVersion();
+}
+
+// Check that the batchId doesn't exists in the segment lineage
+if (segmentLineage.getLineageEntry(batchId) != null) {
+ String errorMsg = String.format("BatchId (%s) already exists in the
segment lineage.", batchId);
+ throw new IllegalArgumentException(errorMsg);
+}
+
+for (String lineageEntryId : segmentLineage.getLineageEntryIds()) {
+ LineageEntry lineageEntry =
segmentLineage.getLineageEntry(lineageEntryId);
+
+ // Check that any segment from 'segmentsFrom' does not appear on the
left side.
+ if
(lineageEntry.getSegmentsFrom().stream().anyMatch(finalSegmentsFrom::contains))
{
+String errorMsg = String.format(
+"It is not allowed to merge segments that are already merged.
(tableName = %s, segmentsFrom from "
++ "existing lineage entry = %s, requested segmentsFrom =
%s)", tableNameWithType,
+lineageEntry.getSegmentsFrom(), finalSegmentsFrom);
+throw new IllegalArgumentException(errorMsg);
+ }
+
+ // Check that merged segments name cannot be the same for different
lineage entry
+ if
(lineageEntry.getSegmentsTo().stream().anyMatch(segmentsTo::contains)) {
+String errorMsg = String.format(
+"It is not allowed to have the same segment name for merged
segments. (tableName = %s, segmentsTo from "
++ "existing lineage entry = %s, requested segmentsTo =
%s)", tableNameWithType,
+lineageEntry.getSegmentsTo(), segmentsTo);
+throw new IllegalArgumentException(errorMsg);
+ }
+}
+
+// Update lineage entry
+segmentLineage.addLineageEntry(batchId,
+new LineageEntry(finalSegmentsFrom, segmentsTo,
LineageEntryState.IN_PROGRESS, System.currentTimeMillis()));
+
+// Write back to the lineage entry
+return SegmentLineageAccessHelper.writeSegmentLineage(_propertyStore,
segmentLineage, expectedVersion);
+ });
+} catch
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457843106
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
##
@@ -455,6 +459,45 @@ public void uploadSegmentAsMultiPartV2(FormDataMultiPart
multiPart,
}
}
+ @POST
+ @Path("segments/{tableName}/startBatchUpload")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Start the batch upload", notes = "Start the batch
upload")
+ public Response startBatchUpload(
+ @ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String
tableTypeStr, String body)
Review comment:
fixed
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457842868
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/StartBatchUploadRequest.java
##
@@ -0,0 +1,42 @@
+/**
+ * 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.pinot.common.restlet.resources;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.List;
+
+
+public class StartBatchUploadRequest {
+ private List _segmentsFrom;
+ private List _segmentsTo;
+
+ public StartBatchUploadRequest(@JsonProperty("segmentsFrom") List
segmentsFrom,
Review comment:
added default & null check
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457840296
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResource.java
##
@@ -455,6 +459,45 @@ public void uploadSegmentAsMultiPartV2(FormDataMultiPart
multiPart,
}
}
+ @POST
+ @Path("segments/{tableName}/startBatchUpload")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Start the batch upload", notes = "Start the batch
upload")
+ public Response startBatchUpload(
+ @ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String
tableTypeStr, String body)
+ throws IOException {
+StartBatchUploadRequest request = JsonUtils.stringToObject(body,
StartBatchUploadRequest.class);
+String tableNameWithType =
+
TableNameBuilder.forType(TableType.valueOf(tableTypeStr.toUpperCase())).tableNameWithType(tableName);
+try {
+ String batchId = _pinotHelixResourceManager
+ .startBatchUpload(tableNameWithType, request.getSegmentsFrom(),
request.getSegmentsTo());
+ return Response.ok(new BatchId(batchId)).build();
+} catch (Exception e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(),
Response.Status.INTERNAL_SERVER_ERROR, e);
+}
+ }
+
+ @POST
+ @Path("segments/{tableName}/endBatchUpload")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "End the batch upload", notes = "End the batch upload")
+ public Response endBatchUpload(
+ @ApiParam(value = "Name of the table", required = true)
@PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String
tableTypeStr, String body)
Review comment:
fixed
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457838835
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/lineage/SegmentLineage.java
##
@@ -70,6 +71,17 @@ public String addLineageEntry(LineageEntry lineageEntry) {
return lineageId;
}
+ /**
+ * Add lineage entry to the segment lineage metadata with the given lineage
entry id
+ * @param lineageEntryId the id for the lineage entry
+ * @param lineageEntry a lineage entry
+ * @return the id for the input lineage entry for the access
+ */
+ public String addLineageEntry(String lineageEntryId, LineageEntry
lineageEntry) {
Review comment:
One reason I added `void addLineageEntry(String lineageEntryId,
LineageEntry lineageEntry)` was because there's no easy way to pass
`lineageEntryId` outside of the retry policy block. (`lineageEntryId` is
generated within retry function and it needs to be passed back to the top
caller)
Due to the above reason, I have removed `String addLineageEntry(LineageEntry
lineageEntry)` and kept `void addLineageEntry(String lineageEntryId,
LineageEntry lineageEntry)`.
I also added `void updateLineageEntry(String lineageEntryId, LineageEntry
lineageEntry) `
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5715: Support alias name to be same as selection
fx19880617 commented on a change in pull request #5715: URL: https://github.com/apache/incubator-pinot/pull/5715#discussion_r457831931 ## File path: pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java ## @@ -113,32 +113,11 @@ private static void validateSelectionClause(Map aliasMap // Sanity check on selection expression shouldn't use alias reference. Review comment: done. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch support_alias_name_fix updated (dc12545 -> e5178e4)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch support_alias_name_fix in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard dc12545 Support alias name to be same as selection add e5178e4 Support alias name to be same as selection This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (dc12545) \ N -- N -- N refs/heads/support_alias_name_fix (e5178e4) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java| 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch support_alias_name_fix updated (e572e68 -> dc12545)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch support_alias_name_fix in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard e572e68 Support alias name to be same as selection add dc12545 Support alias name to be same as selection This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (e572e68) \ N -- N -- N refs/heads/support_alias_name_fix (dc12545) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../apache/pinot/sql/parsers/CalciteSqlParser.java | 23 +++--- .../pinot/sql/parsers/CalciteSqlCompilerTest.java | 13 ++-- 2 files changed, 14 insertions(+), 22 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457825742
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/BatchId.java
##
@@ -0,0 +1,34 @@
+/**
+ * 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.pinot.common.restlet.resources;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+
+public class BatchId {
Review comment:
I was trying to implement the following as the response:
```
POST /startBatchUpload
200
{
"batchId": "xxx"
}
```
One easy way to achieve this on jersey is to use java object.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee commented on a change in pull request #5712: Add startBatchUpload, endBatchUpload controller API
snleee commented on a change in pull request #5712:
URL: https://github.com/apache/incubator-pinot/pull/5712#discussion_r457825393
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/lineage/SegmentLineage.java
##
@@ -108,7 +120,9 @@ public static SegmentLineage fromZNRecord(ZNRecord record) {
String lineageId = listField.getKey();
List value = listField.getValue();
Preconditions.checkState(value.size() == 4);
- List segmentsFrom =
Arrays.asList(value.get(0).split(COMMA_SEPARATOR));
+ String segmentsFromStr = value.get(0);
+ List segmentsFrom = (segmentsFromStr == null ||
segmentsFromStr.length() == 0) ? new ArrayList<>()
+ : Arrays.asList(value.get(0).split(COMMA_SEPARATOR));
List segmentsTo =
Arrays.asList(value.get(1).split(COMMA_SEPARATOR));
Review comment:
changed to `StringUtils.split()`.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on issue #5721: Pinot UI expandable leftmost column(Page Links)
fx19880617 commented on issue #5721: URL: https://github.com/apache/incubator-pinot/issues/5721#issuecomment-661603644 > We already have breadcrumbs at the top bar so it's gonna be best to add a expand/collapse button on the sidebar. I'll work on it. Many Thanks! This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] shahsank3t commented on issue #5721: Pinot UI expandable leftmost column(Page Links)
shahsank3t commented on issue #5721: URL: https://github.com/apache/incubator-pinot/issues/5721#issuecomment-661601146 We already have breadcrumbs at the top bar so it's gonna be best to add a expand/collapse button on the sidebar. I'll work on it. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 merged pull request #5714: Disallow table creation with dot in the table name
fx19880617 merged pull request #5714: URL: https://github.com/apache/incubator-pinot/pull/5714 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: Disallow table creation with dot in the table name (#5714)
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/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 8e412b5 Disallow table creation with dot in the table name (#5714)
8e412b5 is described below
commit 8e412b5754bde73be0d3dd2de893d40fecbff2c1
Author: Xiang Fu
AuthorDate: Mon Jul 20 20:02:10 2020 -0700
Disallow table creation with dot in the table name (#5714)
---
.../api/resources/PinotTableRestletResource.java | 4
.../controller/api/PinotTableRestletResourceTest.java | 11 +++
.../java/org/apache/pinot/core/util/TableConfigUtils.java | 14 ++
3 files changed, 29 insertions(+)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
index 08ec338..429a1de 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
@@ -112,7 +112,11 @@ public class PinotTableRestletResource {
TableConfig tableConfig;
try {
tableConfig = JsonUtils.stringToObject(tableConfigStr,
TableConfig.class);
+ // TableConfigUtils.validate(...) is used across table create/update.
TableConfigUtils.validate(tableConfig);
+ // TableConfigUtils.validateTableName(...) checks table name rules.
+ // So it won't effect already created tables.
+ TableConfigUtils.validateTableName(tableConfig);
} catch (Exception e) {
throw new ControllerApplicationException(LOGGER, e.getMessage(),
Response.Status.BAD_REQUEST, e);
}
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
index 52ad0fa..d1cd559 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
@@ -96,6 +96,17 @@ public class PinotTableRestletResourceTest extends
ControllerTest {
Assert.assertTrue(e.getMessage().startsWith("Server returned HTTP
response code: 400"));
}
+offlineTableConfig = _offlineBuilder.build();
+offlineTableConfigJson = (ObjectNode) offlineTableConfig.toJsonNode();
+offlineTableConfigJson.put(TableConfig.TABLE_NAME_KEY,
"bad.table.with.dot");
+try {
+ sendPostRequest(_createTableUrl, offlineTableConfigJson.toString());
+ Assert.fail("Creation of an OFFLINE table with dot in the table name
does not fail");
+} catch (IOException e) {
+ // Expected 400 Bad Request
+ Assert.assertTrue(e.getMessage().startsWith("Server returned HTTP
response code: 400"));
+}
+
// Create an OFFLINE table with a valid name which should succeed
offlineTableConfig =
_offlineBuilder.setTableName("valid_table_name").build();
String offlineTableConfigString = offlineTableConfig.toJsonString();
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
b/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
index 7dcb1b3..64336a2 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
@@ -56,6 +56,20 @@ public final class TableConfigUtils {
validateIngestionConfig(tableConfig.getIngestionConfig());
}
+ /**
+ * Validates the table name with the following rules:
+ *
+ * Table name shouldn't contain dot in it
+ *
+ */
+ public static void validateTableName(TableConfig tableConfig) {
+String tableName = tableConfig.getTableName();
+if (tableName.contains(".")) {
+ throw new IllegalStateException(
+ "Table name: '" + tableName + "' containing '.' is not allowed");
+}
+ }
+
private static void validateValidationConfig(TableConfig tableConfig) {
SegmentsValidationAndRetentionConfig validationConfig =
tableConfig.getValidationConfig();
if (validationConfig != null) {
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai merged pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
akshayrai merged pull request #5720: URL: https://github.com/apache/incubator-pinot/pull/5720 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (7708341 -> 2aad02b)
This is an automated email from the ASF dual-hosted git repository.
akshayrai09 pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git.
from 7708341 Re-implement TarGzCompressionUtils (#5665)
add 2aad02b [TE] Created a separate doc for Application. +cosmetic
refactor (#5720)
No new revisions were added by this update.
Summary of changes:
thirdeye/docs/{intro.rst => about.rst} | 0
thirdeye/docs/application.rst | 96 ++
thirdeye/docs/configuration.rst| 38 --
thirdeye/docs/getting_started.rst | 21 +---
thirdeye/docs/introduction.rst | 5 +-
5 files changed, 100 insertions(+), 60 deletions(-)
rename thirdeye/docs/{intro.rst => about.rst} (100%)
create mode 100644 thirdeye/docs/application.rst
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] sajjad-moradi commented on a change in pull request #5705: Add error message to broker response in case of broker send error
sajjad-moradi commented on a change in pull request #5705:
URL: https://github.com/apache/incubator-pinot/pull/5705#discussion_r457762443
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##
@@ -125,8 +126,17 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
}
public static ProcessingException getException(ProcessingException
processingException, Exception exception) {
+return getException(processingException,
getTruncatedStackTrace(exception));
+ }
+
+ public static ProcessingException getException(ProcessingException
processingException, String errorMessage) {
String errorType = processingException.getMessage();
ProcessingException copiedProcessingException =
processingException.deepCopy();
+copiedProcessingException.setMessage(errorType + ":\n" + errorMessage);
+return copiedProcessingException;
+ }
+
+ public static String getTruncatedStackTrace(Exception exception) {
Review comment:
I don't have the write access to merge this PR. How can I get the write
access?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5705: Add error message to broker response in case of broker send error
Jackie-Jiang commented on a change in pull request #5705:
URL: https://github.com/apache/incubator-pinot/pull/5705#discussion_r457761728
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##
@@ -125,8 +126,17 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
}
public static ProcessingException getException(ProcessingException
processingException, Exception exception) {
+return getException(processingException,
getTruncatedStackTrace(exception));
+ }
+
+ public static ProcessingException getException(ProcessingException
processingException, String errorMessage) {
String errorType = processingException.getMessage();
ProcessingException copiedProcessingException =
processingException.deepCopy();
+copiedProcessingException.setMessage(errorType + ":\n" + errorMessage);
+return copiedProcessingException;
+ }
+
+ public static String getTruncatedStackTrace(Exception exception) {
Review comment:
The `errorType` I referred to here is `BrokerResponseSendError` which
you put in the `QueryException.class` so that users don't need to look up the
int error code (425) to understand the type of the error. The error message of
the Exception will be:
```
BrokerResponseSendError:
{stacktrace}
```
I already approved the change. These comments are just suggestions. You can
merge it
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5715: Support alias name to be same as selection
Jackie-Jiang commented on a change in pull request #5715: URL: https://github.com/apache/incubator-pinot/pull/5715#discussion_r457759329 ## File path: pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java ## @@ -113,32 +113,11 @@ private static void validateSelectionClause(Map aliasMap // Sanity check on selection expression shouldn't use alias reference. Review comment: I think the correct fix should be just not adding it to the aliasMap? You can rewrite the query before handling the alias by replacing `a AS a` to `a` This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5717: Add streaming query handler
Jackie-Jiang commented on a change in pull request #5717:
URL: https://github.com/apache/incubator-pinot/pull/5717#discussion_r457748048
##
File path: pinot-core/pom.xml
##
@@ -227,5 +227,25 @@
lucene-analyzers-common
${lucene.version}
+
Review comment:
You don't need to import them again here as they are already imported in
`pinot-common`
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/query/StreamingSelectionOnlyOperator.java
##
@@ -0,0 +1,131 @@
+/**
+ * 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.pinot.core.operator.query;
+
+import com.google.common.collect.ImmutableList;
+import com.google.protobuf.ByteString;
+import io.grpc.stub.StreamObserver;
+import org.apache.pinot.common.proto.Server;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.common.utils.DataTable;
+import org.apache.pinot.core.common.BlockValSet;
+import org.apache.pinot.core.common.RowBasedBlockValueFetcher;
+import org.apache.pinot.core.indexsegment.IndexSegment;
+import org.apache.pinot.core.operator.BaseOperator;
+import org.apache.pinot.core.operator.ExecutionStatistics;
+import org.apache.pinot.core.operator.blocks.IntermediateResultsBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.transform.TransformOperator;
+import org.apache.pinot.core.operator.transform.TransformResultMetadata;
+import org.apache.pinot.core.query.request.context.ExpressionContext;
+import org.apache.pinot.core.query.request.context.QueryContext;
+import org.apache.pinot.core.query.selection.SelectionOperatorUtils;
+
+import java.util.ArrayList;
+import java.util.List;
+
+
+public class StreamingSelectionOnlyOperator extends
BaseOperator {
+ private static final String OPERATOR_NAME = "SelectionOnlyOperator";
+
+ private final IndexSegment _indexSegment;
+ private final TransformOperator _transformOperator;
+ private final List _expressions;
+ private final BlockValSet[] _blockValSets;
+ private final DataSchema _dataSchema;
+ private final int _numRowsToKeep;
+ private final List _rows;
+ private final StreamObserver _streamObserver;
+
+ private int _numDocsScanned = 0;
+
+ public StreamingSelectionOnlyOperator(
+ IndexSegment indexSegment,
+ QueryContext queryContext,
+ List expressions,
+ TransformOperator transformOperator,
+ StreamObserver streamObserver) {
Review comment:
Let's not pass the `streamObserver` to the segment level operator
because multiple segments will be processed in parallel, but calls to
`streamObserver.onNext()` need to be synchronized.
We should create a `StreamingCombineOperator` (instance level operator) to
keep fetching `IntermediateResultsBlock` from this operator and handle the
calls to `streamObserver.onNext()`.
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/plan/StreamingSelectionPlanNode.java
##
@@ -0,0 +1,105 @@
+/**
+ * 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.pinot.core.plan;
+
+import io.grpc.stub.StreamObserver;
+import org.apache.pinot.common.proto.Server;
+import
org.apache.pinot.common.utils.CommonConstants.Segment.BuiltInVirtualColumn;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.indexsegment.IndexSegment;
+import
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5715: Support alias name to be same as selection
fx19880617 commented on a change in pull request #5715: URL: https://github.com/apache/incubator-pinot/pull/5715#discussion_r457756336 ## File path: pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java ## @@ -113,32 +113,11 @@ private static void validateSelectionClause(Map aliasMap // Sanity check on selection expression shouldn't use alias reference. Review comment: Added it back, basically remove colA in aliasMap for case like `SELECT colA AS colA`. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch support_alias_name_fix updated (799a75a -> e572e68)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch support_alias_name_fix in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 799a75a Support alias name to be same as selection add e572e68 Support alias name to be same as selection This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (799a75a) \ N -- N -- N refs/heads/support_alias_name_fix (e572e68) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../apache/pinot/sql/parsers/CalciteSqlParser.java | 32 ++ .../pinot/sql/parsers/CalciteSqlCompilerTest.java | 10 +++ 2 files changed, 42 insertions(+) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 commented on a change in pull request #5714: Disallow table creation with dot in the table name
fx19880617 commented on a change in pull request #5714:
URL: https://github.com/apache/incubator-pinot/pull/5714#discussion_r457745576
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##
@@ -113,6 +113,7 @@ public SuccessResponse addTable(String tableConfigStr) {
try {
tableConfig = JsonUtils.stringToObject(tableConfigStr,
TableConfig.class);
TableConfigUtils.validate(tableConfig);
+ TableConfigUtils.validateTableName(tableConfig);
Review comment:
added.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch disallow_table_creation_with_dot updated (6abf49c -> 3ce4b6b)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch disallow_table_creation_with_dot in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 6abf49c Disallow table creation with dot in the table name add 3ce4b6b Disallow table creation with dot in the table name This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (6abf49c) \ N -- N -- N refs/heads/disallow_table_creation_with_dot (3ce4b6b) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../pinot/controller/api/resources/PinotTableRestletResource.java| 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch disallow_table_creation_with_dot updated (7ebd02c -> 6abf49c)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch disallow_table_creation_with_dot in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 7ebd02c Disallow table creation with dot in the table name add 6abf49c Disallow table creation with dot in the table name This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (7ebd02c) \ N -- N -- N refs/heads/disallow_table_creation_with_dot (6abf49c) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../pinot/controller/api/resources/PinotTableRestletResource.java | 2 ++ 1 file changed, 2 insertions(+) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
akshayrai commented on a change in pull request #5720: URL: https://github.com/apache/incubator-pinot/pull/5720#discussion_r457689858 ## File path: thirdeye/docs/application.rst ## @@ -0,0 +1,96 @@ +.. +.. 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. +.. + +.. _application: + + +Application + + +An application is a basic TE entity can serves as a container for metrics, alerts and other entities. +It also can be used to group a bunch of users. + +Creating an Application +## + +You can create an Application in ThirdEye using 2 ways: +1. ThirdEye Admin +2. Using the API Review comment: Awesome! Looks like it was added recently: https://github.com/apache/incubator-pinot/pull/5601 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] suvodeep-pyne commented on a change in pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
suvodeep-pyne commented on a change in pull request #5720:
URL: https://github.com/apache/incubator-pinot/pull/5720#discussion_r457637138
##
File path: thirdeye/docs/application.rst
##
@@ -0,0 +1,96 @@
+..
+.. 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.
+..
+
+.. _application:
+
+
+Application
+
+
+An application is a basic TE entity can serves as a container for metrics,
alerts and other entities.
+It also can be used to group a bunch of users.
+
+Creating an Application
+##
+
+You can create an Application in ThirdEye using 2 ways:
+1. ThirdEye Admin
+2. Using the API
+
+From ThirdEye Admin UI
+***
+
+In order to create an application, follow the steps below.
+
+1. Go to the ``thirdeye-admin`` page. http://localhost:1426/thirdeye-admin
+2. Click the ``Entity Editor`` tab
+3. Choose ``Application`` from ``Select config type``.
+4. In the ``Select Entity to Edit`` menu, select ``Create New``
+5. Copy paste the json block below into the textbox on the right and click
``load to editor``
+
+
+.. code-block:: json
+
+ {
+"application": "myApp",
+"recipients": "myapp_ow...@company.com"
+ }
+
+6. Click ``Submit`` on the bottom left to create an application.
+
+.. image::
https://user-images.githubusercontent.com/44730481/61093659-6c926700-a400-11e9-8690-6a1742671e5e.png
+ :width: 500
+
+From API
+***
+
+Here are the steps to create an Application from the terminal.
+
+1. Obtain an authentication token. By default, TE auth is disabled, so the
credentials are ignored.
+Feel free to modify the values in the script below.
+
+.. code-block:: bash
+
+ function tetoken {
+ curl -s --location --request POST --cookie-jar -
'http://localhost:1426/auth/authenticate' \
+ --header 'Authorization: Bearer temp' \
+ --header 'Content-Type: application/json' \
+ --data-raw '{
+ "principal": "1",
+ "password": "1"
+ }' | grep te_auth | awk '{print $NF}'
+ }
+ token=$(tetoken)
+2. Create the application using the command below. Feel free to update the
inline json as per your needs.
+
+.. code-block:: bash
+
+ function create_te_app {
+ token=$1
+ curl --location --request POST
'http://localhost:1426/thirdeye/entity?entityType=APPLICATION' \
+ --header "Authorization: Bearer ${token}" \
+ --header 'Content-Type: application/json' \
+ --header 'Cookie:
te_auth=8ba22acc7616b77fb62fc0aad88a638a16c74e7922209d30afe8a96fee4b55e9' \
Review comment:
Yes. Thanks for pointing that out. I'll update the PR.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] suvodeep-pyne commented on a change in pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
suvodeep-pyne commented on a change in pull request #5720: URL: https://github.com/apache/incubator-pinot/pull/5720#discussion_r457635722 ## File path: thirdeye/docs/application.rst ## @@ -0,0 +1,96 @@ +.. +.. 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. +.. + +.. _application: + + +Application + + +An application is a basic TE entity can serves as a container for metrics, alerts and other entities. +It also can be used to group a bunch of users. + +Creating an Application +## + +You can create an Application in ThirdEye using 2 ways: +1. ThirdEye Admin +2. Using the API Review comment: Hey @akshayrai I'm able to create an application from the UI. In fact, I was documenting this while I was going through the process. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5714: Disallow table creation with dot in the table name
Jackie-Jiang commented on a change in pull request #5714:
URL: https://github.com/apache/incubator-pinot/pull/5714#discussion_r457632283
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##
@@ -113,6 +113,7 @@ public SuccessResponse addTable(String tableConfigStr) {
try {
tableConfig = JsonUtils.stringToObject(tableConfigStr,
TableConfig.class);
TableConfigUtils.validate(tableConfig);
+ TableConfigUtils.validateTableName(tableConfig);
Review comment:
Probably good to add some comments on why only validate it here
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 opened a new issue #5721: Pinot UI expandable leftmost column(Page Links)
fx19880617 opened a new issue #5721: URL: https://github.com/apache/incubator-pinot/issues/5721 Just realize we have a new UI!  Regarding the new UI change, can we make the left column expandable so I can hide it. Or shall we think of making the page links on the top bar? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5715: Support alias name to be same as selection
Jackie-Jiang commented on a change in pull request #5715: URL: https://github.com/apache/incubator-pinot/pull/5715#discussion_r457626980 ## File path: pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java ## @@ -113,32 +113,11 @@ private static void validateSelectionClause(Map aliasMap // Sanity check on selection expression shouldn't use alias reference. Review comment: We should not remove the sanity check on usage of alias reference E.g. we should reject the following query: ``` SELECT colA AS a, foo(a, colB) FROM table ``` Another way is to replace the alias reference in selection as well This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
akshayrai commented on a change in pull request #5720: URL: https://github.com/apache/incubator-pinot/pull/5720#discussion_r457594891 ## File path: thirdeye/docs/application.rst ## @@ -0,0 +1,96 @@ +.. +.. 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. +.. + +.. _application: + + +Application + + +An application is a basic TE entity can serves as a container for metrics, alerts and other entities. +It also can be used to group a bunch of users. + +Creating an Application +## + +You can create an Application in ThirdEye using 2 ways: +1. ThirdEye Admin +2. Using the API Review comment: Right now, the admin portal doesn't have the capability to add a new application for the first time. We need to create an application using the API first and subsequently we can create more applications from the portal. This is a minor UI bug. Should we probably mention this or list out API as the first option over admin portal? Issue: https://github.com/apache/incubator-pinot/issues/5525 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
akshayrai commented on a change in pull request #5720: URL: https://github.com/apache/incubator-pinot/pull/5720#discussion_r457594891 ## File path: thirdeye/docs/application.rst ## @@ -0,0 +1,96 @@ +.. +.. 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. +.. + +.. _application: + + +Application + + +An application is a basic TE entity can serves as a container for metrics, alerts and other entities. +It also can be used to group a bunch of users. + +Creating an Application +## + +You can create an Application in ThirdEye using 2 ways: +1. ThirdEye Admin +2. Using the API Review comment: Right now, the admin portal doesn't have the capability to add a new application for the first time. We need to create an application using the API first and subsequently, we can create more applications from the portal. This is a minor UI bug. Issue: https://github.com/apache/incubator-pinot/issues/5525 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai commented on a change in pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
akshayrai commented on a change in pull request #5720:
URL: https://github.com/apache/incubator-pinot/pull/5720#discussion_r457593185
##
File path: thirdeye/docs/application.rst
##
@@ -0,0 +1,96 @@
+..
+.. 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.
+..
+
+.. _application:
+
+
+Application
+
+
+An application is a basic TE entity can serves as a container for metrics,
alerts and other entities.
+It also can be used to group a bunch of users.
+
+Creating an Application
+##
+
+You can create an Application in ThirdEye using 2 ways:
+1. ThirdEye Admin
+2. Using the API
+
+From ThirdEye Admin UI
+***
+
+In order to create an application, follow the steps below.
+
+1. Go to the ``thirdeye-admin`` page. http://localhost:1426/thirdeye-admin
+2. Click the ``Entity Editor`` tab
+3. Choose ``Application`` from ``Select config type``.
+4. In the ``Select Entity to Edit`` menu, select ``Create New``
+5. Copy paste the json block below into the textbox on the right and click
``load to editor``
+
+
+.. code-block:: json
+
+ {
+"application": "myApp",
+"recipients": "myapp_ow...@company.com"
+ }
+
+6. Click ``Submit`` on the bottom left to create an application.
+
+.. image::
https://user-images.githubusercontent.com/44730481/61093659-6c926700-a400-11e9-8690-6a1742671e5e.png
+ :width: 500
+
+From API
+***
+
+Here are the steps to create an Application from the terminal.
+
+1. Obtain an authentication token. By default, TE auth is disabled, so the
credentials are ignored.
+Feel free to modify the values in the script below.
+
+.. code-block:: bash
+
+ function tetoken {
+ curl -s --location --request POST --cookie-jar -
'http://localhost:1426/auth/authenticate' \
+ --header 'Authorization: Bearer temp' \
+ --header 'Content-Type: application/json' \
+ --data-raw '{
+ "principal": "1",
+ "password": "1"
+ }' | grep te_auth | awk '{print $NF}'
+ }
+ token=$(tetoken)
+2. Create the application using the command below. Feel free to update the
inline json as per your needs.
+
+.. code-block:: bash
+
+ function create_te_app {
+ token=$1
+ curl --location --request POST
'http://localhost:1426/thirdeye/entity?entityType=APPLICATION' \
+ --header "Authorization: Bearer ${token}" \
+ --header 'Content-Type: application/json' \
+ --header 'Cookie:
te_auth=8ba22acc7616b77fb62fc0aad88a638a16c74e7922209d30afe8a96fee4b55e9' \
Review comment:
Shouldn't the te_auth token be replaced here?
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] suvodeep-pyne opened a new pull request #5720: [TE] Created a separate doc for Application. +cosmetic refactor
suvodeep-pyne opened a new pull request #5720: URL: https://github.com/apache/incubator-pinot/pull/5720 ## Description Refactored ThirdEye Documentation. - Created a separate document for Application. - Moved content from Configuration and Getting Started doc into this doc. - Added documentation to create TE application via API This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] sajjad-moradi commented on a change in pull request #5705: Add error message to broker response in case of broker send error
sajjad-moradi commented on a change in pull request #5705:
URL: https://github.com/apache/incubator-pinot/pull/5705#discussion_r457564370
##
File path:
pinot-common/src/main/java/org/apache/pinot/common/exception/QueryException.java
##
@@ -125,8 +126,17 @@ public static void setMaxLinesOfStackTrace(int
maxLinesOfStackTrace) {
}
public static ProcessingException getException(ProcessingException
processingException, Exception exception) {
+return getException(processingException,
getTruncatedStackTrace(exception));
+ }
+
+ public static ProcessingException getException(ProcessingException
processingException, String errorMessage) {
String errorType = processingException.getMessage();
ProcessingException copiedProcessingException =
processingException.deepCopy();
+copiedProcessingException.setMessage(errorType + ":\n" + errorMessage);
+return copiedProcessingException;
+ }
+
+ public static String getTruncatedStackTrace(Exception exception) {
Review comment:
`errorType`, which is basically `exception.getMessage()`, is always
available in the first line of the stack trace. Since we keep the stack trace,
we don't need to separately keep it.
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch pinot_client_controller_constructor updated (532b0ea -> cbeecde)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch pinot_client_controller_constructor in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. discard 532b0ea Adding pinot java client constructor only by pinot controller add cbeecde Adding pinot java client constructor only by pinot controller This update added new revisions after undoing existing revisions. That is to say, some revisions that were in the old version of the branch are not in the new version. This situation occurs when a user --force pushes a change and generates a repository containing something like this: * -- * -- B -- O -- O -- O (532b0ea) \ N -- N -- N refs/heads/pinot_client_controller_constructor (cbeecde) You should already have received notification emails for all of the O revisions, and so the following emails describe only the N revisions from the common base, B. Any revisions marked "omit" are not gone; other references still refer to them. Any revisions marked "discard" are gone forever. No new revisions were added by this update. Summary of changes: .../pinot/client/ControllerBasedBrokerSelector.java| 18 ++ 1 file changed, 18 insertions(+) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 opened a new pull request #5719: Adding support to construct pinot java client with pinot controller url
fx19880617 opened a new pull request #5719: URL: https://github.com/apache/incubator-pinot/pull/5719 ## Description Adding support to construct pinot java client with pinot controller url. ## Release Notes Adding support to construct pinot java client with pinot controller url. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch pinot_client_controller_constructor created (now 532b0ea)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch pinot_client_controller_constructor in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. at 532b0ea Adding pinot java client constructor only by pinot controller This branch includes the following new commits: new 532b0ea Adding pinot java client constructor only by pinot controller The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] 01/01: Adding pinot java client constructor only by pinot controller
This is an automated email from the ASF dual-hosted git repository.
xiangfu pushed a commit to branch pinot_client_controller_constructor
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit 532b0ea53940644b9e307545e4e22f9fa7b360fc
Author: Xiang Fu
AuthorDate: Mon Jul 20 04:17:05 2020 -0700
Adding pinot java client constructor only by pinot controller
---
pinot-clients/pinot-java-client/pom.xml| 4 +
.../org/apache/pinot/client/ConnectionFactory.java | 19
.../client/ControllerBasedBrokerSelector.java | 122 +
.../tests/BaseClusterIntegrationTest.java | 6 +-
4 files changed, 150 insertions(+), 1 deletion(-)
diff --git a/pinot-clients/pinot-java-client/pom.xml
b/pinot-clients/pinot-java-client/pom.xml
index 6a98e3d..eb457ff 100644
--- a/pinot-clients/pinot-java-client/pom.xml
+++ b/pinot-clients/pinot-java-client/pom.xml
@@ -85,6 +85,10 @@
+ commons-io
+ commons-io
+
+
org.slf4j
slf4j-api
diff --git
a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ConnectionFactory.java
b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ConnectionFactory.java
index 666ca7c..d81a04c 100644
---
a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ConnectionFactory.java
+++
b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ConnectionFactory.java
@@ -49,6 +49,25 @@ public class ConnectionFactory {
}
/**
+ * Creates a connection to a Pinot cluster, given its Controller URL
+ * Please note that this client requires Pinot Controller supports getBroker
APIs,
+ * which is supported from Pinot 0.5.0.
+ *
+ * @param controllerUrls The comma separated URLs to Pinot Controller,
suggest to use Vip hostname or k8s service.
+ * E.g. http://pinot-controller:9000
+ *
http://pinot-controller-0:9000,http://pinot-controller-1:9000,http://pinot-controller-2:9000
+ * @return A connection that connects to the brokers in the given Pinot
cluster
+ */
+ public static Connection fromController(String controllerUrls) {
+try {
+ BrokerSelector brokerSelector = new
ControllerBasedBrokerSelector(controllerUrls);
+ return new Connection(brokerSelector,
_transportFactory.buildTransport(null));
+} catch (Exception e) {
+ throw new PinotClientException(e);
+}
+ }
+
+ /**
* Creates a connection from properties containing the connection parameters.
*
* @param properties The properties to use for the connection
diff --git
a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ControllerBasedBrokerSelector.java
b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ControllerBasedBrokerSelector.java
new file mode 100644
index 000..269475f
--- /dev/null
+++
b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/ControllerBasedBrokerSelector.java
@@ -0,0 +1,122 @@
+package org.apache.pinot.client;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import org.apache.commons.io.IOUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.pinot.client.ExternalViewReader.OFFLINE_SUFFIX;
+import static org.apache.pinot.client.ExternalViewReader.REALTIME_SUFFIX;
+
+
+public class ControllerBasedBrokerSelector implements BrokerSelector {
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ControllerBasedBrokerSelector.class);
+
+ public static final int DEFAULT_CONTROLLER_REQUEST_RETRIES = 3;
+ public static final long
DEFAULT_CONTROLLER_REQUEST_RETRIES_INTERVAL_IN_MILLS = 5000; // 5 seconds
+ public static final long
DEFAULT_CONTROLLER_REQUEST_SCHEDULE_INTERVAL_IN_MILLS = 30; // 5 minutes
+
+ private static final String CONTROLLER_ONLINE_TABLE_BROKERS_MAP_URL_TEMPLATE
= "%s/brokers/tables?state=online";
+ private static final Random RANDOM = new Random();
+ private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+ private final String[] controllerUrls;
+ private final AtomicReference>>
tableToBrokerListMapRef =
+ new AtomicReference>>();
+ private final AtomicReference> allBrokerListRef = new
AtomicReference>();
+ private final int controllerFetchRetries;
+ private final long controllerFetchRetriesIntervalMills;
+ private final long controllerFetchScheduleIntervalMills;
+
+ public ControllerBasedBrokerSelector(String
