You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/01/19 22:50:36 UTC
[GitHub] [incubator-pinot] npawar opened a new pull request #6466: Handle creation of segments with 0 rows
npawar opened a new pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466
## Description
https://github.com/apache/incubator-pinot/issues/6453
Do not fail segment creation if data source has 0 rows. Works for both offline and realtime tables.
This PR simply adds some checks for totalDocs throughout the segment build and load path. This PR will not at all affect the regular case, of records > 0.
## Release Notes
Creating a segment with 0 rows in the data file will not fail anymore.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-765818572
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=h1) Report
> Merging [#6466](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=desc) (b9c3f01) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `0.82%`.
> The diff coverage is `62.18%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6466 +/- ##
==========================================
- Coverage 66.44% 65.62% -0.83%
==========================================
Files 1075 1345 +270
Lines 54773 66138 +11365
Branches 8168 9647 +1479
==========================================
+ Hits 36396 43403 +7007
- Misses 15700 19637 +3937
- Partials 2677 3098 +421
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests | `65.62% <62.18%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: |
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: |
| [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `66.66% <ø> (+9.52%)` | :arrow_up: |
| [...n/java/org/apache/pinot/client/BrokerResponse.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Jyb2tlclJlc3BvbnNlLmphdmE=) | `100.00% <ø> (ø)` | |
| [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <ø> (-13.29%)` | :arrow_down: |
| [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `68.88% <ø> (ø)` | |
| [...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydC5qYXZh) | `10.90% <ø> (-51.10%)` | :arrow_down: |
| [...n/java/org/apache/pinot/client/ResultSetGroup.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFNldEdyb3VwLmphdmE=) | `65.38% <ø> (+0.16%)` | :arrow_up: |
| ... and [1214 more](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=footer). Last update [9708292...b9c3f01](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-777040075
> > Mostly good.
> > Another way to handle the server size empty segments is to not load them. In `ImmutableSegmentLoader`, return `null` if the segment is empty. The caller can handle the `null` segment accordingly.
> > Currently the query engine won't work on the empty `ImmutableSegmentLoader` if the pruner let it pass because the data source is not set.
> > wdyt?
>
> I found that there were too many callers of ImmutableSegmentLoader#load, and felt it would be error prone if I tried to find them all and handle the null case. If you think that's the better approach, I'm open to doing that.
> The case you mention should be handled by the broker side pruner though rt?
I checked the code and the `ImmutableSegmentLoader#load` is only called in 3 different classes (regardless of the tests). There are way more callers to `IndexSegment#getDataSource`, so I feel it might be harder to keep it correct if we allow empty `DataSource`. Another small benefit of not loading the empty segment is that we can remove the `ValidSegmentPruner` and reduce the query overhead a little bit.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-765818572
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=h1) Report
> Merging [#6466](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=desc) (5dc1365) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `1.33%`.
> The diff coverage is `62.09%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6466 +/- ##
==========================================
- Coverage 66.44% 65.11% -1.34%
==========================================
Files 1075 1335 +260
Lines 54773 65678 +10905
Branches 8168 9584 +1416
==========================================
+ Hits 36396 42767 +6371
- Misses 15700 19853 +4153
- Partials 2677 3058 +381
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests | `65.11% <62.09%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: |
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: |
| [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `66.66% <ø> (+9.52%)` | :arrow_up: |
| [...n/java/org/apache/pinot/client/BrokerResponse.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Jyb2tlclJlc3BvbnNlLmphdmE=) | `100.00% <ø> (ø)` | |
| [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <ø> (-13.29%)` | :arrow_down: |
| [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `15.55% <ø> (ø)` | |
| [...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydC5qYXZh) | `10.90% <ø> (-51.10%)` | :arrow_down: |
| [...n/java/org/apache/pinot/client/ResultSetGroup.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L1Jlc3VsdFNldEdyb3VwLmphdmE=) | `65.38% <ø> (+0.16%)` | :arrow_up: |
| ... and [1182 more](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=footer). Last update [dde3c18...5dc1365](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r577835166
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/pruner/SegmentPrunerService.java
##########
@@ -38,11 +39,21 @@
public SegmentPrunerService(SegmentPrunerConfig config) {
int numPruners = config.numSegmentPruners();
- _segmentPruners = new ArrayList<>(numPruners);
+ _segmentPruners = new ArrayList<>(numPruners + 1);
Review comment:
yeah, changed it to be that way
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r574933740
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/ImmutableSegmentLoader.java
##########
@@ -105,32 +105,34 @@ public static ImmutableSegment load(File indexDir, IndexLoadingConfig indexLoadi
// Load the segment
ReadMode readMode = indexLoadingConfig.getReadMode();
SegmentDirectory segmentDirectory = SegmentDirectory.createFromLocalFS(indexDir, segmentMetadata, readMode);
- SegmentDirectory.Reader segmentReader = segmentDirectory.createReader();
Map<String, ColumnIndexContainer> indexContainerMap = new HashMap<>();
- for (Map.Entry<String, ColumnMetadata> entry : segmentMetadata.getColumnMetadataMap().entrySet()) {
- indexContainerMap.put(entry.getKey(),
- new PhysicalColumnIndexContainer(segmentReader, entry.getValue(), indexLoadingConfig, indexDir));
- }
+ StarTreeIndexContainer starTreeIndexContainer = null;
- // Instantiate virtual columns
- Schema segmentSchema = segmentMetadata.getSchema();
- VirtualColumnProviderFactory.addBuiltInVirtualColumnsToSegmentSchema(segmentSchema, segmentName);
- for (FieldSpec fieldSpec : segmentSchema.getAllFieldSpecs()) {
- if (fieldSpec.isVirtualColumn()) {
- String columnName = fieldSpec.getName();
- VirtualColumnContext context = new VirtualColumnContext(fieldSpec, segmentMetadata.getTotalDocs());
- VirtualColumnProvider provider = VirtualColumnProviderFactory.buildProvider(context);
- indexContainerMap.put(columnName, provider.buildColumnIndexContainer(context));
- segmentMetadata.getColumnMetadataMap().put(columnName, provider.buildMetadata(context));
+ if (segmentMetadata.getTotalDocs() > 0) {
Review comment:
Special case empty segment in line 105 and directly return, no need to change the existing code
```suggestion
if (segmentMetadata.getTotalDocs() == 0) {
return new EmptyIndexSegment(segmentMetadata);
}
```
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/ImmutableSegmentImpl.java
##########
@@ -115,7 +116,11 @@ public DataSource getDataSource(String column) {
ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
Preconditions.checkNotNull(columnMetadata,
"ColumnMetadata for " + column + " should not be null. " + "Potentially invalid column name specified.");
- return new ImmutableDataSource(columnMetadata, _indexContainerMap.get(column));
+ if (_indexContainerMap.containsKey(column)) {
Review comment:
This is not needed
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/EmptyIndexSegment.java
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.indexsegment.immutable;
+
+import com.google.common.base.Preconditions;
+import java.util.List;
+import java.util.Set;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.DataSource;
+import org.apache.pinot.core.segment.index.datasource.EmptyDataSource;
+import org.apache.pinot.core.segment.index.metadata.ColumnMetadata;
+import org.apache.pinot.core.segment.index.metadata.SegmentMetadataImpl;
+import org.apache.pinot.core.segment.index.readers.Dictionary;
+import org.apache.pinot.core.segment.index.readers.ForwardIndexReader;
+import org.apache.pinot.core.segment.index.readers.InvertedIndexReader;
+import org.apache.pinot.core.segment.index.readers.ValidDocIndexReader;
+import org.apache.pinot.core.segment.store.SegmentDirectory;
+import org.apache.pinot.core.startree.v2.StarTreeV2;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Immutable segment impl for empty segment
+ * Such an IndexSegment contains only the metadata, and no indexes
+ */
+public class EmptyIndexSegment implements ImmutableSegment {
+ private static final Logger LOGGER = LoggerFactory.getLogger(EmptyIndexSegment.class);
+
+ private final SegmentDirectory _segmentDirectory;
+ private final SegmentMetadataImpl _segmentMetadata;
+
+ public EmptyIndexSegment(SegmentDirectory segmentDirectory, SegmentMetadataImpl segmentMetadata) {
Review comment:
No need to pass in the segmentDirectory.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-765818572
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=h1) Report
> Merging [#6466](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=desc) (7c20a37) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `22.56%`.
> The diff coverage is `43.93%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6466 +/- ##
===========================================
- Coverage 66.44% 43.88% -22.57%
===========================================
Files 1075 1345 +270
Lines 54773 66104 +11331
Branches 8168 9637 +1469
===========================================
- Hits 36396 29011 -7385
- Misses 15700 34628 +18928
+ Partials 2677 2465 -212
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `43.88% <43.93%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
| [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ceselector/StrictReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1N0cmljdFJlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/TimeSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1RpbWVTZWdtZW50UHJ1bmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/interval/Interval.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...r/routing/segmentpruner/interval/IntervalTree.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsVHJlZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [1358 more](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=footer). Last update [9708292...7c20a37](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-777190471
> Discussed offline and seems allowing empty index segment is easier to manage. We may also consider adding an EmptyIndexSegment and EmptyDataSource for this special case. Please fix the tests before merging.
Added the EmptyIndexSegment and EmptyDataSource. It does look cleaner, but I also feel it added a lot of code which only returns nulls.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-776927857
> Mostly good.
>
> Another way to handle the server size empty segments is to not load them. In `ImmutableSegmentLoader`, return `null` if the segment is empty. The caller can handle the `null` segment accordingly.
> Currently the query engine won't work on the empty `ImmutableSegmentLoader` if the pruner let it pass because the data source is not set.
> wdyt?
I found that there were too many callers of ImmutableSegmentLoader#load, and felt it would be error prone if I tried to find them all and handle the null case. If you think that's the better approach, I'm open to doing that.
The case you mention should be handled by the broker side pruner though rt?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r573976548
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/ImmutableSegmentLoader.java
##########
@@ -105,32 +105,34 @@ public static ImmutableSegment load(File indexDir, IndexLoadingConfig indexLoadi
// Load the segment
ReadMode readMode = indexLoadingConfig.getReadMode();
SegmentDirectory segmentDirectory = SegmentDirectory.createFromLocalFS(indexDir, segmentMetadata, readMode);
- SegmentDirectory.Reader segmentReader = segmentDirectory.createReader();
Map<String, ColumnIndexContainer> indexContainerMap = new HashMap<>();
- for (Map.Entry<String, ColumnMetadata> entry : segmentMetadata.getColumnMetadataMap().entrySet()) {
- indexContainerMap.put(entry.getKey(),
- new PhysicalColumnIndexContainer(segmentReader, entry.getValue(), indexLoadingConfig, indexDir));
- }
+ StarTreeIndexContainer starTreeIndexContainer = null;
- // Instantiate virtual columns
- Schema segmentSchema = segmentMetadata.getSchema();
- VirtualColumnProviderFactory.addBuiltInVirtualColumnsToSegmentSchema(segmentSchema, segmentName);
- for (FieldSpec fieldSpec : segmentSchema.getAllFieldSpecs()) {
- if (fieldSpec.isVirtualColumn()) {
- String columnName = fieldSpec.getName();
- VirtualColumnContext context = new VirtualColumnContext(fieldSpec, segmentMetadata.getTotalDocs());
- VirtualColumnProvider provider = VirtualColumnProviderFactory.buildProvider(context);
- indexContainerMap.put(columnName, provider.buildColumnIndexContainer(context));
- segmentMetadata.getColumnMetadataMap().put(columnName, provider.buildMetadata(context));
+ if (segmentMetadata.getTotalDocs() > 0) {
Review comment:
If I don't add this special casing, I get this exceptions from forward index reader
```
java.lang.RuntimeException: java.lang.RuntimeException: Could not find index for column: studentID, type: FORWARD_INDEX, segment: /Users/npawar/quick_start_configs/pinot-quick-start/dataDir/data/transcript_REALTIME/transcript__0__1__20210210T1830Z/v3
at org.apache.pinot.core.data.manager.realtime.RealtimeTableDataManager.replaceLLSegment(RealtimeTableDataManager.java:481) ~[classes/:?]
at org.apache.pinot.core.data.manager.realtime.LLRealtimeSegmentDataManager.commitSegment(LLRealtimeSegmentDataManager.java:854) ~[classes/:?]
at org.apache.pinot.core.data.manager.realtime.LLRealtimeSegmentDataManager$PartitionConsumer.run(LLRealtimeSegmentDataManager.java:615) [classes/:?]
at java.lang.Thread.run(Thread.java:748) [?:1.8.0_242]
Caused by: java.lang.RuntimeException: Could not find index for column: studentID, type: FORWARD_INDEX, segment: /Users/npawar/quick_start_configs/pinot-quick-start/dataDir/data/transcript_REALTIME/transcript__0__1__20210210T1830Z/v3
at org.apache.pinot.core.segment.store.SingleFileIndexDirectory.checkAndGetIndexBuffer(SingleFileIndexDirectory.java:142) ~[classes/:?]
at org.apache.pinot.core.segment.store.SingleFileIndexDirectory.getBuffer(SingleFileIndexDirectory.java:101) ~[classes/:?]
at org.apache.pinot.core.segment.store.SegmentLocalFSDirectory.getIndexForColumn(SegmentLocalFSDirectory.java:229) ~[classes/:?]
at org.apache.pinot.core.segment.store.SegmentLocalFSDirectory.access$000(SegmentLocalFSDirectory.java:36) ~[classes/:?]
at org.apache.pinot.core.segment.store.SegmentLocalFSDirectory$Reader.getIndexFor(SegmentLocalFSDirectory.java:288) ~[classes/:?]
at org.apache.pinot.core.segment.index.column.PhysicalColumnIndexContainer.<init>(PhysicalColumnIndexContainer.java:134) ~[classes/:?]
at org.apache.pinot.core.indexsegment.immutable.ImmutableSegmentLoader.load(ImmutableSegmentLoader.java:114) ~[classes/:?]
at org.apache.pinot.core.data.manager.realtime.RealtimeTableDataManager.replaceLLSegment(RealtimeTableDataManager.java:479) ~[classes/:?]
... 3 more
```
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r574976505
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/ImmutableSegmentImpl.java
##########
@@ -115,7 +116,11 @@ public DataSource getDataSource(String column) {
ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
Preconditions.checkNotNull(columnMetadata,
"ColumnMetadata for " + column + " should not be null. " + "Potentially invalid column name specified.");
- return new ImmutableDataSource(columnMetadata, _indexContainerMap.get(column));
+ if (_indexContainerMap.containsKey(column)) {
Review comment:
When would we use EmptySegmentDataSource in that case?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r577120279
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/ImmutableSegmentImpl.java
##########
@@ -115,7 +116,11 @@ public DataSource getDataSource(String column) {
ColumnMetadata columnMetadata = _segmentMetadata.getColumnMetadataFor(column);
Preconditions.checkNotNull(columnMetadata,
"ColumnMetadata for " + column + " should not be null. " + "Potentially invalid column name specified.");
- return new ImmutableDataSource(columnMetadata, _indexContainerMap.get(column));
+ if (_indexContainerMap.containsKey(column)) {
Review comment:
Nvm, I understood what you meant
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-763855321
> It would help to also describe the following in the PR:
>
> * What is the use case that requires empty segments?
> * For a table with time based realtime segment creation, we might end up creating too many empty segments (if event freq << flush frequency)? If so, we should add checks in the code to not do so.
Both these are in the description and discussions of the issue linked in the description. Check it out #6453
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-767316922
> How would retention manager work on these segments?
Current time will be set as start/end. They will get removed accordingly. Update the description.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r577303043
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/ImmutableSegmentLoader.java
##########
@@ -132,7 +135,7 @@ public static ImmutableSegment load(File indexDir, IndexLoadingConfig indexLoadi
new StarTreeIndexContainer(SegmentDirectoryPaths.findSegmentDirectory(indexDir), segmentMetadata,
indexContainerMap, readMode);
}
-
+
Review comment:
(nit) revert
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/pruner/SegmentPrunerService.java
##########
@@ -38,11 +39,21 @@
public SegmentPrunerService(SegmentPrunerConfig config) {
int numPruners = config.numSegmentPruners();
- _segmentPruners = new ArrayList<>(numPruners);
+ _segmentPruners = new ArrayList<>(numPruners + 1);
Review comment:
We should always add `ValidSegmentPruner` as the first pruner?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r574162371
##########
File path: pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/EmptySegmentPruner.java
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.routing.segmentpruner;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nullable;
+import org.apache.helix.AccessOption;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.IdealState;
+import org.apache.helix.store.zk.ZkHelixPropertyStore;
+import org.apache.pinot.common.metadata.ZKMetadataProvider;
+import org.apache.pinot.common.request.BrokerRequest;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * The {@code EmptySegmentPruner} prunes segments if they have 0 totalDocs.
+ * Does not prune segments with -1 total docs (that can be either error case or CONSUMING segment)
+ */
+public class EmptySegmentPruner implements SegmentPruner {
+ private static final Logger LOGGER = LoggerFactory.getLogger(EmptySegmentPruner.class);
+
+ private final String _tableNameWithType;
+ private final ZkHelixPropertyStore<ZNRecord> _propertyStore;
+ private final String _segmentZKMetadataPathPrefix;
+
+ private final Map<String, Long> _segmentTotalDocsMap = new HashMap<>();
+ private final Set<String> _emptySegments = new HashSet<>();
+
+ public EmptySegmentPruner(TableConfig tableConfig, ZkHelixPropertyStore<ZNRecord> propertyStore) {
+ _tableNameWithType = tableConfig.getTableName();
+ _propertyStore = propertyStore;
+ _segmentZKMetadataPathPrefix = ZKMetadataProvider.constructPropertyStorePathForResource(_tableNameWithType) + "/";
+ }
+
+ @Override
+ public void init(ExternalView externalView, IdealState idealState, Set<String> onlineSegments) {
+ // Bulk load info for all online segments
+ int numSegments = onlineSegments.size();
+ List<String> segments = new ArrayList<>(numSegments);
+ List<String> segmentZKMetadataPaths = new ArrayList<>(numSegments);
+ for (String segment : onlineSegments) {
+ segments.add(segment);
+ segmentZKMetadataPaths.add(_segmentZKMetadataPathPrefix + segment);
+ }
+ List<ZNRecord> znRecords = _propertyStore.get(segmentZKMetadataPaths, null, AccessOption.PERSISTENT, false);
+ for (int i = 0; i < numSegments; i++) {
+ String segment = segments.get(i);
+ long totalDocs = extractTotalDocsFromSegmentZKMetaZNRecord(segment, znRecords.get(i));
+ _segmentTotalDocsMap.put(segment, totalDocs);
+ if (totalDocs == 0) {
+ _emptySegments.add(segment);
+ }
+ }
+ }
+
+ private long extractTotalDocsFromSegmentZKMetaZNRecord(String segment, @Nullable ZNRecord znRecord) {
+ if (znRecord == null) {
+ LOGGER.warn("Failed to find segment ZK metadata for segment: {}, table: {}", segment, _tableNameWithType);
+ return -1;
+ }
+ return znRecord.getLongField(CommonConstants.Segment.TOTAL_DOCS, -1);
+ }
+
+ @Override
+ public synchronized void onExternalViewChange(ExternalView externalView, IdealState idealState,
+ Set<String> onlineSegments) {
+ // NOTE: We don't update all the segment ZK metadata for every external view change, but only the new added/removed
+ // ones. The refreshed segment ZK metadata change won't be picked up.
+ for (String segment : onlineSegments) {
+ _segmentTotalDocsMap.computeIfAbsent(segment, k -> {
+ long totalDocs = extractTotalDocsFromSegmentZKMetaZNRecord(k,
+ _propertyStore.get(_segmentZKMetadataPathPrefix + k, null, AccessOption.PERSISTENT));
+ if (totalDocs == 0) {
+ _emptySegments.add(segment);
+ }
+ return totalDocs;
+ });
+ }
+ _segmentTotalDocsMap.keySet().retainAll(onlineSegments);
+ _emptySegments.retainAll(onlineSegments);
+ }
+
+ @Override
+ public synchronized void refreshSegment(String segment) {
+ long totalDocs = extractTotalDocsFromSegmentZKMetaZNRecord(segment,
+ _propertyStore.get(_segmentZKMetadataPathPrefix + segment, null, AccessOption.PERSISTENT));
+ _segmentTotalDocsMap.put(segment, totalDocs);
+ if (totalDocs == 0) {
+ _emptySegments.add(segment);
+ } else {
+ _emptySegments.remove(segment);
+ }
+ }
+
+ /**
+ * Prune out segments which are empty
+ */
+ @Override
+ public Set<String> prune(BrokerRequest brokerRequest, Set<String> segments) {
+ Set<String> selectedSegments = new HashSet<>(segments);
Review comment:
i remembered why I'd done it that way. The segments set was immutable, and operating on it directly resulted in Unsupported Operation 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r561174145
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/data/readers/PinotSegmentRecordReader.java
##########
@@ -81,25 +81,29 @@ public PinotSegmentRecordReader(File indexDir, @Nullable Schema schema, @Nullabl
_schema = new SegmentMetadataImpl(indexDir).getSchema();
Collection<String> columnNames = _schema.getColumnNames();
_columnReaderMap = new HashMap<>(columnNames.size());
- for (String columnName : columnNames) {
- _columnReaderMap.put(columnName, new PinotSegmentColumnReader(_immutableSegment, columnName));
+ if (_numDocs > 0) {
Review comment:
Instead of sprinkling `if (_numDocs > 0)`, can we branch out for `(if _numDocs == 0)` early on and handle empty segment separately?
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-765857360
How would retention manager work on these 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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-io commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-765818572
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=h1) Report
> Merging [#6466](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=desc) (1263253) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `22.57%`.
> The diff coverage is `43.91%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #6466 +/- ##
===========================================
- Coverage 66.44% 43.87% -22.58%
===========================================
Files 1075 1335 +260
Lines 54773 65656 +10883
Branches 8168 9581 +1413
===========================================
- Hits 36396 28804 -7592
- Misses 15700 34410 +18710
+ Partials 2677 2442 -235
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `43.87% <43.91%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
| [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
| [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
| [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
| [...ceselector/StrictReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1N0cmljdFJlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/TimeSegmentPruner.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL1RpbWVTZWdtZW50UHJ1bmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...roker/routing/segmentpruner/interval/Interval.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...r/routing/segmentpruner/interval/IntervalTree.java](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9zZWdtZW50cHJ1bmVyL2ludGVydmFsL0ludGVydmFsVHJlZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [1330 more](https://codecov.io/gh/apache/incubator-pinot/pull/6466/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=footer). Last update [dde3c18...1263253](https://codecov.io/gh/apache/incubator-pinot/pull/6466?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#issuecomment-776930773
I still think an easier way is to just complete the empty segment without building it It will work for the realtime path, where this is needed.
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466#discussion_r572502585
##########
File path: pinot-broker/src/main/java/org/apache/pinot/broker/routing/segmentpruner/EmptySegmentPruner.java
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.broker.routing.segmentpruner;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import javax.annotation.Nullable;
+import org.apache.helix.AccessOption;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.IdealState;
+import org.apache.helix.store.zk.ZkHelixPropertyStore;
+import org.apache.pinot.common.metadata.ZKMetadataProvider;
+import org.apache.pinot.common.request.BrokerRequest;
+import org.apache.pinot.common.utils.CommonConstants;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * The {@code EmptySegmentPruner} prunes segments if they have 0 totalDocs.
+ * Does not prune segments with -1 total docs (that can be either error case or CONSUMING segment)
+ */
+public class EmptySegmentPruner implements SegmentPruner {
+ private static final Logger LOGGER = LoggerFactory.getLogger(EmptySegmentPruner.class);
+
+ private final String _tableNameWithType;
+ private final ZkHelixPropertyStore<ZNRecord> _propertyStore;
+ private final String _segmentZKMetadataPathPrefix;
+
+ private final Map<String, Long> _segmentTotalDocsMap = new HashMap<>();
+ private final Set<String> _emptySegments = new HashSet<>();
+
+ public EmptySegmentPruner(TableConfig tableConfig, ZkHelixPropertyStore<ZNRecord> propertyStore) {
+ _tableNameWithType = tableConfig.getTableName();
+ _propertyStore = propertyStore;
+ _segmentZKMetadataPathPrefix = ZKMetadataProvider.constructPropertyStorePathForResource(_tableNameWithType) + "/";
+ }
+
+ @Override
+ public void init(ExternalView externalView, IdealState idealState, Set<String> onlineSegments) {
+ // Bulk load info for all online segments
+ int numSegments = onlineSegments.size();
+ List<String> segments = new ArrayList<>(numSegments);
+ List<String> segmentZKMetadataPaths = new ArrayList<>(numSegments);
+ for (String segment : onlineSegments) {
+ segments.add(segment);
+ segmentZKMetadataPaths.add(_segmentZKMetadataPathPrefix + segment);
+ }
+ List<ZNRecord> znRecords = _propertyStore.get(segmentZKMetadataPaths, null, AccessOption.PERSISTENT, false);
+ for (int i = 0; i < numSegments; i++) {
+ String segment = segments.get(i);
+ long totalDocs = extractTotalDocsFromSegmentZKMetaZNRecord(segment, znRecords.get(i));
+ _segmentTotalDocsMap.put(segment, totalDocs);
+ if (totalDocs == 0) {
+ _emptySegments.add(segment);
+ }
+ }
+ }
+
+ private long extractTotalDocsFromSegmentZKMetaZNRecord(String segment, @Nullable ZNRecord znRecord) {
+ if (znRecord == null) {
+ LOGGER.warn("Failed to find segment ZK metadata for segment: {}, table: {}", segment, _tableNameWithType);
+ return -1;
+ }
+ return znRecord.getLongField(CommonConstants.Segment.TOTAL_DOCS, -1);
+ }
+
+ @Override
+ public synchronized void onExternalViewChange(ExternalView externalView, IdealState idealState,
+ Set<String> onlineSegments) {
+ // NOTE: We don't update all the segment ZK metadata for every external view change, but only the new added/removed
+ // ones. The refreshed segment ZK metadata change won't be picked up.
+ for (String segment : onlineSegments) {
+ _segmentTotalDocsMap.computeIfAbsent(segment, k -> {
+ long totalDocs = extractTotalDocsFromSegmentZKMetaZNRecord(k,
+ _propertyStore.get(_segmentZKMetadataPathPrefix + k, null, AccessOption.PERSISTENT));
+ if (totalDocs == 0) {
+ _emptySegments.add(segment);
+ }
+ return totalDocs;
+ });
+ }
+ _segmentTotalDocsMap.keySet().retainAll(onlineSegments);
+ _emptySegments.retainAll(onlineSegments);
+ }
+
+ @Override
+ public synchronized void refreshSegment(String segment) {
+ long totalDocs = extractTotalDocsFromSegmentZKMetaZNRecord(segment,
+ _propertyStore.get(_segmentZKMetadataPathPrefix + segment, null, AccessOption.PERSISTENT));
+ _segmentTotalDocsMap.put(segment, totalDocs);
+ if (totalDocs == 0) {
+ _emptySegments.add(segment);
+ } else {
+ _emptySegments.remove(segment);
+ }
+ }
+
+ /**
+ * Prune out segments which are empty
+ */
+ @Override
+ public Set<String> prune(BrokerRequest brokerRequest, Set<String> segments) {
+ Set<String> selectedSegments = new HashSet<>(segments);
Review comment:
No need to make a copy of the segments. You can directly remove empty segments from it and return.
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/indexsegment/immutable/ImmutableSegmentLoader.java
##########
@@ -105,32 +105,34 @@ public static ImmutableSegment load(File indexDir, IndexLoadingConfig indexLoadi
// Load the segment
ReadMode readMode = indexLoadingConfig.getReadMode();
SegmentDirectory segmentDirectory = SegmentDirectory.createFromLocalFS(indexDir, segmentMetadata, readMode);
- SegmentDirectory.Reader segmentReader = segmentDirectory.createReader();
Map<String, ColumnIndexContainer> indexContainerMap = new HashMap<>();
- for (Map.Entry<String, ColumnMetadata> entry : segmentMetadata.getColumnMetadataMap().entrySet()) {
- indexContainerMap.put(entry.getKey(),
- new PhysicalColumnIndexContainer(segmentReader, entry.getValue(), indexLoadingConfig, indexDir));
- }
+ StarTreeIndexContainer starTreeIndexContainer = null;
- // Instantiate virtual columns
- Schema segmentSchema = segmentMetadata.getSchema();
- VirtualColumnProviderFactory.addBuiltInVirtualColumnsToSegmentSchema(segmentSchema, segmentName);
- for (FieldSpec fieldSpec : segmentSchema.getAllFieldSpecs()) {
- if (fieldSpec.isVirtualColumn()) {
- String columnName = fieldSpec.getName();
- VirtualColumnContext context = new VirtualColumnContext(fieldSpec, segmentMetadata.getTotalDocs());
- VirtualColumnProvider provider = VirtualColumnProviderFactory.buildProvider(context);
- indexContainerMap.put(columnName, provider.buildColumnIndexContainer(context));
- segmentMetadata.getColumnMetadataMap().put(columnName, provider.buildMetadata(context));
+ if (segmentMetadata.getTotalDocs() > 0) {
Review comment:
Do we need to specialize this part? This will make empty `ImmutableSegmentImpl` inconsistent with regular segment
##########
File path: pinot-broker/src/main/java/org/apache/pinot/broker/routing/timeboundary/TimeBoundaryManager.java
##########
@@ -113,13 +113,22 @@ public void init(ExternalView externalView, IdealState idealState, Set<String> o
long maxEndTimeMs = INVALID_END_TIME_MS;
for (int i = 0; i < numSegments; i++) {
String segment = segments.get(i);
- long endTimeMs = extractEndTimeMsFromSegmentZKMetadataZNRecord(segment, znRecords.get(i));
+ long endTimeMs = extractEndTimeMs(segment, znRecords.get(i));
_endTimeMsMap.put(segment, endTimeMs);
maxEndTimeMs = Math.max(maxEndTimeMs, endTimeMs);
}
updateTimeBoundaryInfo(maxEndTimeMs);
}
+ private long extractEndTimeMs(String segment, @Nullable ZNRecord znRecord) {
Review comment:
Suggest adding the extra totalDocs check logic into the `extractEndTimeMsFromSegmentZKMetadataZNRecord`. No need to add these 2 new methods
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] npawar merged pull request #6466: Handle creation of segments with 0 rows
Posted by GitBox <gi...@apache.org>.
npawar merged pull request #6466:
URL: https://github.com/apache/incubator-pinot/pull/6466
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org