[GitHub] [incubator-pinot] xiaohui-sun merged pull request #5172: [TE] Remove MIGRATED_ALGORITHM type
xiaohui-sun merged pull request #5172: [TE] Remove MIGRATED_ALGORITHM type URL: https://github.com/apache/incubator-pinot/pull/5172 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: [TE] Remove MIGRATED_ALGORITHM type (#5172)
This is an automated email from the ASF dual-hosted git repository.
xhsun 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 0a890fe [TE] Remove MIGRATED_ALGORITHM type (#5172)
0a890fe is described below
commit 0a890fe50db38975fcb6f36a3ca475f7cca11fbd
Author: Xiaohui Sun
AuthorDate: Wed Apr 8 19:24:55 2020 -0700
[TE] Remove MIGRATED_ALGORITHM type (#5172)
---
.../thirdeye/detection/annotation/registry/DetectionRegistry.java | 7 ---
.../apache/pinot/thirdeye/detection/yaml/DetectionConfigTuner.java | 4 +---
.../detection/yaml/translator/DetectionConfigTranslator.java | 4 ++--
.../apache/pinot/thirdeye/formatter/DetectionConfigFormatter.java | 3 ---
4 files changed, 3 insertions(+), 15 deletions(-)
diff --git
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/registry/DetectionRegistry.java
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/registry/DetectionRegistry.java
index 3913969..d7f8070 100644
---
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/registry/DetectionRegistry.java
+++
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/annotation/registry/DetectionRegistry.java
@@ -91,13 +91,6 @@ public class DetectionRegistry {
}
}
}
- // todo: get rid of MIGRATED_ALGORITHM and MIGRATED_ALGORITHM_FILTER
after the migration is completed
- if (REGISTRY_MAP.containsKey("ALGORITHM")) {
-REGISTRY_MAP.put("MIGRATED_ALGORITHM", REGISTRY_MAP.get("ALGORITHM"));
- }
- if (REGISTRY_MAP.containsKey("ALGORITHM_FILTER")) {
-REGISTRY_MAP.put("MIGRATED_ALGORITHM_FILTER",
REGISTRY_MAP.get("ALGORITHM_FILTER"));
- }
} catch (Exception e) {
LOG.warn("initialize detection registry error", e);
}
diff --git
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/DetectionConfigTuner.java
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/DetectionConfigTuner.java
index b3a91d5..171c36c 100644
---
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/DetectionConfigTuner.java
+++
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/DetectionConfigTuner.java
@@ -60,8 +60,6 @@ public class DetectionConfigTuner {
private static final DetectionRegistry DETECTION_REGISTRY =
DetectionRegistry.getInstance();
public static final String PROP_YAML_PARAMS = "yamlParams";
- public static final Set TURNOFF_TUNING_COMPONENTS =
- ImmutableSet.of("MIGRATED_ALGORITHM_FILTER", "MIGRATED_ALGORITHM",
"MIGRATED_ALGORITHM_BASELINE");
private final DetectionConfigDTO detectionConfig;
private final DataProvider dataProvider;
@@ -139,7 +137,7 @@ public class DetectionConfigTuner {
// For tunable components, the model params are computed from user
supplied yaml params and previous model params.
String componentClassName =
existingComponentProps.get(PROP_CLASS_NAME).toString();
String type = DetectionUtils.getComponentType(componentKey);
- if (!TURNOFF_TUNING_COMPONENTS.contains(type) &&
DETECTION_REGISTRY.isTunable(componentClassName)) {
+ if (DETECTION_REGISTRY.isTunable(componentClassName)) {
try {
tunedComponentProps.putAll(getTunedSpecs(existingComponentProps,
tuningWindowStart, tuningWindowEnd));
} catch (Exception e) {
diff --git
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/translator/DetectionConfigTranslator.java
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/translator/DetectionConfigTranslator.java
index 4481c41..1610818 100644
---
a/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/translator/DetectionConfigTranslator.java
+++
b/thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/detection/yaml/translator/DetectionConfigTranslator.java
@@ -169,7 +169,7 @@ public class DetectionConfigTranslator extends
ConfigTranslator MOVING_WINDOW_DETECTOR_TYPES =
ImmutableSet.of("ALGORITHM", "MIGRATED_ALGORITHM");
+ private static final Set MOVING_WINDOW_DETECTOR_TYPES =
ImmutableSet.of("ALGORITHM");
private final Map components = new HashMap<>();
private DataProvider dataProvider;
@@ -503,7 +503,7 @@ public class DetectionConfigTranslator extends
ConfigTranslator specs = (Map) entry.getValue();
if (entry.getKey().equals(ALGORITHM_TYPE)) {
extractTimeGranularitiesFromAlgorithmSpecs(specs,
PROP_BUCKET_PERIOD).ifPresent(monitoringGranularities::add);
- } else if (entry.getKey().equals(MIGRATED_ALGORITHM_TYPE)) {
-extractTimeGranularitiesFromAlgorithmSpecs(specs,
[GitHub] [incubator-pinot] Jackie-Jiang commented on issue #5230: Address extra comments in #5224
Jackie-Jiang commented on issue #5230: Address extra comments in #5224 URL: https://github.com/apache/incubator-pinot/pull/5230#issuecomment-611285848 Intentionally cancelled the test because no code change 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on issue #5221: Add a new server api for download of segments.
codecov-io commented on issue #5221: Add a new server api for download of segments. URL: https://github.com/apache/incubator-pinot/pull/5221#issuecomment-611284332 # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5221?src=pr=h1) Report > Merging [#5221](https://codecov.io/gh/apache/incubator-pinot/pull/5221?src=pr=desc) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1f1baf85fb318caa48b3b207e032946ddd5de11c=desc) will **increase** coverage by `0.13%`. > The diff coverage is `66.78%`. [](https://codecov.io/gh/apache/incubator-pinot/pull/5221?src=pr=tree) ```diff @@ Coverage Diff @@ ## master#5221 +/- ## + Coverage 65.90% 66.04% +0.13% Complexity 12 12 Files 1052 1057 +5 Lines 5417054187 +17 Branches 8078 8067 -11 + Hits 3570235787 +85 + Misses1581915766 -53 + Partials 2649 2634 -15 ``` | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/5221?src=pr=tree) | Coverage Δ | Complexity Δ | | |---|---|---|---| | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `76.66% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [.../BrokerResourceOnlineOfflineStateModelFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclJlc291cmNlT25saW5lT2ZmbGluZVN0YXRlTW9kZWxGYWN0b3J5LmphdmE=) | `55.81% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [.../pinot/broker/broker/helix/HelixBrokerStarter.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0hlbGl4QnJva2VyU3RhcnRlci5qYXZh) | `71.97% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [...thandler/SingleConnectionBrokerRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcmVxdWVzdGhhbmRsZXIvU2luZ2xlQ29ubmVjdGlvbkJyb2tlclJlcXVlc3RIYW5kbGVyLmphdmE=) | `92.68% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [...rg/apache/pinot/broker/routing/RoutingManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9Sb3V0aW5nTWFuYWdlci5qYXZh) | `80.00% <ø> (-1.16%)` | `0.00 <0.00> (ø)` | | | [...ting/instanceselector/InstanceSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3JGYWN0b3J5LmphdmE=) | `71.42% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [...er/routing/segmentpruner/SegmentPrunerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1NlZ21lbnRQcnVuZXJGYWN0b3J5LmphdmE=) | `83.33% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [...outing/segmentselector/SegmentSelectorFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50c2VsZWN0b3IvU2VnbWVudFNlbGVjdG9yRmFjdG9yeS5qYXZh) | `60.00% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [...oker/routing/timeboundary/TimeBoundaryManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy90aW1lYm91bmRhcnkvVGltZUJvdW5kYXJ5TWFuYWdlci5qYXZh) | `87.50% <ø> (ø)` | `0.00 <0.00> (ø)` | | | [...mmon/assignment/InstanceAssignmentConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZUFzc2lnbm1lbnRDb25maWdVdGlscy5qYXZh) | `72.50% <ø> (ø)` | `0.00 <0.00> (?)` | | | ... and [255 more](https://codecov.io/gh/apache/incubator-pinot/pull/5221/diff?src=pr=tree-more) | | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5221?src=pr=continue). > **Legend** - [Click here to learn
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #5221: Add a new server api for download of segments.
Jackie-Jiang commented on a change in pull request #5221: Add a new server api
for download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405908059
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,43 @@ public String getCrcMetadataForTable(
}
}
}
+
+ // TODO Add access control similar to
PinotSegmentUploadDownloadRestletResource for segment download.
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/segments/{tableNameWithType}/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableNameWithType")
String tableNameWithType,
+ @ApiParam(value = "Name of the segment", required = true)
@PathParam("segmentName") @Encoded String segmentName,
+ @Context HttpHeaders httpHeaders)
+ throws Exception {
+LOGGER.info("Received a request to download segment {} for table {}",
segmentName, tableNameWithType);
+TableDataManager tableDataManager =
checkGetTableDataManager(tableNameWithType);
+SegmentDataManager segmentDataManager =
tableDataManager.acquireSegment(segmentName);
+if (segmentDataManager == null) {
+ throw new WebApplicationException(
+ String.format("Table %s segment %s does not exist",
tableNameWithType, segmentName),
+ Response.Status.NOT_FOUND);
+}
+try {
+ String tableDir = tableDataManager.getTableDataDir();
+ // TODO Limit the number of concurrent downloads of segments because
compression is an expensive operation.
+ String tarFilePath =
TarGzCompressionUtils.createTarGzOfDirectory(tableDir + File.separator +
segmentName);
Review comment:
You should put all `.tar.gz` file in some temporary directory, and clean
them up whenever server restarted. Otherwise, if the server shut down when
downloading the segment, the segment will remain there forever
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
With regards,
Apache Git Services
-
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 #5221: Add a new server api for download of segments.
Jackie-Jiang commented on a change in pull request #5221: Add a new server api
for download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405907629
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##
@@ -211,4 +211,9 @@ private void closeSegment(SegmentDataManager
segmentDataManager) {
public String getTableName() {
return _tableNameWithType;
}
+
+ @Override
+ public String getTableDataDir() {
Review comment:
Recommend returning an File to avoid the edge case of trailing `\`
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
With regards,
Apache Git Services
-
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 #5221: Add a new server api for download of segments.
Jackie-Jiang commented on a change in pull request #5221: Add a new server api
for download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405906310
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,41 @@ public String getCrcMetadataForTable(
}
}
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/tables/{tableName}/segments/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableName") String
tableName,
+ @ApiParam(value = "Name of the segment", required = true)
@PathParam("segmentName") @Encoded String segmentName,
+ @Context HttpHeaders httpHeaders)
+ throws Exception {
+LOGGER.info("Get a request to download segment {} for table {}",
segmentName, tableName);
+TableDataManager tableDataManager = checkGetTableDataManager(tableName);
+SegmentDataManager segmentDataManager =
tableDataManager.acquireSegment(segmentName);
+if (segmentDataManager == null) {
+ throw new WebApplicationException(String.format("Table %s segments %s
does not exist", tableName, segmentName),
+ Response.Status.NOT_FOUND);
+}
+try {
Review comment:
@mcvsubbu Are we going to merge all these PRs together? We cannot deploy a
version without access control on segment download
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia commented on issue #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation
siddharthteotia commented on issue #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation URL: https://github.com/apache/incubator-pinot/pull/5226#issuecomment-611276567 Ran the profiler again locally to compare the numbers to previous run (when the doc id cache optimizations were implemented https://github.com/apache/incubator-pinot/pull/5199). No difference. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia merged pull request #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation
siddharthteotia merged pull request #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation URL: https://github.com/apache/incubator-pinot/pull/5226 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 With regards, Apache Git Services - 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 inter segment tests for text search and fix bug for Lucene query parser creation (#5226)
This is an automated email from the ASF dual-hosted git repository.
siddteotia 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 62a3e54 Add inter segment tests for text search and fix bug for
Lucene query parser creation (#5226)
62a3e54 is described below
commit 62a3e54f42fdc50aaa0f6e6f8f5b5be41857e506
Author: Sidd
AuthorDate: Wed Apr 8 18:25:55 2020 -0700
Add inter segment tests for text search and fix bug for Lucene query parser
creation (#5226)
* Add inter segment tests for text search
and fix bug for lucene query parser creation
Parser is JavaCC based. So it has to be created
per query.
Also added text search tests for the SQL path
* Create analyzer just once
Co-authored-by: Siddharth Teotia
---
.../index/readers/text/LuceneTextIndexReader.java | 10 +-
.../pinot/queries/TextSearchQueriesTest.java | 104 +
2 files changed, 111 insertions(+), 3 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
index 8e65921..3027242 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
@@ -54,9 +54,9 @@ public class LuceneTextIndexReader implements
InvertedIndexReader(textIndexColumns));
ImmutableSegment segment = ImmutableSegmentLoader.load(new File(INDEX_DIR,
SEGMENT_NAME), indexLoadingConfig);
_indexSegments.add(segment);
+_segmentDataManagers =
+Arrays.asList(new ImmutableSegmentDataManager(segment), new
ImmutableSegmentDataManager(segment));
}
private void createTestData()
@@ -1112,4 +1121,99 @@ public class TextSearchQueriesTest extends
BaseQueriesTest {
long count = (Long) operatorResult.getAggregationResult().get(0);
Assert.assertEquals(expectedCount, count);
}
+
+ @Test
+ public void testInterSegment() {
+String query = "SELECT count(*) FROM MyTable WHERE
TEXT_MATCH(SKILLS_TEXT_COL, '\"Machine learning\" AND \"Tensor flow\"') LIMIT
5";
+testInterSegmentAggregationQueryHelper(query, 12);
+query = "SELECT INT_COL, SKILLS_TEXT_COL FROM MyTable WHERE
TEXT_MATCH(SKILLS_TEXT_COL, '\"Machine learning\" AND \"Tensor flow\"') LIMIT
5";
+List expected = new ArrayList<>();
+expected.add(new Serializable[]{1004, "Machine learning, Tensor flow,
Java, Stanford university,"});
+expected.add(new Serializable[]{1007, "C++, Python, Tensor flow, database
kernel, storage, indexing and transaction processing, building large scale
systems, Machine learning"});
+expected.add(new Serializable[]{1016, "CUDA, GPU processing, Tensor flow,
Pandas, Python, Jupyter notebook, spark, Machine learning, building high
performance scalable systems"});
+expected.add(new Serializable[]{1004, "Machine learning, Tensor flow,
Java, Stanford university,"});
+expected.add(new Serializable[]{1007, "C++, Python, Tensor flow, database
kernel, storage, indexing and transaction processing, building large scale
systems, Machine learning"});
+expected.add(new Serializable[]{1016, "CUDA, GPU processing, Tensor flow,
Pandas, Python, Jupyter notebook, spark, Machine learning, building high
performance scalable systems"});
+expected.add(new Serializable[]{1004, "Machine learning, Tensor flow,
Java, Stanford university,"});
+expected.add(new Serializable[]{1007, "C++, Python, Tensor flow, database
kernel, storage, indexing and transaction processing, building large scale
systems, Machine learning"});
+expected.add(new Serializable[]{1016, "CUDA, GPU processing, Tensor flow,
Pandas, Python, Jupyter notebook, spark, Machine learning, building high
performance scalable systems"});
+expected.add(new Serializable[]{1004, "Machine learning, Tensor flow,
Java, Stanford university,"});
+expected.add(new Serializable[]{1007, "C++, Python, Tensor flow, database
kernel, storage, indexing and transaction processing, building large scale
systems, Machine learning"});
+expected.add(new Serializable[]{1016, "CUDA, GPU processing, Tensor flow,
Pandas, Python, Jupyter notebook, spark, Machine learning, building high
performance scalable systems"});
+testInterSegmentSelectionQueryHelper(query, expected);
+
+// try arbitrary filters in search expressions
+query = "SELECT count(*) FROM MyTable WHERE TEXT_MATCH(SKILLS_TEXT_COL,
'(\"distributed systems\" AND apache) OR (Java AND C++)') LIMIT 5";
+testInterSegmentAggregationQueryHelper(query, 36);
+query = "SELECT count(*) FROM MyTable WHERE TEXT_MATCH(SKILLS_TEXT_COL,
'(\"distributed systems\" AND
[GitHub] [incubator-pinot] siddharthteotia edited a comment on issue #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation
siddharthteotia edited a comment on issue #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation URL: https://github.com/apache/incubator-pinot/pull/5226#issuecomment-611276567 Ran the benchmark again locally to compare the numbers to previous run (when the doc id cache optimizations were implemented https://github.com/apache/incubator-pinot/pull/5199). No difference. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #5224: Support bootstrap mode for table rebalance
mcvsubbu commented on a change in pull request #5224: Support bootstrap mode
for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405902937
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java
##
@@ -230,7 +249,7 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
newAssignment = new TreeMap<>();
for (String segmentName : completedSegmentAssignment.keySet()) {
-List instancesAssigned = assignSegment(segmentName,
consumingInstancePartitions);
+List instancesAssigned = assignConsumingSegment(segmentName,
consumingInstancePartitions);
Review comment:
ah got 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
With regards,
Apache Git Services
-
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 #5224: Support bootstrap mode for table rebalance
Jackie-Jiang commented on a change in pull request #5224: Support bootstrap
mode for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405902698
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java
##
@@ -230,7 +249,7 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
newAssignment = new TreeMap<>();
for (String segmentName : completedSegmentAssignment.keySet()) {
-List instancesAssigned = assignSegment(segmentName,
consumingInstancePartitions);
+List instancesAssigned = assignConsumingSegment(segmentName,
consumingInstancePartitions);
Review comment:
This is intentional. This code path is for scenarios when completed instance
partitions are not provided (no segment relocation), we want to keep the
consuming partition on the correct server. This is useful when adding/removing
servers for real-time tables.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #5224: Support bootstrap mode for table rebalance
mcvsubbu commented on a change in pull request #5224: Support bootstrap mode
for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405899730
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java
##
@@ -230,7 +249,7 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
newAssignment = new TreeMap<>();
for (String segmentName : completedSegmentAssignment.keySet()) {
-List instancesAssigned = assignSegment(segmentName,
consumingInstancePartitions);
+List instancesAssigned = assignConsumingSegment(segmentName,
consumingInstancePartitions);
Review comment:
Is this a typo? Or are you intentionally calling assignConsumingSegment()
method?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #5224: Support bootstrap mode for table rebalance
mcvsubbu commented on a change in pull request #5224: Support bootstrap mode
for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405899394
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java
##
@@ -184,41 +199,45 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
LOGGER
.info("Reassigning COMPLETED segments with COMPLETED instance
partitions for table: {}", _realtimeTableName);
- if (completedInstancePartitions.getNumReplicaGroups() == 1) {
-// Non-replica-group based assignment
+ if (bootstrap) {
+LOGGER.info("Bootstrapping the COMPLETED segments for table: {}",
_realtimeTableName);
Review comment:
"Bootstrapping segment assignment for COMPLETED segments..."
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #5224: Support bootstrap mode for table rebalance
mcvsubbu commented on a change in pull request #5224: Support bootstrap mode
for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405897839
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignment.java
##
@@ -189,45 +163,48 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
InstancePartitions instancePartitions =
instancePartitionsMap.get(InstancePartitionsType.OFFLINE);
Preconditions.checkState(instancePartitions != null, "Failed to find
OFFLINE instance partitions for table: %s",
_offlineTableName);
-LOGGER.info("Rebalancing table: {} with instance partitions: {}",
_offlineTableName, instancePartitions);
+boolean bootstrap =
+config.getBoolean(RebalanceConfigConstants.BOOTSTRAP,
RebalanceConfigConstants.DEFAULT_BOOTSTRAP);
+LOGGER.info("Rebalancing table: {} with instance partitions: {},
bootstrap: {}", _offlineTableName,
+instancePartitions, bootstrap);
+checkReplication(instancePartitions);
Map> newAssignment;
-if (instancePartitions.getNumReplicaGroups() == 1) {
- // Non-replica-group based assignment
+if (bootstrap) {
+ LOGGER.info("Bootstrapping the table: {}", _offlineTableName);
Review comment:
Suggest wording : "Bootstrapping segment assignment for table ..." Otherwise
it may throw an admin off
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #5224: Support bootstrap mode for table rebalance
mcvsubbu commented on a change in pull request #5224: Support bootstrap mode
for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405756684
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignment.java
##
@@ -99,49 +99,48 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
_offlineTableName);
LOGGER.info("Assigning segment: {} with instance partitions: {} for table:
{}", segmentName, instancePartitions,
_offlineTableName);
+checkReplication(instancePartitions);
-List instancesAssigned;
-if (instancePartitions.getNumReplicaGroups() == 1) {
+List instancesAssigned = assignSegment(segmentName,
currentAssignment, instancePartitions);
+
+LOGGER
+.info("Assigned segment: {} to instances: {} for table: {}",
segmentName, instancesAssigned, _offlineTableName);
+return instancesAssigned;
+ }
+
+ /**
+ * Helper method to check whether the number of replica-groups matches the
table replication for replica-group based
Review comment:
When will this be the case? Can you also add that in the comments?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: enable async processing in pinot broker query api (#5229)
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 6943212 enable async processing in pinot broker query api (#5229)
6943212 is described below
commit 6943212f8526beb1a6dbb5ccf1437af6fc59465c
Author: James Shao
AuthorDate: Wed Apr 8 17:50:51 2020 -0700
enable async processing in pinot broker query api (#5229)
right now pinot broker query api issue a blocking call when it try to fetch
result from pinot server. If we are seeing high amount of traffic to pinot
broker with long request process latency, the whole pinot broker instances
could be blocked due to limited amount of processing threads available to
grizzly java web container. This change help to address the issue related to
blocking call and ensure pinot broker have capacity to handle request even when
some of the broker requests are [...]
---
.../broker/api/resources/PinotClientRequest.java | 35 ++
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git
a/pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java
b/pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java
index 9f03494..290de8c 100644
---
a/pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java
+++
b/pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java
@@ -32,6 +32,8 @@ import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.QueryParam;
import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.container.AsyncResponse;
+import javax.ws.rs.container.Suspended;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.apache.pinot.broker.api.RequestStatistics;
@@ -41,6 +43,7 @@ import org.apache.pinot.common.metrics.BrokerMetrics;
import org.apache.pinot.common.response.BrokerResponse;
import org.apache.pinot.common.utils.CommonConstants.Broker.Request;
import org.apache.pinot.spi.utils.JsonUtils;
+import org.glassfish.jersey.server.ManagedAsync;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -57,15 +60,17 @@ public class PinotClientRequest {
private BrokerMetrics brokerMetrics;
@GET
+ @ManagedAsync
@Produces(MediaType.APPLICATION_JSON)
@Path("query")
@ApiOperation(value = "Querying pinot")
@ApiResponses(value = {@ApiResponse(code = 200, message = "Query response"),
@ApiResponse(code = 500, message = "Internal Server Error")})
- public String processQueryGet(
+ public void processQueryGet(
// Query param "bql" is for backward compatibility
@ApiParam(value = "Query", required = true) @QueryParam("bql") String
query,
@ApiParam(value = "Trace enabled") @QueryParam(Request.TRACE) String
traceEnabled,
- @ApiParam(value = "Debug options") @QueryParam(Request.DEBUG_OPTIONS)
String debugOptions) {
+ @ApiParam(value = "Debug options") @QueryParam(Request.DEBUG_OPTIONS)
String debugOptions,
+ @Suspended AsyncResponse asyncResponse) {
try {
ObjectNode requestJson = JsonUtils.newObjectNode();
requestJson.put(Request.PQL, query);
@@ -76,24 +81,25 @@ public class PinotClientRequest {
requestJson.put(Request.DEBUG_OPTIONS, debugOptions);
}
BrokerResponse brokerResponse =
requestHandler.handleRequest(requestJson, null, new RequestStatistics());
- return brokerResponse.toJsonString();
+ asyncResponse.resume(brokerResponse.toJsonString());
} catch (Exception e) {
LOGGER.error("Caught exception while processing GET request", e);
brokerMetrics.addMeteredGlobalValue(BrokerMeter.UNCAUGHT_GET_EXCEPTIONS,
1L);
- throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
+ asyncResponse.resume(new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR));
}
}
@POST
+ @ManagedAsync
@Produces(MediaType.APPLICATION_JSON)
@Path("query")
@ApiOperation(value = "Querying pinot")
@ApiResponses(value = {@ApiResponse(code = 200, message = "Query response"),
@ApiResponse(code = 500, message = "Internal Server Error")})
- public String processQueryPost(String query) {
+ public void processQueryPost(String query, @Suspended AsyncResponse
asyncResponse) {
try {
JsonNode requestJson = JsonUtils.stringToJsonNode(query);
BrokerResponse brokerResponse =
requestHandler.handleRequest(requestJson, null, new RequestStatistics());
- return brokerResponse.toJsonString();
+ asyncResponse.resume(brokerResponse);
} catch (Exception e) {
LOGGER.error("Caught exception while processing POST request", e);
brokerMetrics.addMeteredGlobalValue(BrokerMeter.UNCAUGHT_POST_EXCEPTIONS, 1L);
@@ -102,13 +108,15 @@ public class
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5229: enable async processing in pinot broker query api
Jackie-Jiang merged pull request #5229: enable async processing in pinot broker query api URL: https://github.com/apache/incubator-pinot/pull/5229 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (60bc83a -> 8f0d059)
This is an automated email from the ASF dual-hosted git repository. jackie pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 60bc83a Revert "[TE][subscription] update subscription watermarks to use anomaly create time instead of end time (#5152)" (#5227) add 8f0d059 Support bootstrap mode for table rebalance (#5224) No new revisions were added by this update. Summary of changes: .../api/resources/PinotTableRestletResource.java | 2 + .../segment/OfflineSegmentAssignment.java | 159 + .../segment/RealtimeSegmentAssignment.java | 131 +++-- .../assignment/segment/SegmentAssignmentUtils.java | 49 +++ .../core/rebalance/RebalanceConfigConstants.java | 5 + .../helix/core/rebalance/TableRebalancer.java | 8 +- ...fflineNonReplicaGroupSegmentAssignmentTest.java | 30 +++- .../OfflineReplicaGroupSegmentAssignmentTest.java | 69 - ...altimeNonReplicaGroupSegmentAssignmentTest.java | 59 +--- .../RealtimeReplicaGroupSegmentAssignmentTest.java | 61 +--- .../apache/pinot/tools/PinotTableRebalancer.java | 4 +- .../tools/admin/command/RebalanceTableCommand.java | 13 +- 12 files changed, 412 insertions(+), 178 deletions(-) - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5224: Support bootstrap mode for table rebalance
Jackie-Jiang merged pull request #5224: Support bootstrap mode for table rebalance URL: https://github.com/apache/incubator-pinot/pull/5224 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405892246
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java
##
@@ -149,4 +160,62 @@ public void testSegmentCrcMetadata()
Assert.assertEquals(segmentsCrc.get(segmentName).asText(), crc);
}
}
+
+ @Test
+ public void testDownloadSegments()
+ throws Exception {
+// Verify the content of the downloaded segment from a realtime table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(REALTIME_TABLE_NAME,
_realtimeIndexSegments.get(0)));
+// Verify the content of the downloaded segment from an offline table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(OFFLINE_TABLE_NAME,
_offlineIndexSegments.get(0)));
+
+// Verify non-existent table and segment download return NOT_FOUND status.
+Response response =
_webTarget.path("/tables/UNKNOWN_REALTIME/segments/segmentname").request()
+.get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+
+response = _webTarget.path("/tables/" + REALTIME_TABLE_NAME +
"/segments/UNKNOWN_SEGMENT").request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+ }
+
+ // Verify metadata file from segments.
+ private boolean downLoadAndVerifySegmentContent(String tableNameWithType,
IndexSegment segment) {
+String segmentPath = "/tables/" + tableNameWithType + "/segments/" +
segment.getSegmentName();
+
+// Download the segment and save to a temp local file.
+Response response =
_webTarget.path(segmentPath).request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.OK.getStatusCode());
+File segmentFile = response.readEntity(File.class);
+
+File tempMetadataDir = new File(FileUtils.getTempDirectory(),
"segment_metadata");
+try (// Extract metadata.properties
+InputStream metadataPropertiesInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.MetadataKeys.METADATA_FILE_NAME);
+// Extract creation.meta
+InputStream creationMetaInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.SEGMENT_CREATION_META)) {
+ Preconditions
+ .checkState(tempMetadataDir.mkdirs(), "Failed to create directory:
%s", tempMetadataDir.getAbsolutePath());
+
+ Preconditions.checkNotNull(metadataPropertiesInputStream, "%s does not
exist",
+ V1Constants.MetadataKeys.METADATA_FILE_NAME);
+ java.nio.file.Path metadataPropertiesPath = FileSystems.getDefault()
+ .getPath(tempMetadataDir.getAbsolutePath(),
V1Constants.MetadataKeys.METADATA_FILE_NAME);
+ Files.copy(metadataPropertiesInputStream, metadataPropertiesPath);
+
+ Preconditions.checkNotNull(creationMetaInputStream, "%s does not exist",
V1Constants.SEGMENT_CREATION_META);
+ java.nio.file.Path creationMetaPath =
+ FileSystems.getDefault().getPath(tempMetadataDir.getAbsolutePath(),
V1Constants.SEGMENT_CREATION_META);
+ Files.copy(creationMetaInputStream, creationMetaPath);
+ // Load segment metadata
+ SegmentMetadataImpl metadata = new SegmentMetadataImpl(tempMetadataDir);
+
+ Assert.assertEquals(tableNameWithType, metadata.getTableName());
+ return true;
+} catch (Exception e) {
+ return false;
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405891375
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,41 @@ public String getCrcMetadataForTable(
}
}
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/tables/{tableName}/segments/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableName") String
tableName,
+ @ApiParam(value = "Name of the segment", required = true)
@PathParam("segmentName") @Encoded String segmentName,
+ @Context HttpHeaders httpHeaders)
+ throws Exception {
+LOGGER.info("Get a request to download segment {} for table {}",
segmentName, tableName);
+TableDataManager tableDataManager = checkGetTableDataManager(tableName);
+SegmentDataManager segmentDataManager =
tableDataManager.acquireSegment(segmentName);
+if (segmentDataManager == null) {
+ throw new WebApplicationException(String.format("Table %s segments %s
does not exist", tableName, segmentName),
+ Response.Status.NOT_FOUND);
+}
+try {
+ String tableDir = tableDataManager.getTableDataDir();
+ String tarFilePath =
TarGzCompressionUtils.createTarGzOfDirectory(tableDir + "/" + segmentName);
Review comment:
Definitely this is a place we need to monitor for performance and
optimization if needed. Left TODO here as also suggested also by Subbu.
To you question if/which compression level is used, createTarGzOfDirectory()
in Pinot TarGzCompressionUtils uses the default compression level by Gzip. We
could also tune the level by specifying other levels (9 level in total) like
BEST_SPEED/NO_COMPRESSION and so on.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405887423
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,41 @@ public String getCrcMetadataForTable(
}
}
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/tables/{tableName}/segments/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableName") String
tableName,
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405886331
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java
##
@@ -149,4 +160,62 @@ public void testSegmentCrcMetadata()
Assert.assertEquals(segmentsCrc.get(segmentName).asText(), crc);
}
}
+
+ @Test
+ public void testDownloadSegments()
+ throws Exception {
+// Verify the content of the downloaded segment from a realtime table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(REALTIME_TABLE_NAME,
_realtimeIndexSegments.get(0)));
+// Verify the content of the downloaded segment from an offline table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(OFFLINE_TABLE_NAME,
_offlineIndexSegments.get(0)));
+
+// Verify non-existent table and segment download return NOT_FOUND status.
+Response response =
_webTarget.path("/tables/UNKNOWN_REALTIME/segments/segmentname").request()
+.get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+
+response = _webTarget.path("/tables/" + REALTIME_TABLE_NAME +
"/segments/UNKNOWN_SEGMENT").request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+ }
+
+ // Verify metadata file from segments.
+ private boolean downLoadAndVerifySegmentContent(String tableNameWithType,
IndexSegment segment) {
+String segmentPath = "/tables/" + tableNameWithType + "/segments/" +
segment.getSegmentName();
+
+// Download the segment and save to a temp local file.
+Response response =
_webTarget.path(segmentPath).request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.OK.getStatusCode());
+File segmentFile = response.readEntity(File.class);
+
+File tempMetadataDir = new File(FileUtils.getTempDirectory(),
"segment_metadata");
+try (// Extract metadata.properties
+InputStream metadataPropertiesInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.MetadataKeys.METADATA_FILE_NAME);
+// Extract creation.meta
+InputStream creationMetaInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.SEGMENT_CREATION_META)) {
+ Preconditions
+ .checkState(tempMetadataDir.mkdirs(), "Failed to create directory:
%s", tempMetadataDir.getAbsolutePath());
+
+ Preconditions.checkNotNull(metadataPropertiesInputStream, "%s does not
exist",
+ V1Constants.MetadataKeys.METADATA_FILE_NAME);
+ java.nio.file.Path metadataPropertiesPath = FileSystems.getDefault()
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405886026
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/TableSizeResourceTest.java
##
@@ -37,9 +37,9 @@ public void testTableSizeNotFound() {
@Test
public void testTableSizeDetailed() {
TableSizeInfo tableSizeInfo =
_webTarget.path(TABLE_SIZE_PATH).request().get(TableSizeInfo.class);
-ImmutableSegment defaultSegment = _indexSegments.get(0);
+ImmutableSegment defaultSegment = _realtimeIndexSegments.get(0);
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
With regards,
Apache Git Services
-
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 #5224: Support bootstrap mode for table rebalance
Jackie-Jiang commented on a change in pull request #5224: Support bootstrap
mode for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405880741
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java
##
@@ -184,41 +199,45 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
LOGGER
.info("Reassigning COMPLETED segments with COMPLETED instance
partitions for table: {}", _realtimeTableName);
- if (completedInstancePartitions.getNumReplicaGroups() == 1) {
-// Non-replica-group based assignment
+ if (bootstrap) {
+LOGGER.info("Bootstrapping the COMPLETED segments for table: {}",
_realtimeTableName);
-List instances = SegmentAssignmentUtils
-
.getInstancesForNonReplicaGroupBasedAssignment(completedInstancePartitions,
_replication);
-newAssignment = SegmentAssignmentUtils
-
.rebalanceTableWithHelixAutoRebalanceStrategy(completedSegmentAssignment,
instances, _replication);
+// When bootstrap is enabled, start with an empty table add reassign
all segments
Review comment:
Added more comments
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
With regards,
Apache Git Services
-
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 #5224: Support bootstrap mode for table rebalance
Jackie-Jiang commented on a change in pull request #5224: Support bootstrap
mode for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405880593
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java
##
@@ -86,6 +86,55 @@ private SegmentAssignmentUtils() {
return instances;
}
+ /**
+ * Assigns the segment for the non-replica-group based segment assignment
strategy and returns the assigned instances.
+ */
+ static List assignSegmentWithoutReplicaGroup(Map> currentAssignment,
+ InstancePartitions instancePartitions, int replication) {
+List instances =
+
SegmentAssignmentUtils.getInstancesForNonReplicaGroupBasedAssignment(instancePartitions,
replication);
+int[] numSegmentsAssignedPerInstance =
getNumSegmentsAssignedPerInstance(currentAssignment, instances);
+int numInstances = numSegmentsAssignedPerInstance.length;
+PriorityQueue heap = new PriorityQueue<>(numInstances,
Pairs.intPairComparator());
Review comment:
Yes, it is sorted on the left value firstly, then right value secondly.
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang closed pull request #5228: Remove the cache from Travis
Jackie-Jiang closed pull request #5228: Remove the cache from Travis URL: https://github.com/apache/incubator-pinot/pull/5228 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 With regards, Apache Git Services - 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 issue #5228: Remove the cache from Travis
Jackie-Jiang commented on issue #5228: Remove the cache from Travis URL: https://github.com/apache/incubator-pinot/pull/5228#issuecomment-611251864 Without cache, each job is taking about 1-2 minutes longer to install. Closing this 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405877782
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/data/manager/BaseTableDataManager.java
##
@@ -211,4 +211,7 @@ private void closeSegment(SegmentDataManager
segmentDataManager) {
public String getTableName() {
return _tableNameWithType;
}
+
+ @Override
+ public String getTableDataDir() { return _tableDataDir; }
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405877526
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,41 @@ public String getCrcMetadataForTable(
}
}
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/tables/{tableName}/segments/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableName") String
tableName,
+ @ApiParam(value = "Name of the segment", required = true)
@PathParam("segmentName") @Encoded String segmentName,
+ @Context HttpHeaders httpHeaders)
+ throws Exception {
+LOGGER.info("Get a request to download segment {} for table {}",
segmentName, tableName);
+TableDataManager tableDataManager = checkGetTableDataManager(tableName);
+SegmentDataManager segmentDataManager =
tableDataManager.acquireSegment(segmentName);
+if (segmentDataManager == null) {
+ throw new WebApplicationException(String.format("Table %s segments %s
does not exist", tableName, segmentName),
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405877346
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java
##
@@ -55,11 +56,12 @@
public abstract class BaseResourceTest {
private static final String AVRO_DATA_PATH = "data/test_data-mv.avro";
private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(),
"BaseResourceTest");
- protected static final String TABLE_NAME = "testTable";
+ protected static final String REALTIME_TABLE_NAME = "testTable_REALTIME";
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] snleee removed a comment on issue #5228: Remove the cache from Travis
snleee removed a comment on issue #5228: Remove the cache from Travis URL: https://github.com/apache/incubator-pinot/pull/5228#issuecomment-611248507 As long as the execution time is similar, LGTM 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 With regards, Apache Git Services - 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 issue #5228: Remove the cache from Travis
snleee commented on issue #5228: Remove the cache from Travis URL: https://github.com/apache/incubator-pinot/pull/5228#issuecomment-611248507 As long as the execution time is similar, LGTM 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405871278
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,41 @@ public String getCrcMetadataForTable(
}
}
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/tables/{tableName}/segments/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableName") String
tableName,
+ @ApiParam(value = "Name of the segment", required = true)
@PathParam("segmentName") @Encoded String segmentName,
+ @Context HttpHeaders httpHeaders)
+ throws Exception {
+LOGGER.info("Get a request to download segment {} for table {}",
segmentName, tableName);
+TableDataManager tableDataManager = checkGetTableDataManager(tableName);
+SegmentDataManager segmentDataManager =
tableDataManager.acquireSegment(segmentName);
+if (segmentDataManager == null) {
+ throw new WebApplicationException(String.format("Table %s segments %s
does not exist", tableName, segmentName),
+ Response.Status.NOT_FOUND);
+}
+try {
+ String tableDir = tableDataManager.getTableDataDir();
+ String tarFilePath =
TarGzCompressionUtils.createTarGzOfDirectory(tableDir + "/" + segmentName);
Review comment:
TODO 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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405868731
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,41 @@ public String getCrcMetadataForTable(
}
}
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/tables/{tableName}/segments/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableName") String
tableName,
+ @ApiParam(value = "Name of the segment", required = true)
@PathParam("segmentName") @Encoded String segmentName,
+ @Context HttpHeaders httpHeaders)
+ throws Exception {
+LOGGER.info("Get a request to download segment {} for table {}",
segmentName, tableName);
+TableDataManager tableDataManager = checkGetTableDataManager(tableName);
+SegmentDataManager segmentDataManager =
tableDataManager.acquireSegment(segmentName);
+if (segmentDataManager == null) {
+ throw new WebApplicationException(String.format("Table %s segments %s
does not exist", tableName, segmentName),
+ Response.Status.NOT_FOUND);
+}
+try {
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] chenboat commented on a change in pull request #5221: Add a new server api for download of segments.
chenboat commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405867876
##
File path:
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
##
@@ -175,4 +183,41 @@ public String getCrcMetadataForTable(
}
}
}
+
+ @GET
+ @Produces(MediaType.APPLICATION_OCTET_STREAM)
+ @Path("/tables/{tableName}/segments/{segmentName}")
+ @ApiOperation(value = "Download a segment", notes = "Download a segment in
zipped tar format")
+ public Response downloadSegment(
+ @ApiParam(value = "Name of the table with type REALTIME OR OFFLINE",
required = true, example = "myTable_OFFLINE") @PathParam("tableName") String
tableName,
+ @ApiParam(value = "Name of the segment", required = true)
@PathParam("segmentName") @Encoded String segmentName,
+ @Context HttpHeaders httpHeaders)
+ throws Exception {
+LOGGER.info("Get a request to download segment {} for table {}",
segmentName, tableName);
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (06bd2c6 -> 60bc83a)
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 06bd2c6 [TE] endpoint - harleyjj/metricBreakdownPipeline - add flag for ignoring score when adding dimensions to response (#5212) add 60bc83a Revert "[TE][subscription] update subscription watermarks to use anomaly create time instead of end time (#5152)" (#5227) No new revisions were added by this update. Summary of changes: .../datalayer/bao/MergedAnomalyResultManager.java | 2 +- .../bao/jdbc/MergedAnomalyResultManagerImpl.java | 7 ++- .../datalayer/pojo/DetectionAlertConfigBean.java | 21 +-- .../thirdeye/detection/DefaultDataProvider.java| 3 --- .../pinot/thirdeye/detection/alert/AlertUtils.java | 9 +++- .../detection/alert/DetectionAlertJob.java | 17 --- .../detection/alert/DetectionAlertTaskRunner.java | 24 +- .../alert/StatefulDetectionAlertFilter.java| 18 +--- .../filter/DimensionsRecipientAlertFilter.java | 17 +-- .../alert/filter/PerUserDimensionAlertFilter.java | 18 ++-- .../detection/alert/filter/SubscriptionUtils.java | 1 + .../ToAllRecipientsDetectionAlertFilter.java | 22 ++-- .../components/ThresholdRuleDetector.java | 2 +- .../validators/SubscriptionConfigValidator.java| 5 + .../translator/SubscriptionConfigTranslator.java | 1 + .../thirdeye/detection/alert/SendAlertTest.java| 6 ++ .../ToAllRecipientsDetectionAlertFilterTest.java | 19 + 17 files changed, 130 insertions(+), 62 deletions(-) - 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 #5227: Revert "[TE][subscription] update subscription watermarks to use anom…
akshayrai merged pull request #5227: Revert "[TE][subscription] update subscription watermarks to use anom… URL: https://github.com/apache/incubator-pinot/pull/5227 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 With regards, Apache Git Services - 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 #5224: Support bootstrap mode for table rebalance
snleee commented on a change in pull request #5224: Support bootstrap mode for
table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405844234
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java
##
@@ -86,6 +86,55 @@ private SegmentAssignmentUtils() {
return instances;
}
+ /**
+ * Assigns the segment for the non-replica-group based segment assignment
strategy and returns the assigned instances.
+ */
+ static List assignSegmentWithoutReplicaGroup(Map> currentAssignment,
+ InstancePartitions instancePartitions, int replication) {
+List instances =
+
SegmentAssignmentUtils.getInstancesForNonReplicaGroupBasedAssignment(instancePartitions,
replication);
+int[] numSegmentsAssignedPerInstance =
getNumSegmentsAssignedPerInstance(currentAssignment, instances);
+int numInstances = numSegmentsAssignedPerInstance.length;
+PriorityQueue heap = new PriorityQueue<>(numInstances,
Pairs.intPairComparator());
Review comment:
I assume that `Pairs.intPairComparator` will be an ascending order?
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
With regards,
Apache Git Services
-
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 #5224: Support bootstrap mode for table rebalance
snleee commented on a change in pull request #5224: Support bootstrap mode for
table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405843529
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/RealtimeSegmentAssignment.java
##
@@ -184,41 +199,45 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
LOGGER
.info("Reassigning COMPLETED segments with COMPLETED instance
partitions for table: {}", _realtimeTableName);
- if (completedInstancePartitions.getNumReplicaGroups() == 1) {
-// Non-replica-group based assignment
+ if (bootstrap) {
+LOGGER.info("Bootstrapping the COMPLETED segments for table: {}",
_realtimeTableName);
-List instances = SegmentAssignmentUtils
-
.getInstancesForNonReplicaGroupBasedAssignment(completedInstancePartitions,
_replication);
-newAssignment = SegmentAssignmentUtils
-
.rebalanceTableWithHelixAutoRebalanceStrategy(completedSegmentAssignment,
instances, _replication);
+// When bootstrap is enabled, start with an empty table add reassign
all segments
Review comment:
`empty table -> empty assignment`?
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
With regards,
Apache Git Services
-
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 #5224: Support bootstrap mode for table rebalance
snleee commented on a change in pull request #5224: Support bootstrap mode for
table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405844234
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/SegmentAssignmentUtils.java
##
@@ -86,6 +86,55 @@ private SegmentAssignmentUtils() {
return instances;
}
+ /**
+ * Assigns the segment for the non-replica-group based segment assignment
strategy and returns the assigned instances.
+ */
+ static List assignSegmentWithoutReplicaGroup(Map> currentAssignment,
+ InstancePartitions instancePartitions, int replication) {
+List instances =
+
SegmentAssignmentUtils.getInstancesForNonReplicaGroupBasedAssignment(instancePartitions,
replication);
+int[] numSegmentsAssignedPerInstance =
getNumSegmentsAssignedPerInstance(currentAssignment, instances);
+int numInstances = numSegmentsAssignedPerInstance.length;
+PriorityQueue heap = new PriorityQueue<>(numInstances,
Pairs.intPairComparator());
Review comment:
I assume `Pairs.intPairComparator` that this will be an ascending order?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (a1401de -> 06bd2c6)
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 a1401de [TE] frontend - harleyjj/home - use duration param to set date range (#5198) add 06bd2c6 [TE] endpoint - harleyjj/metricBreakdownPipeline - add flag for ignoring score when adding dimensions to response (#5212) No new revisions were added by this update. Summary of changes: .../rootcause/impl/MetricBreakdownPipeline.java | 19 --- 1 file changed, 16 insertions(+), 3 deletions(-) - 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 #5212: [TE] endpoint - harleyjj/metricBreakdownPipeline - add flag for ignor…
akshayrai merged pull request #5212: [TE] endpoint - harleyjj/metricBreakdownPipeline - add flag for ignor… URL: https://github.com/apache/incubator-pinot/pull/5212 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] harleyjj commented on a change in pull request #5212: [TE] endpoint - harleyjj/metricBreakdownPipeline - add flag for ignor…
harleyjj commented on a change in pull request #5212: [TE] endpoint -
harleyjj/metricBreakdownPipeline - add flag for ignor…
URL: https://github.com/apache/incubator-pinot/pull/5212#discussion_r405840469
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/rootcause/impl/MetricBreakdownPipeline.java
##
@@ -107,9 +112,11 @@
* @param includeDimensions dimensions to break down on (all if empty)
* @param excludeDimensions dimensions not to break down on
* @param k number of top-ranking elements to emit
+ * @param ignoreScore flag to include all breakdowns, even if score is zero
(only set if ignoring wanted)
*/
public MetricBreakdownPipeline(String outputName, Set inputNames,
MetricConfigManager metricDAO,
- DatasetConfigManager datasetDAO, QueryCache cache, ExecutorService
executor, Set includeDimensions, Set excludeDimensions, int k) {
+ DatasetConfigManager datasetDAO, QueryCache cache, ExecutorService
executor, Set includeDimensions,
+ Set excludeDimensions, int k, boolean ignoreScore) {
Review comment:
The rca.yml configuration file is where we trigger and configure the
framework. From the RCA Dev guide:
"Every pipeline configuration consists of four fields: "outputName"
respresents the name of the pipeline. "inputNames" is a list of pipeline names
that deliver their outputs as input to the current pipeline. "className"
specifies the fully qualified class name of the business logic implementing
this pipeline. And finally "properties" holds a map of String-Object tuples
that are passed to the pipeline constructor as configuration parameters (see
"coding conventions" below). The properties map is deserialized using and
ObjectMapper and can contain nested lists and maps in addition to primitive
values."
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jamesyfshao commented on a change in pull request #5229: enable async processing in pinot broker query api
jamesyfshao commented on a change in pull request #5229: enable async
processing in pinot broker query api
URL: https://github.com/apache/incubator-pinot/pull/5229#discussion_r405838239
##
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java
##
@@ -76,24 +85,28 @@ public String processQueryGet(
requestJson.put(Request.DEBUG_OPTIONS, debugOptions);
}
BrokerResponse brokerResponse =
requestHandler.handleRequest(requestJson, null, new RequestStatistics());
- return brokerResponse.toJsonString();
+ asyncResponse.resume(brokerResponse.toJsonString());
} catch (Exception e) {
LOGGER.error("Caught exception while processing GET request", e);
brokerMetrics.addMeteredGlobalValue(BrokerMeter.UNCAUGHT_GET_EXCEPTIONS,
1L);
- throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
+ asyncResponse.resume(new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR));
}
}
@POST
+ @ManagedAsync
@Produces(MediaType.APPLICATION_JSON)
@Path("query")
@ApiOperation(value = "Querying pinot")
@ApiResponses(value = {@ApiResponse(code = 200, message = "Query response"),
@ApiResponse(code = 500, message = "Internal Server Error")})
- public String processQueryPost(String query) {
+ public void processQueryPost(String query, @Suspended final AsyncResponse
asyncResponse) {
try {
JsonNode requestJson = JsonUtils.stringToJsonNode(query);
+ if (requestJson.has("trace")) {
+TimeUnit.SECONDS.sleep(15);
Review comment:
ops, left over from my previous testing and it is supposed to be removed
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
With regards,
Apache Git Services
-
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 #5212: [TE] endpoint - harleyjj/metricBreakdownPipeline - add flag for ignor…
akshayrai commented on a change in pull request #5212: [TE] endpoint -
harleyjj/metricBreakdownPipeline - add flag for ignor…
URL: https://github.com/apache/incubator-pinot/pull/5212#discussion_r405833146
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/rootcause/impl/MetricBreakdownPipeline.java
##
@@ -107,9 +112,11 @@
* @param includeDimensions dimensions to break down on (all if empty)
* @param excludeDimensions dimensions not to break down on
* @param k number of top-ranking elements to emit
+ * @param ignoreScore flag to include all breakdowns, even if score is zero
(only set if ignoring wanted)
*/
public MetricBreakdownPipeline(String outputName, Set inputNames,
MetricConfigManager metricDAO,
- DatasetConfigManager datasetDAO, QueryCache cache, ExecutorService
executor, Set includeDimensions, Set excludeDimensions, int k) {
+ DatasetConfigManager datasetDAO, QueryCache cache, ExecutorService
executor, Set includeDimensions,
+ Set excludeDimensions, int k, boolean ignoreScore) {
Review comment:
Who is calling this or rather how is this being triggered? I do not see any
updates to the caller in this 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
With regards,
Apache Git Services
-
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 #5229: enable async processing in pinot broker query api
Jackie-Jiang commented on a change in pull request #5229: enable async
processing in pinot broker query api
URL: https://github.com/apache/incubator-pinot/pull/5229#discussion_r405832392
##
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java
##
@@ -76,24 +85,28 @@ public String processQueryGet(
requestJson.put(Request.DEBUG_OPTIONS, debugOptions);
}
BrokerResponse brokerResponse =
requestHandler.handleRequest(requestJson, null, new RequestStatistics());
- return brokerResponse.toJsonString();
+ asyncResponse.resume(brokerResponse.toJsonString());
} catch (Exception e) {
LOGGER.error("Caught exception while processing GET request", e);
brokerMetrics.addMeteredGlobalValue(BrokerMeter.UNCAUGHT_GET_EXCEPTIONS,
1L);
- throw new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR);
+ asyncResponse.resume(new WebApplicationException(e,
Response.Status.INTERNAL_SERVER_ERROR));
}
}
@POST
+ @ManagedAsync
@Produces(MediaType.APPLICATION_JSON)
@Path("query")
@ApiOperation(value = "Querying pinot")
@ApiResponses(value = {@ApiResponse(code = 200, message = "Query response"),
@ApiResponse(code = 500, message = "Internal Server Error")})
- public String processQueryPost(String query) {
+ public void processQueryPost(String query, @Suspended final AsyncResponse
asyncResponse) {
try {
JsonNode requestJson = JsonUtils.stringToJsonNode(query);
+ if (requestJson.has("trace")) {
+TimeUnit.SECONDS.sleep(15);
Review comment:
What is this sleep for?
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
With regards,
Apache Git Services
-
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 #5229: enable async processing in pinot broker query api
Jackie-Jiang commented on a change in pull request #5229: enable async processing in pinot broker query api URL: https://github.com/apache/incubator-pinot/pull/5229#discussion_r405831846 ## File path: pinot-broker/src/main/java/org/apache/pinot/broker/api/resources/PinotClientRequest.java ## @@ -32,18 +32,25 @@ import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; import javax.ws.rs.WebApplicationException; +import javax.ws.rs.container.AsyncResponse; +import javax.ws.rs.container.Suspended; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import org.apache.pinot.broker.api.RequestStatistics; import org.apache.pinot.broker.requesthandler.BrokerRequestHandler; +import org.apache.pinot.broker.requesthandler.PinotQueryRequest; import org.apache.pinot.common.metrics.BrokerMeter; import org.apache.pinot.common.metrics.BrokerMetrics; import org.apache.pinot.common.response.BrokerResponse; +import org.apache.pinot.common.utils.CommonConstants; import org.apache.pinot.common.utils.CommonConstants.Broker.Request; import org.apache.pinot.spi.utils.JsonUtils; +import org.glassfish.jersey.server.ManagedAsync; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.concurrent.TimeUnit; Review comment: (format) re-order the imports 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 With regards, Apache Git Services - 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 #5208: [TE] Disable alerts if it has no success run within 30 days
akshayrai commented on a change in pull request #5208: [TE] Disable alerts if
it has no success run within 30 days
URL: https://github.com/apache/incubator-pinot/pull/5208#discussion_r405811811
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/monitor/MonitorTaskRunner.java
##
@@ -120,11 +134,62 @@ private void executeMonitorUpdate(MonitorTaskInfo
monitorTaskInfo) {
// update detection health
updateDetectionHealth();
+
+ // disable alerts that failed consecutively for a long time
+ disableLongFailedAlerts();
+
} catch (Exception e) {
LOG.error("Exception in monitor update task", e);
}
}
+ /**
+ * Disable the alert if it was updated before {@MAX_TASK_FAIL_DAYS} but
there is no success run since then.
+ */
+ private void disableLongFailedAlerts() {
Review comment:
Can we include some unit tests for this?
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
With regards,
Apache Git Services
-
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 #5208: [TE] Disable alerts if it has no success run within 30 days
akshayrai commented on a change in pull request #5208: [TE] Disable alerts if
it has no success run within 30 days
URL: https://github.com/apache/incubator-pinot/pull/5208#discussion_r405818464
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/monitor/MonitorTaskRunner.java
##
@@ -120,11 +134,62 @@ private void executeMonitorUpdate(MonitorTaskInfo
monitorTaskInfo) {
// update detection health
updateDetectionHealth();
+
+ // disable alerts that failed consecutively for a long time
+ disableLongFailedAlerts();
+
} catch (Exception e) {
LOG.error("Exception in monitor update task", e);
}
}
+ /**
+ * Disable the alert if it was updated before {@MAX_TASK_FAIL_DAYS} but
there is no success run since then.
Review comment:
What if the alert was updated(after receiving the below notification) and it
still continues to fail? We don't want to disable & notify them again?
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
With regards,
Apache Git Services
-
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 #5208: [TE] Disable alerts if it has no success run within 30 days
akshayrai commented on a change in pull request #5208: [TE] Disable alerts if
it has no success run within 30 days
URL: https://github.com/apache/incubator-pinot/pull/5208#discussion_r405815219
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/monitor/MonitorTaskRunner.java
##
@@ -120,11 +134,62 @@ private void executeMonitorUpdate(MonitorTaskInfo
monitorTaskInfo) {
// update detection health
updateDetectionHealth();
+
+ // disable alerts that failed consecutively for a long time
+ disableLongFailedAlerts();
+
} catch (Exception e) {
LOG.error("Exception in monitor update task", e);
}
}
+ /**
+ * Disable the alert if it was updated before {@MAX_TASK_FAIL_DAYS} but
there is no success run since then.
+ */
+ private void disableLongFailedAlerts() {
+DetectionConfigManager detectionDAO =
DAO_REGISTRY.getDetectionConfigManager();
+List detectionConfigs = detectionDAO.findAllActive();
+long currentTimeMills = System.currentTimeMillis();
+long maxTaskFailMills = TimeUnit.DAYS.toMillis(MAX_FAILED_DISABLE_DAYS);
+for (DetectionConfigDTO config : detectionConfigs) {
+ try {
+Timestamp updateTime = config.getUpdateTime();
+if (updateTime != null && config.getHealth() != null &&
config.getHealth().getDetectionTaskStatus() != null) {
+ long lastTaskExecutionTime =
config.getHealth().getDetectionTaskStatus().getLastTaskExecutionTime();
+ if (updateTime.getTime() <= currentTimeMills - maxTaskFailMills &&
(lastTaskExecutionTime == -1L
Review comment:
Can we skip the `lastTaskExecutionTime == -1L` check?
Isn't that already covered by `lastTaskExecutionTime <= currentTimeMills -
maxTaskFailMills`?
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
With regards,
Apache Git Services
-
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 #5208: [TE] Disable alerts if it has no success run within 30 days
akshayrai commented on a change in pull request #5208: [TE] Disable alerts if
it has no success run within 30 days
URL: https://github.com/apache/incubator-pinot/pull/5208#discussion_r405816169
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/monitor/MonitorTaskRunner.java
##
@@ -120,11 +134,62 @@ private void executeMonitorUpdate(MonitorTaskInfo
monitorTaskInfo) {
// update detection health
updateDetectionHealth();
+
+ // disable alerts that failed consecutively for a long time
+ disableLongFailedAlerts();
+
} catch (Exception e) {
LOG.error("Exception in monitor update task", e);
}
}
+ /**
+ * Disable the alert if it was updated before {@MAX_TASK_FAIL_DAYS} but
there is no success run since then.
+ */
+ private void disableLongFailedAlerts() {
+DetectionConfigManager detectionDAO =
DAO_REGISTRY.getDetectionConfigManager();
+List detectionConfigs = detectionDAO.findAllActive();
+long currentTimeMills = System.currentTimeMillis();
+long maxTaskFailMills = TimeUnit.DAYS.toMillis(MAX_FAILED_DISABLE_DAYS);
+for (DetectionConfigDTO config : detectionConfigs) {
+ try {
+Timestamp updateTime = config.getUpdateTime();
+if (updateTime != null && config.getHealth() != null &&
config.getHealth().getDetectionTaskStatus() != null) {
+ long lastTaskExecutionTime =
config.getHealth().getDetectionTaskStatus().getLastTaskExecutionTime();
+ if (updateTime.getTime() <= currentTimeMills - maxTaskFailMills &&
(lastTaskExecutionTime == -1L
+ || lastTaskExecutionTime <= currentTimeMills -
maxTaskFailMills)) {
+config.setActive(false);
+detectionDAO.update(config);
+sendDisableAlertNotificationEmail(config);
+LOG.info("Disabled alert " + config.getId() + " since it failed
more than " + MAX_FAILED_DISABLE_DAYS + " days");
+LOG.info("Task last update time: " + config.getUpdateTime());
+LOG.info("Last success task execution time: " +
lastTaskExecutionTime);
Review comment:
Clubbing them into one log statement might be easier to debug.
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
With regards,
Apache Git Services
-
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 #5208: [TE] Disable alerts if it has no success run within 30 days
akshayrai commented on a change in pull request #5208: [TE] Disable alerts if
it has no success run within 30 days
URL: https://github.com/apache/incubator-pinot/pull/5208#discussion_r405803905
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/monitor/MonitorTaskRunner.java
##
@@ -120,11 +134,62 @@ private void executeMonitorUpdate(MonitorTaskInfo
monitorTaskInfo) {
// update detection health
updateDetectionHealth();
+
+ // disable alerts that failed consecutively for a long time
+ disableLongFailedAlerts();
+
} catch (Exception e) {
LOG.error("Exception in monitor update task", e);
}
}
+ /**
+ * Disable the alert if it was updated before {@MAX_TASK_FAIL_DAYS} but
there is no success run since then.
+ */
+ private void disableLongFailedAlerts() {
+DetectionConfigManager detectionDAO =
DAO_REGISTRY.getDetectionConfigManager();
+List detectionConfigs = detectionDAO.findAllActive();
+long currentTimeMills = System.currentTimeMillis();
+long maxTaskFailMills = TimeUnit.DAYS.toMillis(MAX_FAILED_DISABLE_DAYS);
+for (DetectionConfigDTO config : detectionConfigs) {
+ try {
+Timestamp updateTime = config.getUpdateTime();
+if (updateTime != null && config.getHealth() != null &&
config.getHealth().getDetectionTaskStatus() != null) {
+ long lastTaskExecutionTime =
config.getHealth().getDetectionTaskStatus().getLastTaskExecutionTime();
+ if (updateTime.getTime() <= currentTimeMills - maxTaskFailMills &&
(lastTaskExecutionTime == -1L
+ || lastTaskExecutionTime <= currentTimeMills -
maxTaskFailMills)) {
+config.setActive(false);
+detectionDAO.update(config);
+sendDisableAlertNotificationEmail(config);
+LOG.info("Disabled alert " + config.getId() + " since it failed
more than " + MAX_FAILED_DISABLE_DAYS + " days");
+LOG.info("Task last update time: " + config.getUpdateTime());
+LOG.info("Last success task execution time: " +
lastTaskExecutionTime);
+ }
+}
+ } catch (Exception e) {
+LOG.error("Exception in disabling alert ", config.getId());
Review comment:
log the exception.
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
With regards,
Apache Git Services
-
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 #5208: [TE] Disable alerts if it has no success run within 30 days
akshayrai commented on a change in pull request #5208: [TE] Disable alerts if
it has no success run within 30 days
URL: https://github.com/apache/incubator-pinot/pull/5208#discussion_r405814064
##
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/monitor/MonitorTaskRunner.java
##
@@ -120,11 +134,62 @@ private void executeMonitorUpdate(MonitorTaskInfo
monitorTaskInfo) {
// update detection health
updateDetectionHealth();
+
+ // disable alerts that failed consecutively for a long time
+ disableLongFailedAlerts();
+
} catch (Exception e) {
LOG.error("Exception in monitor update task", e);
}
}
+ /**
+ * Disable the alert if it was updated before {@MAX_TASK_FAIL_DAYS} but
there is no success run since then.
+ */
+ private void disableLongFailedAlerts() {
+DetectionConfigManager detectionDAO =
DAO_REGISTRY.getDetectionConfigManager();
+List detectionConfigs = detectionDAO.findAllActive();
+long currentTimeMills = System.currentTimeMillis();
Review comment:
nit: typo `currentTimeMillis` & `maxTaskFailMillis`
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jamesyfshao opened a new pull request #5229: enable async processing in pinot broker query api
jamesyfshao opened a new pull request #5229: enable async processing in pinot broker query api URL: https://github.com/apache/incubator-pinot/pull/5229 right now pinot broker query api issue a blocking call when it try to fetch result from pinot server. If we are seeing high amount of traffic to pinot broker with long request process latency, the whole pinot broker instances could be blocked due to limited amount of processing threads available to grizzly java web container. This change help to address the issue related to blocking call and ensure pinot broker have capacity to handle request even when some of the broker requests are taking too long 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 With regards, Apache Git Services - 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 #5228: Remove the cache from Travis
Jackie-Jiang opened a new pull request #5228: Remove the cache from Travis URL: https://github.com/apache/incubator-pinot/pull/5228 If cache does not provide much gain for the test performance, disable it to save the extra cost of managing 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 With regards, Apache Git Services - 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 #5224: Support bootstrap mode for table rebalance
Jackie-Jiang commented on a change in pull request #5224: Support bootstrap
mode for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405806197
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignment.java
##
@@ -189,45 +163,48 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
InstancePartitions instancePartitions =
instancePartitionsMap.get(InstancePartitionsType.OFFLINE);
Preconditions.checkState(instancePartitions != null, "Failed to find
OFFLINE instance partitions for table: %s",
_offlineTableName);
-LOGGER.info("Rebalancing table: {} with instance partitions: {}",
_offlineTableName, instancePartitions);
+boolean bootstrap =
+config.getBoolean(RebalanceConfigConstants.BOOTSTRAP,
RebalanceConfigConstants.DEFAULT_BOOTSTRAP);
+LOGGER.info("Rebalancing table: {} with instance partitions: {},
bootstrap: {}", _offlineTableName,
+instancePartitions, bootstrap);
+checkReplication(instancePartitions);
Map> newAssignment;
-if (instancePartitions.getNumReplicaGroups() == 1) {
- // Non-replica-group based assignment
+if (bootstrap) {
+ LOGGER.info("Bootstrapping the table: {}", _offlineTableName);
- List instances =
-
SegmentAssignmentUtils.getInstancesForNonReplicaGroupBasedAssignment(instancePartitions,
_replication);
- newAssignment = SegmentAssignmentUtils
- .rebalanceTableWithHelixAutoRebalanceStrategy(currentAssignment,
instances, _replication);
+ // When bootstrap is enabled, start with an empty table add reassign all
segments
Review comment:
What's the suggestion? I feel this comment is clear?
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
With regards,
Apache Git Services
-
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 #5224: Support bootstrap mode for table rebalance
Jackie-Jiang commented on a change in pull request #5224: Support bootstrap
mode for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405805821
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignment.java
##
@@ -99,49 +99,48 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
_offlineTableName);
LOGGER.info("Assigning segment: {} with instance partitions: {} for table:
{}", segmentName, instancePartitions,
_offlineTableName);
+checkReplication(instancePartitions);
-List instancesAssigned;
-if (instancePartitions.getNumReplicaGroups() == 1) {
+List instancesAssigned = assignSegment(segmentName,
currentAssignment, instancePartitions);
+
+LOGGER
Review comment:
That is exactly the reason why the `assignSegment` is extracted into a
helper method (line 128)
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated (df3b904 -> 7ca6f33)
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 df3b904 Fix travis cache (#5225)
add 7ca6f33 [TE] frontend - harleyjj/rca - reformat dimension-algorithm
table (#5206)
No new revisions were added by this update.
Summary of changes:
.../components/contribution-table/component.js | 211 -
.../components/contribution-table/template.hbs | 249 -
.../rootcause-dimensions-algorithm/component.js| 24 +-
.../dimensions-table/change-bars/template.hbs | 2 +-
.../pods/custom/dimensions-table/cost/template.hbs | 1 -
.../custom/dimensions-table/dimension/template.hbs | 4 +-
.../{cost => percent-change}/component.js | 0
.../dimensions-table/percent-change/template.hbs | 1 +
.../app/shared/dimensionAnalysisTableConfig.js | 23 +-
thirdeye/thirdeye-frontend/app/styles/app.scss | 1 -
.../app/styles/components/contribution-table.scss | 132 ---
.../app/styles/pods/custom/dimensions-table.scss | 29 ++-
.../contribution-table/component-test.js | 26 ---
13 files changed, 58 insertions(+), 645 deletions(-)
delete mode 100644
thirdeye/thirdeye-frontend/app/pods/components/contribution-table/component.js
delete mode 100644
thirdeye/thirdeye-frontend/app/pods/components/contribution-table/template.hbs
delete mode 100644
thirdeye/thirdeye-frontend/app/pods/custom/dimensions-table/cost/template.hbs
rename thirdeye/thirdeye-frontend/app/pods/custom/dimensions-table/{cost =>
percent-change}/component.js (100%)
create mode 100644
thirdeye/thirdeye-frontend/app/pods/custom/dimensions-table/percent-change/template.hbs
delete mode 100644
thirdeye/thirdeye-frontend/app/styles/components/contribution-table.scss
delete mode 100644
thirdeye/thirdeye-frontend/tests/integration/pods/components/contribution-table/component-test.js
-
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 #5198: [TE] frontend - harleyjj/home - use duration param to set date range
akshayrai merged pull request #5198: [TE] frontend - harleyjj/home - use duration param to set date range URL: https://github.com/apache/incubator-pinot/pull/5198 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 With regards, Apache Git Services - 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 #5206: [TE] frontend - harleyjj/rca - reformat dimension-algorithm table
akshayrai merged pull request #5206: [TE] frontend - harleyjj/rca - reformat dimension-algorithm table URL: https://github.com/apache/incubator-pinot/pull/5206 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #5224: Support bootstrap mode for table rebalance
jackjlli commented on a change in pull request #5224: Support bootstrap mode
for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405795607
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignment.java
##
@@ -99,49 +99,48 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
_offlineTableName);
LOGGER.info("Assigning segment: {} with instance partitions: {} for table:
{}", segmentName, instancePartitions,
_offlineTableName);
+checkReplication(instancePartitions);
-List instancesAssigned;
-if (instancePartitions.getNumReplicaGroups() == 1) {
+List instancesAssigned = assignSegment(segmentName,
currentAssignment, instancePartitions);
+
+LOGGER
Review comment:
Will it print too many messages if there are many segments in a table?
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #5224: Support bootstrap mode for table rebalance
jackjlli commented on a change in pull request #5224: Support bootstrap mode
for table rebalance
URL: https://github.com/apache/incubator-pinot/pull/5224#discussion_r405789065
##
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/assignment/segment/OfflineSegmentAssignment.java
##
@@ -189,45 +163,48 @@ public void init(HelixManager helixManager, TableConfig
tableConfig) {
InstancePartitions instancePartitions =
instancePartitionsMap.get(InstancePartitionsType.OFFLINE);
Preconditions.checkState(instancePartitions != null, "Failed to find
OFFLINE instance partitions for table: %s",
_offlineTableName);
-LOGGER.info("Rebalancing table: {} with instance partitions: {}",
_offlineTableName, instancePartitions);
+boolean bootstrap =
+config.getBoolean(RebalanceConfigConstants.BOOTSTRAP,
RebalanceConfigConstants.DEFAULT_BOOTSTRAP);
+LOGGER.info("Rebalancing table: {} with instance partitions: {},
bootstrap: {}", _offlineTableName,
+instancePartitions, bootstrap);
+checkReplication(instancePartitions);
Map> newAssignment;
-if (instancePartitions.getNumReplicaGroups() == 1) {
- // Non-replica-group based assignment
+if (bootstrap) {
+ LOGGER.info("Bootstrapping the table: {}", _offlineTableName);
- List instances =
-
SegmentAssignmentUtils.getInstancesForNonReplicaGroupBasedAssignment(instancePartitions,
_replication);
- newAssignment = SegmentAssignmentUtils
- .rebalanceTableWithHelixAutoRebalanceStrategy(currentAssignment,
instances, _replication);
+ // When bootstrap is enabled, start with an empty table add reassign all
segments
Review comment:
Can you adjust the comment 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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] akshayrai opened a new pull request #5227: Revert "[TE][subscription] update subscription watermarks to use anom…
akshayrai opened a new pull request #5227: Revert "[TE][subscription] update subscription watermarks to use anom… URL: https://github.com/apache/incubator-pinot/pull/5227 …aly create time instead of end time (#5152)" This reverts commit 2cfbe7e17ebc4a9ed6b119209b905adf91d8ae0f to unblock deployment. This change will be added back when we have the script to update the watermarks from anomaly end time to create time. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch remove_MaxDirectMemorySize_in_scripts updated (1a86786 -> be21822)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch remove_MaxDirectMemorySize_in_scripts in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 1a86786 Merge branch 'master' into remove_MaxDirectMemorySize_in_scripts add be21822 Update values.yaml No new revisions were added by this update. Summary of changes: kubernetes/helm/values.yaml | 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 remove_MaxDirectMemorySize_in_scripts updated (428b219 -> 1a86786)
This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a change to branch remove_MaxDirectMemorySize_in_scripts in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git. from 428b219 Remove MaxDirectMemorySize settings add a6c903d Bump jackson.version from 2.9.9 to 2.10.2 in /thirdeye (#5094) add 83215f0 Fix a NullPointerException that can occur if an Exception is raised when reading a JSON Record file. (#5128) add 43db343 pinot-quickstart companion container for thirdeye (#5125) add a91ffd6 Fix an NPE that is encountered when the offline table config does not specify a time column but the realtime table config does. (#5131) add 76f9c53 Support schema evolution for consuming segments (#4954) add a5fb1bd docker - generator script for synthetic time series in pinot (#5134) add 3e451e7 Refactor the data source to include all information needed for query execution (#5115) add 330f37f Fix the NPE when no segment exists in RealtimeSegmentSelector (#5138) add 5c7ece2 Add test for RealtimeSegmentSelector for no segment scenario (#5139) add d8fabec [TE] database - add missing schema fields and index to application_index (#5124) add 6d8c60a [TE] Minor corrections to the default yaml template (#5140) add 46698d8 Update links in README to latest docs (#5141) add 6daeb59 Fix the default value provider classes (#5137) add f7f9f4a Fix hybrid quickstart (#5143) add 6b6fa56a Update license and notice (#5145) add 888700d [TE] Remove validation to make recipients backwards compatible (#5149) add 82cfbdd Fix a formatting bug in AggregationFunctionUtils (#5148) add d15a91a update pinot assembly scripts (#5146) add d989427 Fix the SQL group-by for empty data table (#5151) add 494874d [TE] Detection creation endpoints should trigger Replay & Tuning (#5142) add be37b56 Refactor value based segment pruner to work on DataSource instead of ColumnMetadata (#5144) add 9933870 Move SegmentMetadata from pinot-common to pinot-core (#5156) add 4be032a [TE] frontend - harleyjj/alert-details - stop showing bounds for minutely granularity on Alert Overview (#5157) add 1376331 [TE] Setup a test Github workflow for ThirdEye (#5158) add c81a656 Add Azure Data Lake Gen2 connector for PinotFS (#5116) add bbaa1d9 [TE] anomaly detection model downloader (#5155) add 8eb5933 Updated Readme with Pinot clients and replace gitter badge with slack (#5164) add e442bb9 Updating readme (#5133) add 917493f Optimization for selection only queries: Allow early termination (#5163) add 32a54ae Fixing the missing fields in k8s helm template and reduce hardware & jvm requirements to start up pinot stack (#5161) add 2e800e0 Fix the NPE for DISTINCT aggregation function when no record is selected (#5154) add d289db8 Add QueryConfig to TableConfig with the query timeout setting (#5162) add 56f3e97 Fix the bug of broker not able to handle json format rest api response (#5170) add b8a4a1e [TE] frontend - harleyjj/rca - add Alert Correlation events to RCA event table (#5136) add 977c79c [TE] frontend - harleyjj/rca - cube table updates (#5159) add 2cfbe7e [TE][subscription] update subscription watermarks to use anomaly create time instead of end time (#5152) add 562180d [TE] Remove hard-coded server data datetimeformatter (#5171) add 20b1da9 Add Confluent Schema Registry Avro Message Decoder (#4973) add ef32b27 Fix selection early termination test (#5173) add 385c0d3 [TE] Fix issue of displaying latests data in the speed-up case (#5165) add ae484e5 Table level timeout implementation (#5169) add b8ed426 Add EarlyTerminationException to prevent logging too many invalid error messages (#5174) add 28f2643 Fix the numEntriesScannedPostFilter field test in SelectionOnlyEarlyTerminationTest (#5179) add 4a6b21d Fix docker compose & quick start (#5178) add b274d02 Prepare for pinot release 0.4.0 (#5186) add ed235d2 Add a simple PinotFS benchmark driver (#5160) add 8ff155a Change the name for azure data lake gen2 implementation (#5188) add 48fb505 Change readme link to gitbook for kafka plugins readme. (#5191) add f1e2086 [TE] frontend - harleyjj/validation - surface errors in dom for create and edit alert (#5187) add 8f0ed55 Update travis scripts to test quickstart over jdk 10-15 (#5182) add 772f51e [TE] frontend - harleyjj/alert-details - show bounds for minute granularity again (#5192) add 1f1baf8 Adding missing license files for jquery-requestAnimationFrame and jquery-sizzle, requested in Issue #5183 (#5195) add 00fcb1d Move table config into pinot-spi (#5194) add 8dfa51a Lucene DocId to PinotDocId cache to improve performance (#5177) add 9cb716f Nightly publish to bintray
[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation
siddharthteotia commented on a change in pull request #5226: Add inter segment
tests for text search and fix bug for Lucene query parser creation
URL: https://github.com/apache/incubator-pinot/pull/5226#discussion_r405717756
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
##
@@ -133,7 +131,8 @@ public MutableRoaringBitmap getDocIds(Object value) {
MutableRoaringBitmap docIds = new MutableRoaringBitmap();
Collector docIDCollector = new LuceneDocIdCollector(docIds,
_docIdTranslator);
try {
- Query query = _queryParser.parse(searchQuery);
+ QueryParser parser = new QueryParser(_column, new StandardAnalyzer());
Review comment:
So the parser is JavaCC based and so it is stateful. We have to instantiate
it per query (this is the bug). Analyzer on the other hand seems stateless so I
reverted that part to be instantiated just once.
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
With regards,
Apache Git Services
-
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 #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation
Jackie-Jiang commented on a change in pull request #5226: Add inter segment
tests for text search and fix bug for Lucene query parser creation
URL: https://github.com/apache/incubator-pinot/pull/5226#discussion_r405702794
##
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/readers/text/LuceneTextIndexReader.java
##
@@ -133,7 +131,8 @@ public MutableRoaringBitmap getDocIds(Object value) {
MutableRoaringBitmap docIds = new MutableRoaringBitmap();
Collector docIDCollector = new LuceneDocIdCollector(docIds,
_docIdTranslator);
try {
- Query query = _queryParser.parse(searchQuery);
+ QueryParser parser = new QueryParser(_column, new StandardAnalyzer());
Review comment:
Is there any way to reuse some components here? Will this cause too much
garbage?
Please also add some comments so that future developers know that it cannot
be reused.
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
With regards,
Apache Git Services
-
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 issue #5225: Fix travis cache
Jackie-Jiang commented on issue #5225: Fix travis cache URL: https://github.com/apache/incubator-pinot/pull/5225#issuecomment-611082555 > This is great! > Quick question, do we also need to do so for quickstart jobs? @fx19880617 The main purpose for this fix is to remove extra files before uploading cache (with the fix the tests finally passed). Seems the quick-start tests also work fine, so I'll leave it like this now. 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #5225: Fix travis cache
Jackie-Jiang merged pull request #5225: Fix travis cache URL: https://github.com/apache/incubator-pinot/pull/5225 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[incubator-pinot] branch master updated: Fix travis cache (#5225)
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 df3b904 Fix travis cache (#5225)
df3b904 is described below
commit df3b90403a60ebf336496161eb29de962e347d65
Author: Xiaotian (Jackie) Jiang <1751+jackie-ji...@users.noreply.github.com>
AuthorDate: Wed Apr 8 10:15:33 2020 -0700
Fix travis cache (#5225)
---
.travis.yml | 3 +++
.travis/.travis_quickstart.sh | 4
.travis/.travis_test.sh | 9 -
3 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 8523ef3..84dfceb 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -11,6 +11,9 @@ before_install:
- curl -sSL -o ~/bin/install-jdk.sh
https://raw.githubusercontent.com/sormuras/bach/master/install-jdk.sh && chmod
+x ~/bin/install-jdk.sh
- source ./.travis/.travis_set_deploy_build_opts.sh
+before_cache:
+ - rm -rf $HOME/.m2/repository/org/apache/pinot
+
cache:
directories:
- $HOME/.m2
diff --git a/.travis/.travis_quickstart.sh b/.travis/.travis_quickstart.sh
index 09c0edc..1105927 100755
--- a/.travis/.travis_quickstart.sh
+++ b/.travis/.travis_quickstart.sh
@@ -22,13 +22,9 @@
git diff --name-only "${TRAVIS_COMMIT_RANGE}" | egrep '^(thirdeye)'
if [ $? -eq 0 ]; then
echo 'Skip ThirdEye tests for Quickstart'
- rm -rf ~/.m2/repository/com/linkedin/pinot
~/.m2/repository/com/linkedin/thirdeye
exit 0
fi
-# Remove Pinot files from local Maven repository to avoid a useless cache
rebuild
-rm -rf ~/.m2/repository/org/apache/pinot/
-
# Java version
java -version
diff --git a/.travis/.travis_test.sh b/.travis/.travis_test.sh
index a61f93c..451f964 100755
--- a/.travis/.travis_test.sh
+++ b/.travis/.travis_test.sh
@@ -25,21 +25,17 @@ if [ $? -eq 0 ]; then
if [ "$TRAVIS_JDK_VERSION" != 'oraclejdk8' ]; then
echo 'Skip ThirdEye tests for version other than oracle jdk8.'
-rm -rf ~/.m2/repository/com/linkedin/pinot
~/.m2/repository/com/linkedin/thirdeye
exit 0
fi
if [ "RUN_INTEGRATION_TESTS" == 'false' ]; then
echo 'Skip ThirdEye tests when integration tests off'
-rm -rf ~/.m2/repository/com/linkedin/pinot
~/.m2/repository/com/linkedin/thirdeye
exit 0
fi
cd thirdeye
mvn test -B ${DEPLOY_BUILD_OPTS}
failed=$?
- # Remove Pinot/ThirdEye files from local Maven repository to avoid a useless
cache rebuild
- rm -rf ~/.m2/repository/com/linkedin/pinot
~/.m2/repository/com/linkedin/thirdeye
if [ $failed -eq 0 ]; then
exit 0
else
@@ -50,8 +46,6 @@ fi
# Only run tests for JDK 8
if [ "$TRAVIS_JDK_VERSION" != 'oraclejdk8' ]; then
echo 'Skip tests for version other than oracle jdk8.'
- # Remove Pinot files from local Maven repository to avoid a useless cache
rebuild
- rm -rf ~/.m2/repository/com/linkedin/pinot
exit 0
fi
@@ -75,9 +69,6 @@ else
fi
fi
-# Remove Pinot files from local Maven repository to avoid a useless cache
rebuild
-rm -rf ~/.m2/repository/com/linkedin/pinot
-
if [ $passed -eq 1 ]; then
# Only send code coverage data if passed
bash <(cat .codecov_bash)
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] fx19880617 closed pull request #5023: Make pinot-client to query sql endpoint by default
fx19880617 closed pull request #5023: Make pinot-client to query sql endpoint by default URL: https://github.com/apache/incubator-pinot/pull/5023 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] siddharthteotia opened a new pull request #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation
siddharthteotia opened a new pull request #5226: Add inter segment tests for text search and fix bug for Lucene query parser creation URL: https://github.com/apache/incubator-pinot/pull/5226 Add inter segment tests for text search and fix bug for Lucene query parser creation. Parser should be instantiated per query (search expression). Also added text search tests for the SQL path 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 With regards, Apache Git Services - To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #5221: Add a new server api for download of segments.
jackjlli commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405280519
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java
##
@@ -149,4 +160,62 @@ public void testSegmentCrcMetadata()
Assert.assertEquals(segmentsCrc.get(segmentName).asText(), crc);
}
}
+
+ @Test
+ public void testDownloadSegments()
+ throws Exception {
+// Verify the content of the downloaded segment from a realtime table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(REALTIME_TABLE_NAME,
_realtimeIndexSegments.get(0)));
+// Verify the content of the downloaded segment from an offline table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(OFFLINE_TABLE_NAME,
_offlineIndexSegments.get(0)));
+
+// Verify non-existent table and segment download return NOT_FOUND status.
+Response response =
_webTarget.path("/tables/UNKNOWN_REALTIME/segments/segmentname").request()
+.get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+
+response = _webTarget.path("/tables/" + REALTIME_TABLE_NAME +
"/segments/UNKNOWN_SEGMENT").request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+ }
+
+ // Verify metadata file from segments.
+ private boolean downLoadAndVerifySegmentContent(String tableNameWithType,
IndexSegment segment) {
+String segmentPath = "/tables/" + tableNameWithType + "/segments/" +
segment.getSegmentName();
+
+// Download the segment and save to a temp local file.
+Response response =
_webTarget.path(segmentPath).request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.OK.getStatusCode());
+File segmentFile = response.readEntity(File.class);
+
+File tempMetadataDir = new File(FileUtils.getTempDirectory(),
"segment_metadata");
+try (// Extract metadata.properties
+InputStream metadataPropertiesInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.MetadataKeys.METADATA_FILE_NAME);
+// Extract creation.meta
+InputStream creationMetaInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.SEGMENT_CREATION_META)) {
+ Preconditions
+ .checkState(tempMetadataDir.mkdirs(), "Failed to create directory:
%s", tempMetadataDir.getAbsolutePath());
+
+ Preconditions.checkNotNull(metadataPropertiesInputStream, "%s does not
exist",
+ V1Constants.MetadataKeys.METADATA_FILE_NAME);
+ java.nio.file.Path metadataPropertiesPath = FileSystems.getDefault()
+ .getPath(tempMetadataDir.getAbsolutePath(),
V1Constants.MetadataKeys.METADATA_FILE_NAME);
+ Files.copy(metadataPropertiesInputStream, metadataPropertiesPath);
+
+ Preconditions.checkNotNull(creationMetaInputStream, "%s does not exist",
V1Constants.SEGMENT_CREATION_META);
+ java.nio.file.Path creationMetaPath =
+ FileSystems.getDefault().getPath(tempMetadataDir.getAbsolutePath(),
V1Constants.SEGMENT_CREATION_META);
+ Files.copy(creationMetaInputStream, creationMetaPath);
+ // Load segment metadata
+ SegmentMetadataImpl metadata = new SegmentMetadataImpl(tempMetadataDir);
+
+ Assert.assertEquals(tableNameWithType, metadata.getTableName());
+ return true;
+} catch (Exception e) {
+ return false;
Review comment:
May be good to log the exception 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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #5221: Add a new server api for download of segments.
jackjlli commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405280224
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java
##
@@ -149,4 +160,62 @@ public void testSegmentCrcMetadata()
Assert.assertEquals(segmentsCrc.get(segmentName).asText(), crc);
}
}
+
+ @Test
+ public void testDownloadSegments()
+ throws Exception {
+// Verify the content of the downloaded segment from a realtime table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(REALTIME_TABLE_NAME,
_realtimeIndexSegments.get(0)));
+// Verify the content of the downloaded segment from an offline table.
+Assert.assertTrue(downLoadAndVerifySegmentContent(OFFLINE_TABLE_NAME,
_offlineIndexSegments.get(0)));
+
+// Verify non-existent table and segment download return NOT_FOUND status.
+Response response =
_webTarget.path("/tables/UNKNOWN_REALTIME/segments/segmentname").request()
+.get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+
+response = _webTarget.path("/tables/" + REALTIME_TABLE_NAME +
"/segments/UNKNOWN_SEGMENT").request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.NOT_FOUND.getStatusCode());
+ }
+
+ // Verify metadata file from segments.
+ private boolean downLoadAndVerifySegmentContent(String tableNameWithType,
IndexSegment segment) {
+String segmentPath = "/tables/" + tableNameWithType + "/segments/" +
segment.getSegmentName();
+
+// Download the segment and save to a temp local file.
+Response response =
_webTarget.path(segmentPath).request().get(Response.class);
+Assert.assertEquals(response.getStatus(),
Response.Status.OK.getStatusCode());
+File segmentFile = response.readEntity(File.class);
+
+File tempMetadataDir = new File(FileUtils.getTempDirectory(),
"segment_metadata");
+try (// Extract metadata.properties
+InputStream metadataPropertiesInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.MetadataKeys.METADATA_FILE_NAME);
+// Extract creation.meta
+InputStream creationMetaInputStream = TarGzCompressionUtils
+.unTarOneFile(new FileInputStream(segmentFile),
V1Constants.SEGMENT_CREATION_META)) {
+ Preconditions
+ .checkState(tempMetadataDir.mkdirs(), "Failed to create directory:
%s", tempMetadataDir.getAbsolutePath());
+
+ Preconditions.checkNotNull(metadataPropertiesInputStream, "%s does not
exist",
+ V1Constants.MetadataKeys.METADATA_FILE_NAME);
+ java.nio.file.Path metadataPropertiesPath = FileSystems.getDefault()
Review comment:
import java.nio.file.Path;
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #5221: Add a new server api for download of segments.
jackjlli commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405279481
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/TableSizeResourceTest.java
##
@@ -37,9 +37,9 @@ public void testTableSizeNotFound() {
@Test
public void testTableSizeDetailed() {
TableSizeInfo tableSizeInfo =
_webTarget.path(TABLE_SIZE_PATH).request().get(TableSizeInfo.class);
-ImmutableSegment defaultSegment = _indexSegments.get(0);
+ImmutableSegment defaultSegment = _realtimeIndexSegments.get(0);
Review comment:
getTableSize() method is often used for offline table. Can you add the test
for offline table 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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
[GitHub] [incubator-pinot] jackjlli commented on a change in pull request #5221: Add a new server api for download of segments.
jackjlli commented on a change in pull request #5221: Add a new server api for
download of segments.
URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r405276990
##
File path:
pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java
##
@@ -55,11 +56,12 @@
public abstract class BaseResourceTest {
private static final String AVRO_DATA_PATH = "data/test_data-mv.avro";
private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(),
"BaseResourceTest");
- protected static final String TABLE_NAME = "testTable";
+ protected static final String REALTIME_TABLE_NAME = "testTable_REALTIME";
Review comment:
+1
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
With regards,
Apache Git Services
-
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
