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 2020/07/20 00:59:49 UTC
[GitHub] [incubator-pinot] guruguha opened a new pull request #5718: Feature/#5390 segment indexing reload status api
guruguha opened a new pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718
## Description
This PR adds to two APIs on the controller and one new API on the server. The purpose of the APIs is to provide segment reload status and the segment metadata from the Pinot Server.
Following updates need to be made in the documentation:
- Need to add documentation for querying the segments reload status API
- Need to update documentation for querying segment metadata from the server
## Documentation
The documentation to the PR is [here](https://docs.google.com/document/d/1E_J7PxF9WtaE6ido__u0O-Emtyu0IghQicC-1eLZeRo/edit#heading=h.8t9gsm5i7ove).
----------------------------------------------------------------
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-commenter edited a comment on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-685294191
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@7708341`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `62.02%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5718 +/- ##
=========================================
Coverage ? 67.15%
=========================================
Files ? 1205
Lines ? 63599
Branches ? 9741
=========================================
Hits ? 42708
Misses ? 17747
Partials ? 3144
```
| Flag | Coverage Δ | |
|---|---|---|
| #integration | `43.55% <0.63%> (?)` | |
| #unittests | `58.36% <62.02%> (?)` | |
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/5718?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...che/pinot/controller/util/TableMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlTWV0YWRhdGFSZWFkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `14.19% <5.88%> (ø)` | |
| [...che/pinot/server/api/resources/TablesResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | `72.83% <25.00%> (ø)` | |
| [...ontroller/api/resources/ServerTableSizeReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1NlcnZlclRhYmxlU2l6ZVJlYWRlci5qYXZh) | `83.87% <58.33%> (ø)` | |
| [...t/server/api/resources/SegmentMetadataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9TZWdtZW50TWV0YWRhdGFGZXRjaGVyLmphdmE=) | `70.45% <70.45%> (ø)` | |
| [...t/controller/util/ServerSegmentMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlcnZlclNlZ21lbnRNZXRhZGF0YVJlYWRlci5qYXZh) | `84.37% <84.37%> (ø)` | |
| [...pinot/controller/util/CompletionServiceHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbXBsZXRpb25TZXJ2aWNlSGVscGVyLmphdmE=) | `93.75% <93.75%> (ø)` | |
| [...e/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `74.35% <100.00%> (ø)` | |
| [...pinot/core/util/IntDoubleIndexedPriorityQueue.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0ludERvdWJsZUluZGV4ZWRQcmlvcml0eVF1ZXVlLmphdmE=) | `85.36% <0.00%> (ø)` | |
| [...java/org/apache/pinot/spi/utils/DataSizeUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvRGF0YVNpemVVdGlscy5qYXZh) | `88.23% <0.00%> (ø)` | |
| ... and [1203 more](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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/5718?src=pr&el=footer). Last update [7708341...b36d4cf](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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] kishoreg commented on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-671062149
is this ready to go?
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463833747
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
Review comment:
Thanks for the suggestion. Makes sense to add limit. I did think about this, but then, the issue will be knowing the status of the remaining segments. For a table with say, 1000 segments, how do we let the user know of the status of the rest of the 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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460492664
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -246,21 +268,7 @@
return segmentMetadata;
} else {
throw new ControllerApplicationException(LOGGER,
- "Failed to find segment: " + segmentName + " in table: " + tableName, Status.NOT_FOUND);
- }
- }
-
- @Nullable
Review comment:
my bad.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460489744
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
SegmentStatus is used in the server code as well. So, added a class to common for referencing.
----------------------------------------------------------------
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 a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463895616
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
Review comment:
If you are adding one to get the status of one segment at a time, then the user can (if needed) iterate over the segments and get each segment. Let us evaluate the use case first. Are we talking about a full table reload or a segment reload? If full table reload, maybe we only want to return those segments that DID NOT reload properly?
The API definition leaves much discussion to be desired, and a PR is NOT the place to discuss API. If you have a design doc, we will discuss there.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r479518772
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+/**
+ * Holds segment last reload time status along with any errors for a segment with unsuccessful call to get reload times.
+ *
+ * NOTE: This class is being used in both the controller and the server. There is tight coupling between them.
+ * So, the API contract cannot be changed without changing or refactoring this class.
+ *
+ * TODO: refactor this class to be handled better. Make sure to have an extensible design that helps add more
+ */
+public class SegmentStatus {
+ // Name of the segment itself
+ public String _segmentName;
+ // The last segment reload time in ISO date format (yyyy-MM-dd HH:mm:ss:SSS UTC)
+ // If the segment reload failed for a segment, then the value will be the previous segment reload was successful
+ public String _segmentReloadTimeUTC;
Review comment:
I have updated the API to return long instead of String
----------------------------------------------------------------
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] kishoreg commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
kishoreg commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r469006145
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
Review comment:
this can be done in another PR. Lets get this in and add the optimizations as we need them. Million segments in a table is not a common use 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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463892137
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
Review comment:
Updated in latest commit.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463838803
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
Thanks for the suggestions! I will think about how to refactor my code and commit 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:
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-commenter edited a comment on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-685294191
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@7708341`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `60.51%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5718 +/- ##
=========================================
Coverage ? 67.10%
=========================================
Files ? 1200
Lines ? 63221
Branches ? 9683
=========================================
Hits ? 42426
Misses ? 17671
Partials ? 3124
```
| Flag | Coverage Δ | |
|---|---|---|
| #integration | `43.28% <0.42%> (?)` | |
| #unittests | `58.63% <60.51%> (?)` | |
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/5718?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...che/pinot/controller/util/TableMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlTWV0YWRhdGFSZWFkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `12.86% <3.03%> (ø)` | |
| [...che/pinot/server/api/resources/TablesResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | `69.89% <43.75%> (ø)` | |
| [...ontroller/api/resources/ServerTableSizeReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1NlcnZlclRhYmxlU2l6ZVJlYWRlci5qYXZh) | `83.87% <58.33%> (ø)` | |
| [...t/server/api/resources/SegmentMetadataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9TZWdtZW50TWV0YWRhdGFGZXRjaGVyLmphdmE=) | `72.34% <72.34%> (ø)` | |
| [...t/controller/util/ServerSegmentMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlcnZlclNlZ21lbnRNZXRhZGF0YVJlYWRlci5qYXZh) | `84.37% <84.37%> (ø)` | |
| [...ot/common/restlet/resources/SegmentLoadStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudExvYWRTdGF0dXMuamF2YQ==) | `87.50% <87.50%> (ø)` | |
| [...pinot/controller/util/CompletionServiceHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbXBsZXRpb25TZXJ2aWNlSGVscGVyLmphdmE=) | `93.75% <93.75%> (ø)` | |
| [...e/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `74.35% <100.00%> (ø)` | |
| [...restlet/resources/StartReplaceSegmentsRequest.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU3RhcnRSZXBsYWNlU2VnbWVudHNSZXF1ZXN0LmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [1199 more](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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/5718?src=pr&el=footer). Last update [7708341...2cf5629](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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] guruguha commented on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-671066095
There were a few concerns raised by Subbu on the number of API calls that
would be made to a server in the case where there are a large number of
segments.
Wanted to discuss further on what to do next for this feature. What do you
suggest?
On Sun, Aug 9, 2020 at 7:58 AM Kishore Gopalakrishna <
notifications@github.com> wrote:
> is this ready to go?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-pinot/pull/5718#issuecomment-671062149>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACFCTWVTNNAWM3UHNLGNRV3R722SLANCNFSM4PBWT4GA>
> .
>
----------------------------------------------------------------
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] guruguha commented on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-673286243
> > A lot of code duplication with the existing table size reader. Please find ways to use a common base class if possible
>
> Is this addressed?
Oh! Somehow missed this comment. I think it got lost in between. Let me make this change and commit again. Sorry about that!
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460491469
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.controller.api.resources.ServerSegmentMetadataReader;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TableMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableMetadataReader.class);
+
+ private final Executor executor;
+ private final HttpConnectionManager connectionManager;
+ private final PinotHelixResourceManager pinotHelixResourceManager;
+
+ public TableMetadataReader(Executor executor, HttpConnectionManager connectionManager,
+ PinotHelixResourceManager helixResourceManager) {
+ this.executor = executor;
+ this.connectionManager = connectionManager;
+ this.pinotHelixResourceManager = helixResourceManager;
+ }
+
+ public TableReloadStatus getReloadStatus(String tableNameWithType, Map<String, List<String>> serverToSegmentsMap,
Review comment:
It is outside in both methods as the "server side" classes don't have the resourceManager object. If it is moved to server side classes then the resourceManager object needs to be passed on - which might be overkill?
----------------------------------------------------------------
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] kishoreg commented on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-673284226
> A lot of code duplication with the existing table size reader. Please find ways to use a common base class if possible
Is this addressed?
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r461303906
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
Working on changing the equals and hashcode. Will commit the changes.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
Actually, we may not need the equality methods yet, removing them.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.controller.api.resources.ServerSegmentMetadataReader;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TableMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableMetadataReader.class);
+
+ private final Executor executor;
+ private final HttpConnectionManager connectionManager;
+ private final PinotHelixResourceManager pinotHelixResourceManager;
+
+ public TableMetadataReader(Executor executor, HttpConnectionManager connectionManager,
+ PinotHelixResourceManager helixResourceManager) {
+ this.executor = executor;
+ this.connectionManager = connectionManager;
+ this.pinotHelixResourceManager = helixResourceManager;
+ }
+
+ public TableReloadStatus getReloadStatus(String tableNameWithType, Map<String, List<String>> serverToSegmentsMap,
Review comment:
I see, got it. updated the same.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460489410
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/SegmentColumnIndexesFetcher.java
##########
@@ -0,0 +1,83 @@
+/**
+ * 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.server.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.pinot.core.data.manager.SegmentDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegment;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegmentImpl;
+import org.apache.pinot.core.segment.index.column.ColumnIndexContainer;
+import org.apache.pinot.spi.utils.JsonUtils;
+
+public class SegmentColumnIndexesFetcher {
+ public static JsonNode getIndexesForSegmentColumns(SegmentDataManager segmentDataManager, Set<String> columnSet) {
+ ArrayNode columnsIndexMetadata = JsonUtils.newArrayNode();
+ if (segmentDataManager instanceof ImmutableSegmentDataManager) {
+ ImmutableSegmentDataManager immutableSegmentDataManager = (ImmutableSegmentDataManager) segmentDataManager;
+ ImmutableSegment immutableSegment = immutableSegmentDataManager.getSegment();
+ if (immutableSegment instanceof ImmutableSegmentImpl) {
+ ImmutableSegmentImpl immutableSegmentImpl = (ImmutableSegmentImpl) immutableSegment;
+// Set<String> columns = immutableSegmentImpl.getSegmentMetadata().getAllColumns();
+ Map<String, ColumnIndexContainer> columnIndexContainerMap = immutableSegmentImpl.getIndexContainerMap();
+ columnsIndexMetadata.add(getImmutableSegmentColumnIndexes(columnIndexContainerMap, columnSet));
+ }
+ }
+ return columnsIndexMetadata;
+ }
+
+ private static ObjectNode getImmutableSegmentColumnIndexes(Map<String, ColumnIndexContainer> columnIndexContainerMap,
+ Set<String> columnSet) {
+ ObjectNode columnIndexMap = JsonUtils.newObjectNode();
+
+ for (Map.Entry<String, ColumnIndexContainer> e : columnIndexContainerMap.entrySet()) {
+ if (columnSet != null && !columnSet.contains(e.getKey())) {
+ continue;
+ }
+ ColumnIndexContainer columnIndexContainer = e.getValue();
+ ObjectNode indexesNode = JsonUtils.newObjectNode();
+ if (Objects.isNull(columnIndexContainer.getBloomFilter())) indexesNode.put("bloom-filter", "NO");
+ else indexesNode.put("bloom-filter", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getDictionary())) indexesNode.put("dictionary", "NO");
+ else indexesNode.put("dictionary", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getForwardIndex())) indexesNode.put("forward-index", "NO");
+ else indexesNode.put("forward-index", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getInvertedIndex())) indexesNode.put("inverted-index", "NO");
+ else indexesNode.put("inverted-index", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getNullValueVector()))
+ indexesNode.put("null-value-vector-reader", "NO");
+ else indexesNode.put("null-value-vector-reader", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getNullValueVector())) indexesNode.put("range-index", "NO");
+ else indexesNode.put("range-index", "YES");
+
+ columnIndexMap.set(e.getKey(), indexesNode);
+ }
+ return columnIndexMap;
Review comment:
The segment metadata is a JsonNode converted to a json string. I thought of using the same object conversion so that it is kept consistent.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460490074
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ continue;
+ }
+ JsonNode segmentMetadata =
+ JsonUtils.inputStreamToJsonNode(getMethod.getResponseBodyAsStream());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ LOGGER.info("Updated segment metadata: {}", segmentMetadata.size());
Review comment:
changed log message
----------------------------------------------------------------
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-commenter edited a comment on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-685294191
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@7708341`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `0.42%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5718 +/- ##
=========================================
Coverage ? 43.43%
=========================================
Files ? 1197
Lines ? 62844
Branches ? 9584
=========================================
Hits ? 27298
Misses ? 33097
Partials ? 2449
```
| Flag | Coverage Δ | |
|---|---|---|
| #integration | `43.43% <0.42%> (?)` | |
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/5718?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ot/common/restlet/resources/SegmentLoadStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudExvYWRTdGF0dXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ontroller/api/resources/ServerTableSizeReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1NlcnZlclRhYmxlU2l6ZVJlYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...pinot/controller/util/CompletionServiceHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbXBsZXRpb25TZXJ2aWNlSGVscGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...t/controller/util/ServerSegmentMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlcnZlclNlZ21lbnRNZXRhZGF0YVJlYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/controller/util/TableMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlTWV0YWRhdGFSZWFkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...e/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `64.10% <0.00%> (ø)` | |
| [...t/server/api/resources/SegmentMetadataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9TZWdtZW50TWV0YWRhdGFGZXRjaGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/server/api/resources/TablesResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `11.11% <3.03%> (ø)` | |
| [.../segment/index/datasource/ImmutableDataSource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zZWdtZW50L2luZGV4L2RhdGFzb3VyY2UvSW1tdXRhYmxlRGF0YVNvdXJjZS5qYXZh) | `88.88% <0.00%> (ø)` | |
| ... and [1196 more](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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/5718?src=pr&el=footer). Last update [7708341...ac98a41](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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 a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r457517934
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
Review comment:
If you are returning 404 (NOT_FOUND) then please do not use "Failed" in the exception message. Since the exception is invalid config, determine what is invalid and throw that exception, may be as 400 (BAD_REQUEST)
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
Review comment:
If there are no tables found, then this is the place to throw 404
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -176,23 +198,23 @@
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "Get a map from server to segments hosted by the server (deprecated, use 'GET /segments/{tableName}/servers' instead)", notes = "Get a map from server to segments hosted by the server (deprecated, use 'GET /segments/{tableName}/servers' instead)")
public List<Map<String, String>> getServerToSegmentsMapDeprecated1(
- @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
- @ApiParam(value = "MUST be null") @QueryParam("state") String stateStr,
- @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr)
- throws JsonProcessingException {
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
Review comment:
Is your IDE set to pinot coding guidelines?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
Please document each member in this object clearly, what it contains in various situations
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,43 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+/**
+ * Holds segment last reload time status along with any errors for a segment with unsuccessful call to get reload times.
+ */
+public class SegmentStatus {
+ public String _segmentName;
+ public String _segmentReloadTime;
Review comment:
* Why is this a "String"? Why not 'long'?
* Please clearly indicate the time unit in the field name. e.g. `segmentReloadTimeUTCMs`
* What is the value in this field if the segment was never reloaded, or the server restarts after the reload? Or, the reload faced an error and failed?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, ServerSegmentMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, ServerSegmentMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ ServerSegmentMetadataReader.TableReloadStatus tableReloadStatus;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ }
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private ServerSegmentMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the server metadata for all table segments", notes = "Get the server metadata for all table segments")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ LOGGER.info("Received a request to fetch metadata for all segments for table {}", tableName);
Review comment:
Seems like a debug level log.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
I strongly suggest we return a different object to the user instead of returning this one. It will enable us to evolve the internal interface independent of interface exposed to the user.
Also, the controller returned status could include other run-time status about a segment. For example:
1. Helix externalview of the server so we know if it is online or not.
2. The crc, size, date uploaded/refreshed, etc. from segment metadata
3. Other info as wee want to add later (e.g. number of times segment was hit/searched/selected since reboot, etc.)
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
Review comment:
Let us call it "loadStatus" or "runTimeStatus" (I think all our APIs are in camel case). And change the comments/String below appropriately
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
So, you are using this same class in both Controller as well as Server APIs. That is nice but has its pit falls. Imagine a case when we add a new field to this object. We cannot upgrade controllers and servers at the same time. So, there can exist a situation where servers are sending the old object (serialized) and the controller is trying to deserialize them using the new object. Or, vice versa.
At the minimum:
1. Write a block of comment at the top of the class that this class is upgrade sensitive explaining what may happen if it is changed without regard to upgrade consideration.
2. Specifically mention that fields cannot be removed from the class (I suppose, unless there are proper defaults)
3. Add an annotation to ignore unknown fields at the class level.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, ServerSegmentMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, ServerSegmentMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ ServerSegmentMetadataReader.TableReloadStatus tableReloadStatus;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ }
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private ServerSegmentMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the server metadata for all table segments", notes = "Get the server metadata for all table segments")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ LOGGER.info("Received a request to fetch metadata for all segments for table {}", tableName);
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ String tableNameWithType = getExistingTableNamesWithType(tableName, tableType).get(0);
+ Map<String, String> segmentsMetadata;
+ try {
+ segmentsMetadata = getSegmentsMetadataFromServer(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ } catch (IOException ioe) {
+ throw new ControllerApplicationException(LOGGER,
+ "Error parsing Pinot server response: " + ioe.getMessage(), Status.INTERNAL_SERVER_ERROR, ioe);
Review comment:
Indicate the server name here that caused the error (unless that is logged elsewhere)
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,121 @@
+package org.apache.pinot.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
Review comment:
thy is this class in controller/api/resources? It should be inside controller/util
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
Review comment:
The exception message reads as if the table is not found. That is not the case, right?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME)
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, String> segmentsMetadata = null;
+ try {
+ segmentsMetadata = getSegmentsMetadataFromServer(tableNamesWithType.get(0));
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ } catch (IOException ioe) {
+ throw new ControllerApplicationException(LOGGER,
+ "Error parsing response to cluster config!", Response.Status.BAD_REQUEST, ioe);
+ }
+ if (segmentsMetadata == null)
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ return segmentsMetadata;
+ }
+
+ private Map<String, String> getSegmentsMetadataFromServer(String tableNameWithType)
+ throws InvalidConfigException, IOException {
+ LOGGER.trace("Inside getSegmentsMetadataFromServer() entry");
Review comment:
remove these trace logs please
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
Review comment:
I strongly suggest adding a verbosity level and/or a limit here. Can be added later if you wish. Imagine a table with a million segments. Do we really want to kill the servers trying to query all the segments? Or, output them only to let the client time out?
An example could be: limit=100 by default, verbosity=5. A level of 4, 3, 2,1 will show less information for each segment. Maybe 0 will only show how many segments that are online/offline etc.?
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460490918
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ continue;
+ }
+ JsonNode segmentMetadata =
+ JsonUtils.inputStreamToJsonNode(getMethod.getResponseBodyAsStream());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ LOGGER.info("Updated segment metadata: {}", segmentMetadata.size());
+ } catch (Exception e) {
+ // Ignore individual exceptions because the exception has been logged in MultiGetRequest
+ // Log the number of failed servers after gathering all responses
+ } finally {
+ if (Objects.nonNull(getMethod)) {
+ getMethod.releaseConnection();
+ }
+ }
+ }
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/metadata";
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/reload-status";
+ }
+
+ public List<SegmentStatus> getSegmentReloadTime(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment reload status from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateReloadStatusServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ List<SegmentStatus> segmentsStatus = new ArrayList<>();
+
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
Review comment:
Would the message be formatted in anyway or will it be a full stack trace?
----------------------------------------------------------------
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-commenter commented on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-685294191
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@7708341`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `0.42%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/5718?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #5718 +/- ##
=========================================
Coverage ? 43.41%
=========================================
Files ? 1197
Lines ? 62844
Branches ? 9584
=========================================
Hits ? 27284
Misses ? 33103
Partials ? 2457
```
| Flag | Coverage Δ | |
|---|---|---|
| #integration | `43.41% <0.42%> (?)` | |
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/5718?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ot/common/restlet/resources/SegmentLoadStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvU2VnbWVudExvYWRTdGF0dXMuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ontroller/api/resources/ServerTableSizeReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1NlcnZlclRhYmxlU2l6ZVJlYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...pinot/controller/util/CompletionServiceHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbXBsZXRpb25TZXJ2aWNlSGVscGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...t/controller/util/ServerSegmentMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1NlcnZlclNlZ21lbnRNZXRhZGF0YVJlYWRlci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/controller/util/TableMetadataReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlTWV0YWRhdGFSZWFkZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...e/indexsegment/immutable/ImmutableSegmentImpl.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | `64.10% <0.00%> (ø)` | |
| [...t/server/api/resources/SegmentMetadataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9TZWdtZW50TWV0YWRhdGFGZXRjaGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/server/api/resources/TablesResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `11.11% <3.03%> (ø)` | |
| [...e/pinot/core/transport/InstanceRequestHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvSW5zdGFuY2VSZXF1ZXN0SGFuZGxlci5qYXZh) | `74.46% <0.00%> (ø)` | |
| ... and [1196 more](https://codecov.io/gh/apache/incubator-pinot/pull/5718/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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/5718?src=pr&el=footer). Last update [7708341...ac98a41](https://codecov.io/gh/apache/incubator-pinot/pull/5718?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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460497483
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
Review comment:
You're right, we're skipping CONSUMING segments in both endpoints. Added the check for both endpoints.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460493179
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
Review comment:
since it is a propagated exception, it is better to have the message originally thrown. Updating the same.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460491623
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.controller.api.resources.ServerSegmentMetadataReader;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TableMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableMetadataReader.class);
+
+ private final Executor executor;
+ private final HttpConnectionManager connectionManager;
+ private final PinotHelixResourceManager pinotHelixResourceManager;
+
+ public TableMetadataReader(Executor executor, HttpConnectionManager connectionManager,
+ PinotHelixResourceManager helixResourceManager) {
+ this.executor = executor;
+ this.connectionManager = connectionManager;
+ this.pinotHelixResourceManager = helixResourceManager;
+ }
+
+ public TableReloadStatus getReloadStatus(String tableNameWithType, Map<String, List<String>> serverToSegmentsMap,
+ int timeoutMs)
+ throws InvalidConfigException {
+ BiMap<String, String> endpoints = pinotHelixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
+ ServerSegmentMetadataReader serverSegmentMetadataReader = new ServerSegmentMetadataReader(executor, connectionManager);
+
+ List<SegmentStatus> segmentStatus = serverSegmentMetadataReader.getSegmentReloadTime(tableNameWithType, serverToSegmentsMap, endpoints, timeoutMs);
+ TableReloadStatus tableReloadStatus = new TableReloadStatus();
+ tableReloadStatus._tableName = tableNameWithType;
+ tableReloadStatus._segmentStatus = segmentStatus;
+ return tableReloadStatus;
+ }
+
+ public Map<String, String> getSegmentsMetadata(String tableNameWithType, int timeoutMs) throws InvalidConfigException, IOException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ BiMap<String, String> endpoints = pinotHelixResourceManager.getDataInstanceAdminEndpoints(serversToSegmentsMap.keySet());
+ ServerSegmentMetadataReader serverSegmentMetadataReader = new ServerSegmentMetadataReader(executor, connectionManager);
+
+ List<String> segmentsMetadata = serverSegmentMetadataReader.getSegmentMetadataFromServer(tableNameWithType,
+ serversToSegmentsMap, endpoints, timeoutMs);
+
+ Map<String, String> response = new HashMap<>();
+ for (String segmentMetadata : segmentsMetadata) {
+ JsonNode responseJson = JsonUtils.stringToJsonNode(segmentMetadata);
+ ObjectNode objectNode = responseJson.deepCopy();
Review comment:
I had it for a different logic - will remove.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460495237
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME)
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, String> segmentsMetadata = null;
+ try {
+ segmentsMetadata = getSegmentsMetadataFromServer(tableNamesWithType.get(0));
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ } catch (IOException ioe) {
+ throw new ControllerApplicationException(LOGGER,
+ "Error parsing response to cluster config!", Response.Status.BAD_REQUEST, ioe);
+ }
+ if (segmentsMetadata == null)
Review comment:
That exception has been updated to the propagated exception. Also, as mentioned before, the `segmentsMetadata` will not be `null`.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r475101707
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
I think we do have that separation. We have a class `TableReloadStatus` in the controller that can be used to enhance any future requirements. The `SegmentLoadStatus` class can be then used for specific API - the loadStatus API.
I have added JSON ignore to the `SegmentLoadStatus` class.
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+/**
+ * Holds segment last reload time status along with any errors for a segment with unsuccessful call to get reload times.
+ *
+ * NOTE: This class is being used in both the controller and the server. There is tight coupling between them.
+ * So, the API contract cannot be changed without changing or refactoring this class.
+ *
+ * TODO: refactor this class to be handled better. Make sure to have an extensible design that helps add more
+ */
+public class SegmentStatus {
+ // Name of the segment itself
+ public String _segmentName;
+ // The last segment reload time in ISO date format (yyyy-MM-dd HH:mm:ss:SSS UTC)
+ // If the segment reload failed for a segment, then the value will be the previous segment reload was successful
+ public String _segmentReloadTimeUTC;
Review comment:
The date is in string format: ISO date format (yyyy-MM-dd HH:mm:ss:SSS UTC)
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463895752
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, ServerSegmentMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, ServerSegmentMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ ServerSegmentMetadataReader.TableReloadStatus tableReloadStatus;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ }
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private ServerSegmentMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the server metadata for all table segments", notes = "Get the server metadata for all table segments")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ LOGGER.info("Received a request to fetch metadata for all segments for table {}", tableName);
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ String tableNameWithType = getExistingTableNamesWithType(tableName, tableType).get(0);
+ Map<String, String> segmentsMetadata;
+ try {
+ segmentsMetadata = getSegmentsMetadataFromServer(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ } catch (IOException ioe) {
+ throw new ControllerApplicationException(LOGGER,
+ "Error parsing Pinot server response: " + ioe.getMessage(), Status.INTERNAL_SERVER_ERROR, ioe);
Review comment:
Yes, is logged in the helper class
----------------------------------------------------------------
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 #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460572555
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
Could you change equals and hashcode to use EqualityUtils? Look at other classes in pinot-api for reference
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.controller.api.resources.ServerSegmentMetadataReader;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TableMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableMetadataReader.class);
+
+ private final Executor executor;
+ private final HttpConnectionManager connectionManager;
+ private final PinotHelixResourceManager pinotHelixResourceManager;
+
+ public TableMetadataReader(Executor executor, HttpConnectionManager connectionManager,
+ PinotHelixResourceManager helixResourceManager) {
+ this.executor = executor;
+ this.connectionManager = connectionManager;
+ this.pinotHelixResourceManager = helixResourceManager;
+ }
+
+ public TableReloadStatus getReloadStatus(String tableNameWithType, Map<String, List<String>> serverToSegmentsMap,
Review comment:
oh i meant this call
```
final Map<String, List<String>> serverToSegments = _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
```
It is called in the getSegmentsMetadata, but for getReloadStatus, it is being passed from caller
----------------------------------------------------------------
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 #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460574185
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ continue;
+ }
+ JsonNode segmentMetadata =
+ JsonUtils.inputStreamToJsonNode(getMethod.getResponseBodyAsStream());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ LOGGER.info("Updated segment metadata: {}", segmentMetadata.size());
+ } catch (Exception e) {
+ // Ignore individual exceptions because the exception has been logged in MultiGetRequest
+ // Log the number of failed servers after gathering all responses
+ } finally {
+ if (Objects.nonNull(getMethod)) {
+ getMethod.releaseConnection();
+ }
+ }
+ }
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/metadata";
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/reload-status";
+ }
+
+ public List<SegmentStatus> getSegmentReloadTime(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment reload status from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateReloadStatusServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ List<SegmentStatus> segmentsStatus = new ArrayList<>();
+
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
Review comment:
Whatever it is is fine. Just something more than just a code
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
Review comment:
REALTIME tables have some CONSUMING segments (Mutable) and some ONLINE segments (Immutable). We want to still process the Immutable segments of the REALTIME table. The current implementation will skip REALTIME entirely. It's okay if you want to take that up in a follow-up. You can mention in description that this is only for OFFLINE as of 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:
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 #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r459165596
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -246,21 +268,7 @@
return segmentMetadata;
} else {
throw new ControllerApplicationException(LOGGER,
- "Failed to find segment: " + segmentName + " in table: " + tableName, Status.NOT_FOUND);
- }
- }
-
- @Nullable
Review comment:
i could spot no change in this method vs same method moved at the bottom. can we keep it here, so that the scope of review stays limited to the actual changes?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -128,20 +137,33 @@
@Api(tags = Constants.SEGMENT_TAG)
@Path("/")
public class PinotSegmentRestletResource {
- private static Logger LOGGER = LoggerFactory.getLogger(PinotSegmentRestletResource.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(PinotSegmentRestletResource.class);
+
+ @Inject
+ ControllerConf _controllerConf;
@Inject
PinotHelixResourceManager _pinotHelixResourceManager;
+ @Inject
+ Executor _executor;
+
+ @Inject
+ HttpConnectionManager _connectionManager;
+
+ @Inject
+ ControllerMetrics _controllerMetrics;
+
+
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/segments/{tableName}")
@ApiOperation(value = "List all segments", notes = "List all segments")
public List<Map<TableType, List<String>>> getSegments(
- @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
- @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
Review comment:
there seems to be some unnecessary formatting/whitespaces on all methods of this file. Could you revert those? you should be able to auto-format using the IDE
same comment for TablesResourceTest file
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460493987
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
Review comment:
This is taken care in the `getExistingTableNamesWithType` 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:
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 a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r474097491
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerTableSizeReader.java
##########
@@ -64,40 +62,25 @@ public ServerTableSizeReader(Executor executor, HttpConnectionManager connection
serverUrls.add(tableSizeUri);
}
- // TODO: use some service other than completion service so that we know which server encounters the error
- CompletionService<GetMethod> completionService =
- new MultiGetRequest(_executor, _connectionManager).execute(serverUrls, timeoutMs);
+ // Helper service to run a httpget call to the server
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager,
+ endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverUrls, tableNameWithType, timeoutMs);
Map<String, List<SegmentSizeInfo>> serverToSegmentSizeInfoListMap = new HashMap<>();
-
- for (int i = 0; i < numServers; i++) {
- GetMethod getMethod = null;
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
try {
- getMethod = completionService.take().get();
- URI uri = getMethod.getURI();
- String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
- if (getMethod.getStatusCode() >= 300) {
- LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
- continue;
- }
TableSizeInfo tableSizeInfo =
- JsonUtils.inputStreamToObject(getMethod.getResponseBodyAsStream(), TableSizeInfo.class);
- serverToSegmentSizeInfoListMap.put(instance, tableSizeInfo.segments);
- } catch (Exception e) {
- // Ignore individual exceptions because the exception has been logged in MultiGetRequest
- // Log the number of failed servers after gathering all responses
- } finally {
- if (getMethod != null) {
- getMethod.releaseConnection();
- }
+ JsonUtils.stringToObject(streamResponse.getValue(), TableSizeInfo.class);
+ serverToSegmentSizeInfoListMap.put(streamResponse.getKey(), tableSizeInfo.segments);
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
Review comment:
Add the server name to this log
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerTableSizeReader.java
##########
@@ -64,40 +62,25 @@ public ServerTableSizeReader(Executor executor, HttpConnectionManager connection
serverUrls.add(tableSizeUri);
}
- // TODO: use some service other than completion service so that we know which server encounters the error
- CompletionService<GetMethod> completionService =
- new MultiGetRequest(_executor, _connectionManager).execute(serverUrls, timeoutMs);
+ // Helper service to run a httpget call to the server
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager,
+ endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverUrls, tableNameWithType, timeoutMs);
Map<String, List<SegmentSizeInfo>> serverToSegmentSizeInfoListMap = new HashMap<>();
-
- for (int i = 0; i < numServers; i++) {
- GetMethod getMethod = null;
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
try {
- getMethod = completionService.take().get();
- URI uri = getMethod.getURI();
- String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
- if (getMethod.getStatusCode() >= 300) {
- LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
- continue;
- }
TableSizeInfo tableSizeInfo =
- JsonUtils.inputStreamToObject(getMethod.getResponseBodyAsStream(), TableSizeInfo.class);
- serverToSegmentSizeInfoListMap.put(instance, tableSizeInfo.segments);
- } catch (Exception e) {
- // Ignore individual exceptions because the exception has been logged in MultiGetRequest
- // Log the number of failed servers after gathering all responses
- } finally {
- if (getMethod != null) {
- getMethod.releaseConnection();
- }
+ JsonUtils.stringToObject(streamResponse.getValue(), TableSizeInfo.class);
+ serverToSegmentSizeInfoListMap.put(streamResponse.getKey(), tableSizeInfo.segments);
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
}
}
-
- int numServersResponded = serverToSegmentSizeInfoListMap.size();
- if (numServersResponded != numServers) {
- LOGGER.warn("Finish reading segment sizes for table: {} with {}/{} servers responded", tableNameWithType,
- numServersResponded, numServers);
- } else {
- LOGGER.info("Finish reading segment sizes for table: {}", tableNameWithType);
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment size info responses from server.", failedParses);
Review comment:
```suggestion
LOGGER.warn("Failed to parse segment size info responses from {} servers.", failedParses);
```
If possible, add the total number of servers to this message as well
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,163 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is a helper class that calls the server API endpoints to fetch server metadata and the segment reload status
+ * Only the servers returning success are returned by the method. For servers returning errors (http error or otherwise),
+ * no entry is created in the return list
+ */
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * This method makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serversToSegmentsMap
+ * @param endpoints
+ * @param timeoutMs
+ * @return list of segments and their metadata as a JSON string
+ */
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.debug("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ JsonNode segmentMetadata = JsonUtils.stringToJsonNode(streamResponse.getValue());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
+ }
+ }
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment metadata responses from server.", failedParses);
+ }
+
+ LOGGER.debug("Retrieved segment metadata from servers.");
Review comment:
add the number of servers to this log message
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,163 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is a helper class that calls the server API endpoints to fetch server metadata and the segment reload status
+ * Only the servers returning success are returned by the method. For servers returning errors (http error or otherwise),
+ * no entry is created in the return list
+ */
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * This method makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serversToSegmentsMap
+ * @param endpoints
+ * @param timeoutMs
+ * @return list of segments and their metadata as a JSON string
+ */
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.debug("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ JsonNode segmentMetadata = JsonUtils.stringToJsonNode(streamResponse.getValue());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
+ }
+ }
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment metadata responses from server.", failedParses);
Review comment:
```suggestion
LOGGER.warn("Failed to parse segment metadata responses from {} servers.", failedParses);
```
If possible add total number of servers to this message as well
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,163 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is a helper class that calls the server API endpoints to fetch server metadata and the segment reload status
+ * Only the servers returning success are returned by the method. For servers returning errors (http error or otherwise),
+ * no entry is created in the return list
+ */
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * This method makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serversToSegmentsMap
+ * @param endpoints
+ * @param timeoutMs
+ * @return list of segments and their metadata as a JSON string
+ */
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.debug("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ JsonNode segmentMetadata = JsonUtils.stringToJsonNode(streamResponse.getValue());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
+ }
+ }
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment metadata responses from server.", failedParses);
+ }
+
+ LOGGER.debug("Retrieved segment metadata from servers.");
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return String.format("http://%s/tables/%s/segments/%s/metadata", endpoint, tableNameWithType, segmentName);
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return String.format("http://%s/tables/%s/segments/%s/loadStatus", endpoint, tableNameWithType, segmentName);
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * It makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serverToSegments
+ * @param serverToEndpoint
+ * @param timeoutMs
+ * @return list of segments along with their last reload times
+ */
+ public TableReloadStatus getSegmentReloadTime(String tableNameWithType,
+ Map<String, List<String>> serverToSegments,
+ BiMap<String, String> serverToEndpoint, int timeoutMs) {
+ LOGGER.debug("Reading segment reload status from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegmentsEntry : serverToSegments.entrySet()) {
+ List<String> segments = serverToSegmentsEntry.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateReloadStatusServerURL(tableNameWithType, segment, serverToEndpoint.get(serverToSegmentsEntry.getKey())));
+ }
+ }
+
+ BiMap<String, String> endpointsToServers = serverToEndpoint.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<SegmentStatus> segmentsStatus = new ArrayList<>();
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ SegmentStatus segmentStatus = JsonUtils.stringToObject(streamResponse.getValue(), SegmentStatus.class);
+ segmentsStatus.add(segmentStatus);
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
+ }
+ }
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment load status responses from server.", failedParses);
Review comment:
```suggestion
LOGGER.warn("Failed to parse segment load status responses from {} servers.", failedParses);
```
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+/**
+ * Holds segment last reload time status along with any errors for a segment with unsuccessful call to get reload times.
+ *
+ * NOTE: This class is being used in both the controller and the server. There is tight coupling between them.
+ * So, the API contract cannot be changed without changing or refactoring this class.
+ *
+ * TODO: refactor this class to be handled better. Make sure to have an extensible design that helps add more
+ */
+public class SegmentStatus {
+ // Name of the segment itself
+ public String _segmentName;
+ // The last segment reload time in ISO date format (yyyy-MM-dd HH:mm:ss:SSS UTC)
+ // If the segment reload failed for a segment, then the value will be the previous segment reload was successful
+ public String _segmentReloadTimeUTC;
Review comment:
Why is this a String and not long?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,163 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is a helper class that calls the server API endpoints to fetch server metadata and the segment reload status
+ * Only the servers returning success are returned by the method. For servers returning errors (http error or otherwise),
+ * no entry is created in the return list
+ */
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * This method makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serversToSegmentsMap
+ * @param endpoints
+ * @param timeoutMs
+ * @return list of segments and their metadata as a JSON string
+ */
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.debug("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ JsonNode segmentMetadata = JsonUtils.stringToJsonNode(streamResponse.getValue());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
+ }
+ }
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment metadata responses from server.", failedParses);
+ }
+
+ LOGGER.debug("Retrieved segment metadata from servers.");
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return String.format("http://%s/tables/%s/segments/%s/metadata", endpoint, tableNameWithType, segmentName);
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return String.format("http://%s/tables/%s/segments/%s/loadStatus", endpoint, tableNameWithType, segmentName);
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * It makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serverToSegments
+ * @param serverToEndpoint
+ * @param timeoutMs
+ * @return list of segments along with their last reload times
+ */
+ public TableReloadStatus getSegmentReloadTime(String tableNameWithType,
+ Map<String, List<String>> serverToSegments,
+ BiMap<String, String> serverToEndpoint, int timeoutMs) {
+ LOGGER.debug("Reading segment reload status from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegmentsEntry : serverToSegments.entrySet()) {
+ List<String> segments = serverToSegmentsEntry.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateReloadStatusServerURL(tableNameWithType, segment, serverToEndpoint.get(serverToSegmentsEntry.getKey())));
+ }
+ }
+
+ BiMap<String, String> endpointsToServers = serverToEndpoint.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<SegmentStatus> segmentsStatus = new ArrayList<>();
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ SegmentStatus segmentStatus = JsonUtils.stringToObject(streamResponse.getValue(), SegmentStatus.class);
+ segmentsStatus.add(segmentStatus);
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
Review comment:
add server name
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,163 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is a helper class that calls the server API endpoints to fetch server metadata and the segment reload status
+ * Only the servers returning success are returned by the method. For servers returning errors (http error or otherwise),
+ * no entry is created in the return list
+ */
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * This method makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serversToSegmentsMap
+ * @param endpoints
+ * @param timeoutMs
+ * @return list of segments and their metadata as a JSON string
+ */
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.debug("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ JsonNode segmentMetadata = JsonUtils.stringToJsonNode(streamResponse.getValue());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
Review comment:
Add server name in the log
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/CompletionServiceHelper.java
##########
@@ -0,0 +1,103 @@
+/**
+ * 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.controller.util;
+
+import com.google.common.collect.BiMap;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is a helper class that can be used to make HttpGet (MultiGet) calls and get the responses back.
+ * The responses are returned as a string.
+ *
+ * The helper also records number of failed responses so that the caller knows if any of the calls
+ * failed to respond. The failed instance is logged for debugging.
+ */
+public class CompletionServiceHelper {
+ private static final Logger LOGGER = LoggerFactory.getLogger(CompletionServiceHelper.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _httpConnectionManager;
+ private final BiMap<String, String> _endpointsToServers;
+
+ public CompletionServiceHelper(Executor executor, HttpConnectionManager httpConnectionManager,
+ BiMap<String, String> endpointsToServers) {
+ _executor = executor;
+ _httpConnectionManager = httpConnectionManager;
+ _endpointsToServers = endpointsToServers;
+ }
+
+ public CompletionServiceResponse doMultiGetRequest(List<String> serverURLs, String tableNameWithType, int timeoutMs) {
+ CompletionServiceResponse completionServiceResponse = new CompletionServiceResponse();
+
+ // TODO: use some service other than completion service so that we know which server encounters the error
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _httpConnectionManager).execute(serverURLs, timeoutMs);
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = _endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ completionServiceResponse._failedResponseCount++;
+ continue;
+ }
+ completionServiceResponse._httpResponses.put(instance, getMethod.getResponseBodyAsString());
+ } catch (Exception e) {
+ // Ignore individual exceptions because the exception has been logged in MultiGetRequest
+ // Log the number of failed servers after gathering all responses
+ } finally {
+ if (getMethod != null) {
+ getMethod.releaseConnection();
+ }
+ }
+ }
+
+ int numServersResponded = completionServiceResponse._httpResponses.size();
+ if (numServersResponded != serverURLs.size()) {
+ LOGGER.warn("Finish reading information for table: {} with {}/{} server responses", tableNameWithType,
+ numServersResponded, serverURLs);
Review comment:
```suggestion
numServersResponded, serverURLs.size());
```
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,163 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This is a helper class that calls the server API endpoints to fetch server metadata and the segment reload status
+ * Only the servers returning success are returned by the method. For servers returning errors (http error or otherwise),
+ * no entry is created in the return list
+ */
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * This method makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serversToSegmentsMap
+ * @param endpoints
+ * @param timeoutMs
+ * @return list of segments and their metadata as a JSON string
+ */
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.debug("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ JsonNode segmentMetadata = JsonUtils.stringToJsonNode(streamResponse.getValue());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
+ }
+ }
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment metadata responses from server.", failedParses);
+ }
+
+ LOGGER.debug("Retrieved segment metadata from servers.");
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return String.format("http://%s/tables/%s/segments/%s/metadata", endpoint, tableNameWithType, segmentName);
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return String.format("http://%s/tables/%s/segments/%s/loadStatus", endpoint, tableNameWithType, segmentName);
+ }
+
+ /**
+ * This method is called when the API request is to fetch segment metadata for all segments of the table.
+ * It makes a MultiGet call to all servers that host their respective segments and gets the results.
+ * @param tableNameWithType
+ * @param serverToSegments
+ * @param serverToEndpoint
+ * @param timeoutMs
+ * @return list of segments along with their last reload times
+ */
+ public TableReloadStatus getSegmentReloadTime(String tableNameWithType,
+ Map<String, List<String>> serverToSegments,
+ BiMap<String, String> serverToEndpoint, int timeoutMs) {
+ LOGGER.debug("Reading segment reload status from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegmentsEntry : serverToSegments.entrySet()) {
+ List<String> segments = serverToSegmentsEntry.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateReloadStatusServerURL(tableNameWithType, segment, serverToEndpoint.get(serverToSegmentsEntry.getKey())));
+ }
+ }
+
+ BiMap<String, String> endpointsToServers = serverToEndpoint.inverse();
+ CompletionServiceHelper completionServiceHelper = new CompletionServiceHelper(_executor, _connectionManager, endpointsToServers);
+ CompletionServiceHelper.CompletionServiceResponse serviceResponse =
+ completionServiceHelper.doMultiGetRequest(serverURLs, tableNameWithType, timeoutMs);
+ List<SegmentStatus> segmentsStatus = new ArrayList<>();
+ int failedParses = 0;
+ for (Map.Entry<String, String> streamResponse : serviceResponse._httpResponses.entrySet()) {
+ try {
+ SegmentStatus segmentStatus = JsonUtils.stringToObject(streamResponse.getValue(), SegmentStatus.class);
+ segmentsStatus.add(segmentStatus);
+ } catch (IOException e) {
+ failedParses++;
+ LOGGER.error("Unable to parse server response due to an error: ", e);
+ }
+ }
+ if (failedParses != 0) {
+ LOGGER.warn("Failed to parse {} segment load status responses from server.", failedParses);
+ }
+
+ TableReloadStatus tableReloadStatus = new TableReloadStatus();
+ tableReloadStatus._tableName = tableNameWithType;
+ tableReloadStatus._segmentStatus = segmentsStatus;
+ tableReloadStatus._numSegmentsFailed = serviceResponse._failedResponseCount;
+ return tableReloadStatus;
+ }
+
+ /**
+ * Structure to hold the reload statsus for all segments of a given table.
Review comment:
nit: typo
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +504,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/loadStatus")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Load status of a table segment", notes = "Load status of a table segment")
+ public Map<String, ServerSegmentMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, ServerSegmentMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ ServerSegmentMetadataReader.TableReloadStatus tableReloadStatus;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ }
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private ServerSegmentMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
Review comment:
Ideally we should have timeout declared in milliseconds, so that we can configure sub-second values for fast responses. I suppose you are re-using a previously declared config here?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
Not all comments have been addressed. Please justify the use of the same class to return values to the user. It makes upgrades bad. Add json ignore case so that the pain is at least reduced a 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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460494153
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
Review comment:
That's true! Will remove the null check.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460495915
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
Review comment:
@kishoreg had mentioned that we will discuss further into the PR on this. So, we're yet to finalize on that.
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r475101993
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +504,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/loadStatus")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Load status of a table segment", notes = "Load status of a table segment")
+ public Map<String, ServerSegmentMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, ServerSegmentMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ ServerSegmentMetadataReader.TableReloadStatus tableReloadStatus;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ }
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private ServerSegmentMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
Review comment:
Yes, it was previously used in another code segment. Referred the same here 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:
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] guruguha commented on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-667438068
Initially, the thought was to have status for a single segment reload. But
then it was observed that usually, users request for a full segment reload.
So, I added the API to show reload status for all segments.
I do have a design doc
<https://docs.google.com/document/d/1E_J7PxF9WtaE6ido__u0O-Emtyu0IghQicC-1eLZeRo/edit#>.
Sure, let's discuss there. :)
-Guru
On Fri, Jul 31, 2020 at 5:25 PM Subbu Subramaniam <no...@github.com>
wrote:
> *@mcvsubbu* commented on this pull request.
> ------------------------------
>
> In
> pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
> <https://github.com/apache/incubator-pinot/pull/5718#discussion_r463895616>
> :
>
> > @@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
> throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
> }
> }
> +
> + @GET
> + @Path("segments/{tableName}/reload-status")
> + @Produces(MediaType.APPLICATION_JSON)
> + @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
>
> If you are adding one to get the status of one segment at a time, then the
> user can (if needed) iterate over the segments and get each segment. Let us
> evaluate the use case first. Are we talking about a full table reload or a
> segment reload? If full table reload, maybe we only want to return those
> segments that DID NOT reload properly?
>
> The API definition leaves much discussion to be desired, and a PR is NOT
> the place to discuss API. If you have a design doc, we will discuss there.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-pinot/pull/5718#discussion_r463895616>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACFCTWTQNNHNVC63IBMT3DDR6NOHNANCNFSM4PBWT4GA>
> .
>
----------------------------------------------------------------
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 #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460472217
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
Review comment:
Let's specify that this metadata includes metadata from server. Was this the api name that we finally settled on? It's too similar to the other "metadata" endpoint and bound to cause confusion
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ continue;
+ }
+ JsonNode segmentMetadata =
+ JsonUtils.inputStreamToJsonNode(getMethod.getResponseBodyAsStream());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ LOGGER.info("Updated segment metadata: {}", segmentMetadata.size());
+ } catch (Exception e) {
+ // Ignore individual exceptions because the exception has been logged in MultiGetRequest
+ // Log the number of failed servers after gathering all responses
+ } finally {
+ if (Objects.nonNull(getMethod)) {
+ getMethod.releaseConnection();
+ }
+ }
+ }
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/metadata";
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/reload-status";
+ }
+
+ public List<SegmentStatus> getSegmentReloadTime(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment reload status from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateReloadStatusServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ List<SegmentStatus> segmentsStatus = new ArrayList<>();
+
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
Review comment:
how about logging the message also here?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME)
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, String> segmentsMetadata = null;
+ try {
+ segmentsMetadata = getSegmentsMetadataFromServer(tableNamesWithType.get(0));
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ } catch (IOException ioe) {
+ throw new ControllerApplicationException(LOGGER,
+ "Error parsing response to cluster config!", Response.Status.BAD_REQUEST, ioe);
+ }
+ if (segmentsMetadata == null)
Review comment:
if table is not found, line 546 itself will throw exception.
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/SegmentColumnIndexesFetcher.java
##########
@@ -0,0 +1,83 @@
+/**
+ * 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.server.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.pinot.core.data.manager.SegmentDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegment;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegmentImpl;
+import org.apache.pinot.core.segment.index.column.ColumnIndexContainer;
+import org.apache.pinot.spi.utils.JsonUtils;
+
+public class SegmentColumnIndexesFetcher {
+ public static JsonNode getIndexesForSegmentColumns(SegmentDataManager segmentDataManager, Set<String> columnSet) {
+ ArrayNode columnsIndexMetadata = JsonUtils.newArrayNode();
+ if (segmentDataManager instanceof ImmutableSegmentDataManager) {
+ ImmutableSegmentDataManager immutableSegmentDataManager = (ImmutableSegmentDataManager) segmentDataManager;
+ ImmutableSegment immutableSegment = immutableSegmentDataManager.getSegment();
+ if (immutableSegment instanceof ImmutableSegmentImpl) {
+ ImmutableSegmentImpl immutableSegmentImpl = (ImmutableSegmentImpl) immutableSegment;
+// Set<String> columns = immutableSegmentImpl.getSegmentMetadata().getAllColumns();
+ Map<String, ColumnIndexContainer> columnIndexContainerMap = immutableSegmentImpl.getIndexContainerMap();
+ columnsIndexMetadata.add(getImmutableSegmentColumnIndexes(columnIndexContainerMap, columnSet));
+ }
+ }
+ return columnsIndexMetadata;
+ }
+
+ private static ObjectNode getImmutableSegmentColumnIndexes(Map<String, ColumnIndexContainer> columnIndexContainerMap,
+ Set<String> columnSet) {
+ ObjectNode columnIndexMap = JsonUtils.newObjectNode();
+
+ for (Map.Entry<String, ColumnIndexContainer> e : columnIndexContainerMap.entrySet()) {
+ if (columnSet != null && !columnSet.contains(e.getKey())) {
+ continue;
+ }
+ ColumnIndexContainer columnIndexContainer = e.getValue();
+ ObjectNode indexesNode = JsonUtils.newObjectNode();
Review comment:
nit: wrap all the if else using {}
create private static final constants for the keys and values that are being put in indexesNode.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
Review comment:
is there a better return code for this? NOT_FOUND suggests that the table was not found
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ continue;
+ }
+ JsonNode segmentMetadata =
+ JsonUtils.inputStreamToJsonNode(getMethod.getResponseBodyAsStream());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ LOGGER.info("Updated segment metadata: {}", segmentMetadata.size());
+ } catch (Exception e) {
+ // Ignore individual exceptions because the exception has been logged in MultiGetRequest
+ // Log the number of failed servers after gathering all responses
+ } finally {
+ if (Objects.nonNull(getMethod)) {
+ getMethod.releaseConnection();
+ }
+ }
+ }
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/metadata";
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/reload-status";
Review comment:
nit: How about String.format for constructing these
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
Review comment:
some validation to check if table was not found?
same for the metadata endpoint
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ continue;
+ }
+ JsonNode segmentMetadata =
+ JsonUtils.inputStreamToJsonNode(getMethod.getResponseBodyAsStream());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ LOGGER.info("Updated segment metadata: {}", segmentMetadata.size());
+ } catch (Exception e) {
+ // Ignore individual exceptions because the exception has been logged in MultiGetRequest
+ // Log the number of failed servers after gathering all responses
+ } finally {
+ if (Objects.nonNull(getMethod)) {
+ getMethod.releaseConnection();
+ }
+ }
+ }
+ return segmentsMetadata;
+ }
+
+ private String generateSegmentMetadataServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/metadata";
+ }
+
+ private String generateReloadStatusServerURL(String tableNameWithType, String segmentName, String endpoint) {
+ return "http://" + endpoint + "/tables/" + tableNameWithType + "/segments/" + segmentName + "/reload-status";
+ }
+
+ public List<SegmentStatus> getSegmentReloadTime(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
Review comment:
nit: for readability
s/serversToSegmentsMap/serverToSegments
s/endpoints/serverToEndpoint
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME)
Review comment:
nit: wrap this in {}
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
Review comment:
In the TableReloadStatus, can we also add a message telling the caller how many segments failed to report?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
Review comment:
can tableReloadStatus ever be null? you'll either catch exception above, or have a non-null reload status
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME)
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
Review comment:
you already have tableType 3 lines above
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/SegmentColumnIndexesFetcher.java
##########
@@ -0,0 +1,83 @@
+/**
+ * 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.server.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.pinot.core.data.manager.SegmentDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegment;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegmentImpl;
+import org.apache.pinot.core.segment.index.column.ColumnIndexContainer;
+import org.apache.pinot.spi.utils.JsonUtils;
+
+public class SegmentColumnIndexesFetcher {
+ public static JsonNode getIndexesForSegmentColumns(SegmentDataManager segmentDataManager, Set<String> columnSet) {
+ ArrayNode columnsIndexMetadata = JsonUtils.newArrayNode();
+ if (segmentDataManager instanceof ImmutableSegmentDataManager) {
+ ImmutableSegmentDataManager immutableSegmentDataManager = (ImmutableSegmentDataManager) segmentDataManager;
+ ImmutableSegment immutableSegment = immutableSegmentDataManager.getSegment();
+ if (immutableSegment instanceof ImmutableSegmentImpl) {
+ ImmutableSegmentImpl immutableSegmentImpl = (ImmutableSegmentImpl) immutableSegment;
+// Set<String> columns = immutableSegmentImpl.getSegmentMetadata().getAllColumns();
Review comment:
remove comment
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.controller.api.resources.ServerSegmentMetadataReader;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TableMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableMetadataReader.class);
+
+ private final Executor executor;
+ private final HttpConnectionManager connectionManager;
+ private final PinotHelixResourceManager pinotHelixResourceManager;
+
+ public TableMetadataReader(Executor executor, HttpConnectionManager connectionManager,
+ PinotHelixResourceManager helixResourceManager) {
+ this.executor = executor;
+ this.connectionManager = connectionManager;
+ this.pinotHelixResourceManager = helixResourceManager;
+ }
+
+ public TableReloadStatus getReloadStatus(String tableNameWithType, Map<String, List<String>> serverToSegmentsMap,
Review comment:
can we move call to fetch serverToSegmentsMap inside this method, so that it's consistent with the getSegmentsMetadata Method?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType, serversToSegmentsMap,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the metadata for a segment", notes = "Get the metadata for a segment")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME)
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, String> segmentsMetadata = null;
+ try {
+ segmentsMetadata = getSegmentsMetadataFromServer(tableNamesWithType.get(0));
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ } catch (IOException ioe) {
+ throw new ControllerApplicationException(LOGGER,
+ "Error parsing response to cluster config!", Response.Status.BAD_REQUEST, ioe);
+ }
+ if (segmentsMetadata == null)
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ return segmentsMetadata;
+ }
+
+ private Map<String, String> getSegmentsMetadataFromServer(String tableNameWithType)
+ throws InvalidConfigException, IOException {
+ LOGGER.trace("Inside getSegmentsMetadataFromServer() entry");
Review comment:
nit: remove trace logs?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +493,91 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, TableMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, TableMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ TableMetadataReader.TableReloadStatus tableReloadStatus = null;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER,
+ "Failed to load segment reload status for table: " + tableName, Status.NOT_FOUND);
+ }
+ if (Objects.isNull(tableReloadStatus))
+ throw new ControllerApplicationException(LOGGER,
+ "Table: " + tableName + " not found.", Status.NOT_FOUND);
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private TableMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ _pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
Review comment:
does this call return the CONSUMING (mutable) segments and do we need to skip them here?
Also how come we're supporting realtime table in reload-status, but not in metadata? I thought we're only planning to skip the CONSUMING segment in both endpoints
##########
File path: pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java
##########
@@ -44,7 +48,7 @@
@Test
public void getTables()
- throws Exception {
+ throws Exception {
Review comment:
remove all the unnecessary whitespace additions in this file
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/TableMetadataReader.java
##########
@@ -0,0 +1,89 @@
+/**
+ * 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.controller.util;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.common.collect.BiMap;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.pinot.common.exception.InvalidConfigException;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.controller.api.resources.ServerSegmentMetadataReader;
+import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TableMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(TableMetadataReader.class);
+
+ private final Executor executor;
+ private final HttpConnectionManager connectionManager;
+ private final PinotHelixResourceManager pinotHelixResourceManager;
+
+ public TableMetadataReader(Executor executor, HttpConnectionManager connectionManager,
+ PinotHelixResourceManager helixResourceManager) {
+ this.executor = executor;
+ this.connectionManager = connectionManager;
+ this.pinotHelixResourceManager = helixResourceManager;
+ }
+
+ public TableReloadStatus getReloadStatus(String tableNameWithType, Map<String, List<String>> serverToSegmentsMap,
+ int timeoutMs)
+ throws InvalidConfigException {
+ BiMap<String, String> endpoints = pinotHelixResourceManager.getDataInstanceAdminEndpoints(serverToSegmentsMap.keySet());
+ ServerSegmentMetadataReader serverSegmentMetadataReader = new ServerSegmentMetadataReader(executor, connectionManager);
+
+ List<SegmentStatus> segmentStatus = serverSegmentMetadataReader.getSegmentReloadTime(tableNameWithType, serverToSegmentsMap, endpoints, timeoutMs);
+ TableReloadStatus tableReloadStatus = new TableReloadStatus();
+ tableReloadStatus._tableName = tableNameWithType;
+ tableReloadStatus._segmentStatus = segmentStatus;
+ return tableReloadStatus;
+ }
+
+ public Map<String, String> getSegmentsMetadata(String tableNameWithType, int timeoutMs) throws InvalidConfigException, IOException {
+ final Map<String, List<String>> serversToSegmentsMap =
+ pinotHelixResourceManager.getServerToSegmentsMap(tableNameWithType);
+ BiMap<String, String> endpoints = pinotHelixResourceManager.getDataInstanceAdminEndpoints(serversToSegmentsMap.keySet());
+ ServerSegmentMetadataReader serverSegmentMetadataReader = new ServerSegmentMetadataReader(executor, connectionManager);
+
+ List<String> segmentsMetadata = serverSegmentMetadataReader.getSegmentMetadataFromServer(tableNameWithType,
+ serversToSegmentsMap, endpoints, timeoutMs);
+
+ Map<String, String> response = new HashMap<>();
+ for (String segmentMetadata : segmentsMetadata) {
+ JsonNode responseJson = JsonUtils.stringToJsonNode(segmentMetadata);
+ ObjectNode objectNode = responseJson.deepCopy();
Review comment:
didnt understand why this needs to be copied to another ObjectNode. Can we get it directly from JsonNode?
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/SegmentStatus.java
##########
@@ -0,0 +1,64 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.restlet.resources;
+
+import java.util.Objects;
+
+public class SegmentStatus {
Review comment:
How about moving this also to TableReader just like TableReloadStatus? Also, if you need the equals and hashcode, use EqualityUtils (look at Schema class for example)
##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/SegmentColumnIndexesFetcher.java
##########
@@ -0,0 +1,83 @@
+/**
+ * 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.server.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.pinot.core.data.manager.SegmentDataManager;
+import org.apache.pinot.core.data.manager.offline.ImmutableSegmentDataManager;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegment;
+import org.apache.pinot.core.indexsegment.immutable.ImmutableSegmentImpl;
+import org.apache.pinot.core.segment.index.column.ColumnIndexContainer;
+import org.apache.pinot.spi.utils.JsonUtils;
+
+public class SegmentColumnIndexesFetcher {
+ public static JsonNode getIndexesForSegmentColumns(SegmentDataManager segmentDataManager, Set<String> columnSet) {
+ ArrayNode columnsIndexMetadata = JsonUtils.newArrayNode();
+ if (segmentDataManager instanceof ImmutableSegmentDataManager) {
+ ImmutableSegmentDataManager immutableSegmentDataManager = (ImmutableSegmentDataManager) segmentDataManager;
+ ImmutableSegment immutableSegment = immutableSegmentDataManager.getSegment();
+ if (immutableSegment instanceof ImmutableSegmentImpl) {
+ ImmutableSegmentImpl immutableSegmentImpl = (ImmutableSegmentImpl) immutableSegment;
+// Set<String> columns = immutableSegmentImpl.getSegmentMetadata().getAllColumns();
+ Map<String, ColumnIndexContainer> columnIndexContainerMap = immutableSegmentImpl.getIndexContainerMap();
+ columnsIndexMetadata.add(getImmutableSegmentColumnIndexes(columnIndexContainerMap, columnSet));
+ }
+ }
+ return columnsIndexMetadata;
+ }
+
+ private static ObjectNode getImmutableSegmentColumnIndexes(Map<String, ColumnIndexContainer> columnIndexContainerMap,
+ Set<String> columnSet) {
+ ObjectNode columnIndexMap = JsonUtils.newObjectNode();
+
+ for (Map.Entry<String, ColumnIndexContainer> e : columnIndexContainerMap.entrySet()) {
+ if (columnSet != null && !columnSet.contains(e.getKey())) {
+ continue;
+ }
+ ColumnIndexContainer columnIndexContainer = e.getValue();
+ ObjectNode indexesNode = JsonUtils.newObjectNode();
+ if (Objects.isNull(columnIndexContainer.getBloomFilter())) indexesNode.put("bloom-filter", "NO");
+ else indexesNode.put("bloom-filter", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getDictionary())) indexesNode.put("dictionary", "NO");
+ else indexesNode.put("dictionary", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getForwardIndex())) indexesNode.put("forward-index", "NO");
+ else indexesNode.put("forward-index", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getInvertedIndex())) indexesNode.put("inverted-index", "NO");
+ else indexesNode.put("inverted-index", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getNullValueVector()))
+ indexesNode.put("null-value-vector-reader", "NO");
+ else indexesNode.put("null-value-vector-reader", "YES");
+
+ if (Objects.isNull(columnIndexContainer.getNullValueVector())) indexesNode.put("range-index", "NO");
+ else indexesNode.put("range-index", "YES");
+
+ columnIndexMap.set(e.getKey(), indexesNode);
+ }
+ return columnIndexMap;
Review comment:
any reason you're using ObjectNode instead of Map?
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ServerSegmentMetadataReader.java
##########
@@ -0,0 +1,139 @@
+/**
+ * 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.controller.api.resources;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.google.common.collect.BiMap;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.Executor;
+import org.apache.commons.httpclient.HttpConnectionManager;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.pinot.common.http.MultiGetRequest;
+import org.apache.pinot.common.restlet.resources.SegmentStatus;
+import org.apache.pinot.spi.utils.JsonUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ServerSegmentMetadataReader {
+ private static final Logger LOGGER = LoggerFactory.getLogger(ServerSegmentMetadataReader.class);
+
+ private final Executor _executor;
+ private final HttpConnectionManager _connectionManager;
+
+ public ServerSegmentMetadataReader(Executor executor, HttpConnectionManager connectionManager) {
+ _executor = executor;
+ _connectionManager = connectionManager;
+ }
+
+ public List<String> getSegmentMetadataFromServer(String tableNameWithType,
+ Map<String, List<String>> serversToSegmentsMap,
+ BiMap<String, String> endpoints, int timeoutMs) {
+ LOGGER.info("Reading segment metadata from servers for table {}.", tableNameWithType);
+ List<String> serverURLs = new ArrayList<>();
+ for (Map.Entry<String, List<String>> serverToSegments : serversToSegmentsMap.entrySet()) {
+ List<String> segments = serverToSegments.getValue();
+ for (String segment : segments) {
+ serverURLs.add(generateSegmentMetadataServerURL(tableNameWithType, segment, endpoints.get(serverToSegments.getKey())));
+ }
+ }
+ CompletionService<GetMethod> completionService =
+ new MultiGetRequest(_executor, _connectionManager).execute(serverURLs, timeoutMs);
+ List<String> segmentsMetadata = new ArrayList<>();
+
+ BiMap<String, String> endpointsToServers = endpoints.inverse();
+ for (int i = 0; i < serverURLs.size(); i++) {
+ GetMethod getMethod = null;
+ try {
+ getMethod = completionService.take().get();
+ URI uri = getMethod.getURI();
+ String instance = endpointsToServers.get(uri.getHost() + ":" + uri.getPort());
+ if (getMethod.getStatusCode() >= 300) {
+ LOGGER.error("Server: {} returned error: {}", instance, getMethod.getStatusCode());
+ continue;
+ }
+ JsonNode segmentMetadata =
+ JsonUtils.inputStreamToJsonNode(getMethod.getResponseBodyAsStream());
+ segmentsMetadata.add(JsonUtils.objectToString(segmentMetadata));
+ LOGGER.info("Updated segment metadata: {}", segmentMetadata.size());
Review comment:
nit: this log is misleading "Updated metadata"
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r460489469
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -128,20 +137,33 @@
@Api(tags = Constants.SEGMENT_TAG)
@Path("/")
public class PinotSegmentRestletResource {
- private static Logger LOGGER = LoggerFactory.getLogger(PinotSegmentRestletResource.class);
+ private static final Logger LOGGER = LoggerFactory.getLogger(PinotSegmentRestletResource.class);
+
+ @Inject
+ ControllerConf _controllerConf;
@Inject
PinotHelixResourceManager _pinotHelixResourceManager;
+ @Inject
+ Executor _executor;
+
+ @Inject
+ HttpConnectionManager _connectionManager;
+
+ @Inject
+ ControllerMetrics _controllerMetrics;
+
+
@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/segments/{tableName}")
@ApiOperation(value = "List all segments", notes = "List all segments")
public List<Map<TableType, List<String>>> getSegments(
- @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
- @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
Review comment:
Some setting on my IDE got changed for the Pinot checkstyle. Have updated it again. Should be fixed in my next commit.
----------------------------------------------------------------
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] guruguha commented on pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#issuecomment-681329302
@mcvsubbu can you please review 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:
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] kishoreg merged pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
kishoreg merged pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463895752
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
+ public Map<String, ServerSegmentMetadataReader.TableReloadStatus> getReloadStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ List<String> tableNamesWithType = getExistingTableNamesWithType(tableName, Constants.validateTableType(tableTypeStr));
+ Map<String, ServerSegmentMetadataReader.TableReloadStatus> reloadStatusMap = new HashMap<>();
+ for (String tableNameWithType : tableNamesWithType) {
+ ServerSegmentMetadataReader.TableReloadStatus tableReloadStatus;
+ try {
+ tableReloadStatus = getSegmentsReloadStatus(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ }
+ reloadStatusMap.put(tableNameWithType, tableReloadStatus);
+ }
+ return reloadStatusMap;
+ }
+
+ private ServerSegmentMetadataReader.TableReloadStatus getSegmentsReloadStatus(String tableNameWithType)
+ throws InvalidConfigException {
+ TableMetadataReader tableMetadataReader =
+ new TableMetadataReader(_executor, _connectionManager, _pinotHelixResourceManager);
+ return tableMetadataReader.getReloadStatus(tableNameWithType,
+ _controllerConf.getServerAdminRequestTimeoutSeconds() * 1000);
+ }
+
+ @GET
+ @Path("segments/{tableName}/metadata")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Get the server metadata for all table segments", notes = "Get the server metadata for all table segments")
+ public Map<String, String> getServerMetadata(@ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr) {
+ LOGGER.info("Received a request to fetch metadata for all segments for table {}", tableName);
+ TableType tableType = Constants.validateTableType(tableTypeStr);
+ if (tableType == TableType.REALTIME) {
+ throw new ControllerApplicationException(LOGGER,
+ "Table type : " + tableTypeStr + " not yet supported.", Status.NOT_IMPLEMENTED);
+ }
+
+ String tableNameWithType = getExistingTableNamesWithType(tableName, tableType).get(0);
+ Map<String, String> segmentsMetadata;
+ try {
+ segmentsMetadata = getSegmentsMetadataFromServer(tableNameWithType);
+ } catch (InvalidConfigException e) {
+ throw new ControllerApplicationException(LOGGER, e.getMessage(), Status.BAD_REQUEST);
+ } catch (IOException ioe) {
+ throw new ControllerApplicationException(LOGGER,
+ "Error parsing Pinot server response: " + ioe.getMessage(), Status.INTERNAL_SERVER_ERROR, ioe);
Review comment:
Logged in the helper class
----------------------------------------------------------------
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] guruguha commented on a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
guruguha commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463827691
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -176,23 +198,23 @@
@Produces(MediaType.APPLICATION_JSON)
@ApiOperation(value = "Get a map from server to segments hosted by the server (deprecated, use 'GET /segments/{tableName}/servers' instead)", notes = "Get a map from server to segments hosted by the server (deprecated, use 'GET /segments/{tableName}/servers' instead)")
public List<Map<String, String>> getServerToSegmentsMapDeprecated1(
- @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
- @ApiParam(value = "MUST be null") @QueryParam("state") String stateStr,
- @ApiParam(value = "OFFLINE|REALTIME") @QueryParam("type") String tableTypeStr)
- throws JsonProcessingException {
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
Review comment:
Yes, there was some issue with my IDE itself. I updated the coding guidelines again and reformatted the files.
----------------------------------------------------------------
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 a change in pull request #5718: Feature/#5390 segment indexing reload status api
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #5718:
URL: https://github.com/apache/incubator-pinot/pull/5718#discussion_r463895282
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSegmentRestletResource.java
##########
@@ -485,4 +503,80 @@ private void deleteSegmentsInternal(String tableNameWithType, List<String> segme
throw new ControllerApplicationException(LOGGER, e.getMessage(), Response.Status.FORBIDDEN);
}
}
+
+ @GET
+ @Path("segments/{tableName}/reload-status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "Status of segment reload", notes = "Status of segment reload")
Review comment:
Add an API to get status for a range of segments, maybe? Or, add some sort of start/limit?
----------------------------------------------------------------
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