You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/05/07 04:03:38 UTC
[GitHub] [incubator-pinot] icefury71 opened a new pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
icefury71 opened a new pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890
… table
## Description
This is part 1 of #6524 . This PR provides an overall ingestion status for Pinot realtime table. The status is HEALTHY if all consuming segments are in the CONSUMING state. UNHEALTHY otherwise.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade?
No
Does this PR fix a zero-downtime upgrade introduced earlier?
No
Does this PR otherwise need attention when creating release notes?
No
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r634914658
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
+1 to Chinmay's observation. Hence when transient errors will make a segment OFFLINE in IS, it will be ERROR state in EV, and hence NOT_CONSUMING, resulting in UNHEALTHY.
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r631453814
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +135,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(RealtimeSegmentDataManager.ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
The controller should not depend on the server. RealtimeSegmentDataManager belongs in pinot-sever right? If it is in core now, it should probably be moved to pinot-server anyway.
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r629032580
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,49 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ serversForSegment.removeAll(consumingSegmentInfoList);
Review comment:
Aah ... I'm not sure what I was doing here. Thanks for pointing out ! Will fix
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-834062294
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6890](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5f54bba) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6c43af70c76c14c07eef26fb97c9759e3f7b6f9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c43af7) will **decrease** coverage by `0.06%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6890 +/- ##
============================================
- Coverage 73.53% 73.46% -0.07%
Complexity 12 12
============================================
Files 1420 1431 +11
Lines 69853 70591 +738
Branches 10083 10204 +121
============================================
+ Hits 51366 51863 +497
- Misses 15088 15295 +207
- Partials 3399 3433 +34
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `43.19% <0.00%> (-0.21%)` | `7.00 <0.00> (ø)` | |
| unittests | `65.40% <0.00%> (-0.12%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `56.41% <0.00%> (-3.05%)` | `0.00 <0.00> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.47% <0.00%> (-0.20%)` | `0.00 <0.00> (ø)` | |
| [...ot/controller/util/ConsumingSegmentInfoReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbnN1bWluZ1NlZ21lbnRJbmZvUmVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...manager/realtime/HLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvSExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `83.10% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `72.96% <ø> (-0.09%)` | `0.00 <0.00> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `100.00% <ø> (+71.42%)` | `0.00 <0.00> (ø)` | |
| [...org/apache/pinot/spi/config/table/TableStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `29.82% <0.00%> (-1.66%)` | `0.00 <0.00> (ø)` | |
| [...readers/constant/ConstantMVForwardIndexReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvY29uc3RhbnQvQ29uc3RhbnRNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `14.28% <0.00%> (-28.58%)` | `0.00% <0.00%> (ø%)` | |
| [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | `0.00% <0.00%> (ø%)` | |
| ... and [95 more](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6c43af7...5f54bba](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-834062294
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6890](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f7c56d6) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6c43af70c76c14c07eef26fb97c9759e3f7b6f9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c43af7) will **decrease** coverage by `8.06%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6890 +/- ##
============================================
- Coverage 73.53% 65.46% -8.07%
Complexity 12 12
============================================
Files 1420 1424 +4
Lines 69853 70121 +268
Branches 10083 10127 +44
============================================
- Hits 51366 45905 -5461
- Misses 15088 20934 +5846
+ Partials 3399 3282 -117
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `?` | `?` | |
| unittests | `65.46% <0.00%> (-0.06%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `54.70% <0.00%> (-4.76%)` | `0.00 <0.00> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `60.03% <0.00%> (-2.64%)` | `0.00 <0.00> (ø)` | |
| [...ot/controller/util/ConsumingSegmentInfoReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbnN1bWluZ1NlZ21lbnRJbmZvUmVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...org/apache/pinot/spi/config/table/TableStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| ... and [362 more](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6c43af7...f7c56d6](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r633866369
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
No that should be fine. the NOT_CONSUMING state is set when the LLRealtimeSegmentDataManager.State is set to ERROR.
--
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] icefury71 commented on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-836127372
> Are you deprecating this PR?
> #6322
> That makes it the second deprecation of consuming segments info API
No I'm not deprecating the consuming segments info. In fact, this diff depends on the underlying functionality
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-843629007
> @icefury71 @npawar
>
> To avoid creating multiple debug api's on servers, I propose to consolidate the one I am creating for segments as follows:
>
> 1. The api consumingSegmentsInfo will be deprecated.
> 2. We will create a new api called segmentStatusInfo (or another name such as segmentDebugInfo) which will have:
>
> * Consumption info from 1.
> * Errors/exceptions related to segment
> * Any other info we want to add in future.
> 3. On the controller side, these can all roll up inside the existing table debug endpoint, if we want to further consolidate.
>
> We can certainly do it in steps. Please let me know your thoughts.
Fine by me.
Here is something that we should give a thought to. It may not look pretty in code, but will help us avoid backward incompatibility during upgrades, etc. I suggest that we don't re-use the objects that are returned by server as controller return to the user. The advantage of using a different object to return to the user is that we can evolve the server return object (purely internal to Pinot) independent of the other one (exposed to the user). If we re-use the same one, then changing one will affect the other, and we may be forced into some strange upgrade sequence (on a per-release basis) to keep compatibility -- if we manage to even detect the compatibility.
--
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] icefury71 commented on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-843606952
> @icefury71 @npawar
>
> To avoid creating multiple debug api's on servers, I propose to consolidate the one I am creating for segments as follows:
>
> 1. The api consumingSegmentsInfo will be deprecated.
> 2. We will create a new api called segmentStatusInfo (or another name such as segmentDebugInfo) which will have:
>
> * Consumption info from 1.
> * Errors/exceptions related to segment
> * Any other info we want to add in future.
> 3. On the controller side, these can all roll up inside the existing table debug endpoint, if we want to further consolidate.
>
> We can certainly do it in steps. Please let me know your thoughts.
Sounds good +1 on this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-834062294
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6890](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f7c56d6) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6c43af70c76c14c07eef26fb97c9759e3f7b6f9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c43af7) will **decrease** coverage by `0.03%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6890 +/- ##
============================================
- Coverage 73.53% 73.50% -0.04%
Complexity 12 12
============================================
Files 1420 1424 +4
Lines 69853 70121 +268
Branches 10083 10127 +44
============================================
+ Hits 51366 51541 +175
- Misses 15088 15164 +76
- Partials 3399 3416 +17
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `43.24% <0.00%> (-0.16%)` | `7.00 <0.00> (ø)` | |
| unittests | `65.46% <0.00%> (-0.06%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `56.41% <0.00%> (-3.05%)` | `0.00 <0.00> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.47% <0.00%> (-0.20%)` | `0.00 <0.00> (ø)` | |
| [...ot/controller/util/ConsumingSegmentInfoReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbnN1bWluZ1NlZ21lbnRJbmZvUmVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...org/apache/pinot/spi/config/table/TableStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | `0.00% <0.00%> (ø%)` | |
| [...mpl/dictionary/DoubleOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT2ZmSGVhcE11dGFibGVEaWN0aW9uYXJ5LmphdmE=) | `51.06% <0.00%> (-8.52%)` | `0.00% <0.00%> (ø%)` | |
| [...inot/spi/stream/PartitionGroupMetadataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1BhcnRpdGlvbkdyb3VwTWV0YWRhdGFGZXRjaGVyLmphdmE=) | `56.00% <0.00%> (-6.07%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `72.34% <0.00%> (-5.32%)` | `0.00% <0.00%> (ø%)` | |
| [...cal/startree/v2/builder/BaseSingleTreeBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS92Mi9idWlsZGVyL0Jhc2VTaW5nbGVUcmVlQnVpbGRlci5qYXZh) | `88.39% <0.00%> (-4.47%)` | `0.00% <0.00%> (ø%)` | |
| [...ot/segment/local/startree/OffHeapStarTreeNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zdGFydHJlZS9PZmZIZWFwU3RhclRyZWVOb2RlLmphdmE=) | `81.48% <0.00%> (-3.71%)` | `0.00% <0.00%> (ø%)` | |
| ... and [56 more](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6c43af7...f7c56d6](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv edited a comment on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mayankshriv edited a comment on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-843582088
@icefury71 @npawar
To avoid creating multiple debug api's on servers, I propose to consolidate the one I am creating for segments as follows:
1. The api consumingSegmentsInfo will be deprecated.
2. We will create a new api called segmentStatusInfo (or another name such as segmentDebugInfo) which will have:
- Consumption info from 1.
- Errors/exceptions related to segment
- Any other info we want to add in future.
3. On the controller side, these can all roll up inside the existing table debug endpoint, if we want to further consolidate.
We can certainly do it in steps. Please let me know your thoughts.
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r633928656
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
We now include the freshness metric as well (as in how old is the data).
Linkedin Kafka drivers stopped exporting the per-partition lag metrics.
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r631390720
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -586,4 +596,31 @@ private void checkHybridTableConfig(String rawTableName, TableConfig tableConfig
}
}
}
+
+ @GET
+ @Path("/tables/{tableName}/status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "table status", notes = "Provides status of the table including ingestion status")
+ public String getTableStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ try {
+ TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableName);
+ if (TableType.OFFLINE == tableType) {
+ // TODO: Support table status for offline table. Currently only supported for realtime.
+ throw new IllegalStateException("Table status for OFFLINE table: " + tableName + " is currently unsupported");
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
npawar commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r635426521
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
regardless, the consumer will be in error state, and that is what the underlying server API is looking for
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mcvsubbu commented on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-844273786
> > > @icefury71 @npawar
> > > To avoid creating multiple debug api's on servers, I propose to consolidate the one I am creating for segments as follows:
> > >
> > > 1. The api consumingSegmentsInfo will be deprecated.
> > > 2. We will create a new api called segmentStatusInfo (or another name such as segmentDebugInfo) which will have:
> > >
> > > * Consumption info from 1.
> > > * Errors/exceptions related to segment
> > > * Any other info we want to add in future.
> > > 3. On the controller side, these can all roll up inside the existing table debug endpoint, if we want to further consolidate.
> > >
> > > We can certainly do it in steps. Please let me know your thoughts.
> >
> >
> > Fine by me.
> > Here is something that we should give a thought to. It may not look pretty in code, but will help us avoid backward incompatibility during upgrades, etc. I suggest that we don't re-use the objects that are returned by server as controller return to the user. The advantage of using a different object to return to the user is that we can evolve the server return object (purely internal to Pinot) independent of the other one (exposed to the user). If we re-use the same one, then changing one will affect the other, and we may be forced into some strange upgrade sequence (on a per-release basis) to keep compatibility -- if we manage to even detect the compatibility.
>
> @mcvsubbu we're not reusing server objects in the response. The controller parses the individual server responses and builds a custom object which is returned.
>
> Lemme know if you have any outstanding concerns on this particular PR.
This PR is fine. My comment was in response to Mayan's comment about migrating towards a new set of APIs. As an example, PR #6322 reuses the return object from server to controller in the controller response.
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r633914256
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
This means that if for some reason we have a transient failure with Kafka that results in CONSUMING segments going OFFLINE, we could be displaying HEALTHY for a long time.
We have had multiple combination of such issues at Linkedin, and over time, have arrived at the conclusion that monitoring the consumption metric (we emit 1 when consuming and 0 when not), is really the best alternative to decide whether or not some manual intervention is needed. A persistent value of 0 (where the time of persistence depends on application tolerance to non-fresh data) indicates call to action.
We can also check with how Uber handles this, since they are another large installation of real-time Pinot.
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r633886095
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/table/TableStatus.java
##########
@@ -0,0 +1,68 @@
+/**
+ * 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.spi.config.table;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+
+/**
+ * Container object to encapsulate table status which contains
+ * - Ingestion status: specifies if the table is ingesting properly or not
+ */
+public class TableStatus {
+
+ private IngestionStatus _ingestionStatus;
+
+ public enum IngestionState {
+ HEALTHY,
+ UNHEALTHY
+ }
+
+ public static class IngestionStatus {
Review comment:
Added. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-834062294
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6890](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8a5e91e) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6c43af70c76c14c07eef26fb97c9759e3f7b6f9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c43af7) will **decrease** coverage by `0.01%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6890 +/- ##
============================================
- Coverage 73.53% 73.51% -0.02%
Complexity 12 12
============================================
Files 1420 1424 +4
Lines 69853 70084 +231
Branches 10083 10126 +43
============================================
+ Hits 51366 51522 +156
- Misses 15088 15148 +60
- Partials 3399 3414 +15
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `43.31% <0.00%> (-0.09%)` | `7.00 <0.00> (ø)` | |
| unittests | `65.48% <0.00%> (-0.04%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `56.41% <0.00%> (-3.05%)` | `0.00 <0.00> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.47% <0.00%> (-0.20%)` | `0.00 <0.00> (ø)` | |
| [...ot/controller/util/ConsumingSegmentInfoReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbnN1bWluZ1NlZ21lbnRJbmZvUmVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...org/apache/pinot/spi/config/table/TableStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/core/transport/DataTableHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS90cmFuc3BvcnQvRGF0YVRhYmxlSGFuZGxlci5qYXZh) | `88.00% <0.00%> (-12.00%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/DoubleOnHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRG91YmxlT25IZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `46.98% <0.00%> (-7.23%)` | `0.00% <0.00%> (ø%)` | |
| [...inot/spi/stream/PartitionGroupMetadataFetcher.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvc3RyZWFtL1BhcnRpdGlvbkdyb3VwTWV0YWRhdGFGZXRjaGVyLmphdmE=) | `56.00% <0.00%> (-6.07%)` | `0.00% <0.00%> (ø%)` | |
| [...impl/dictionary/FloatOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvRmxvYXRPZmZIZWFwTXV0YWJsZURpY3Rpb25hcnkuamF2YQ==) | `72.34% <0.00%> (-5.32%)` | `0.00% <0.00%> (ø%)` | |
| [...operator/filter/RangeIndexBasedFilterOperator.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvUmFuZ2VJbmRleEJhc2VkRmlsdGVyT3BlcmF0b3IuamF2YQ==) | `40.42% <0.00%> (-4.26%)` | `0.00% <0.00%> (ø%)` | |
| ... and [44 more](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6c43af7...8a5e91e](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r632197571
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/table/TableStatus.java
##########
@@ -0,0 +1,68 @@
+/**
+ * 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.spi.config.table;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+
+
+/**
+ * Container object to encapsulate table status which contains
+ * - Ingestion status: specifies if the table is ingesting properly or not
+ */
+public class TableStatus {
+
+ private IngestionStatus _ingestionStatus;
+
+ public enum IngestionState {
+ HEALTHY,
+ UNHEALTHY
+ }
+
+ public static class IngestionStatus {
Review comment:
Always good to add `@JsonIgnoreProperties(ignoreUnknown = true)`
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
We stop consumption during segment generation. Will that be treated as error?
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r631549910
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +135,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
Review comment:
Documenting our discussion here: this is a convenient API to find out if the table (realtime or offline) is ingesting correctly or not. Detailed stack traces/debug info is handled in another PR (by Mayank).
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r633920187
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
From my read, it seemed like whenever ServerGauge.LLC_PARTITION_CONSUMING is set to 0 under error conditions (except for when consumption is complete or we're going from consuming to ONLINE), the
LLRealtimeSegmentDataManager.State is set to ERROR. So it should work for any such erroneous conditions.
If that's not the case, then we can treat it as part of a bug fix for #6322
In Uber, AFAIK we rely on the ingestion time lag (from Kafka) to determine problems in ingestions.
--
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] icefury71 merged pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 merged pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890
--
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] icefury71 commented on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-843739023
> > @icefury71 @npawar
> > To avoid creating multiple debug api's on servers, I propose to consolidate the one I am creating for segments as follows:
> >
> > 1. The api consumingSegmentsInfo will be deprecated.
> > 2. We will create a new api called segmentStatusInfo (or another name such as segmentDebugInfo) which will have:
> >
> > * Consumption info from 1.
> > * Errors/exceptions related to segment
> > * Any other info we want to add in future.
> > 3. On the controller side, these can all roll up inside the existing table debug endpoint, if we want to further consolidate.
> >
> > We can certainly do it in steps. Please let me know your thoughts.
>
> Fine by me.
>
> Here is something that we should give a thought to. It may not look pretty in code, but will help us avoid backward incompatibility during upgrades, etc. I suggest that we don't re-use the objects that are returned by server as controller return to the user. The advantage of using a different object to return to the user is that we can evolve the server return object (purely internal to Pinot) independent of the other one (exposed to the user). If we re-use the same one, then changing one will affect the other, and we may be forced into some strange upgrade sequence (on a per-release basis) to keep compatibility -- if we manage to even detect the compatibility.
@mcvsubbu we're not reusing server objects in the response. The controller parses the individual server responses and builds a custom object which is returned.
Lemme know if you have any outstanding concerns on this particular PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-834062294
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6890](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3758474) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6c43af70c76c14c07eef26fb97c9759e3f7b6f9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c43af7) will **decrease** coverage by `8.12%`.
> The diff coverage is `62.07%`.
> :exclamation: Current head 3758474 differs from pull request most recent head ed6a6f3. Consider uploading reports for the commit ed6a6f3 to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6890 +/- ##
============================================
- Coverage 73.53% 65.41% -8.13%
Complexity 12 12
============================================
Files 1420 1432 +12
Lines 69853 70678 +825
Branches 10083 10227 +144
============================================
- Hits 51366 46231 -5135
- Misses 15088 21137 +6049
+ Partials 3399 3310 -89
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `?` | `?` | |
| unittests | `65.41% <62.07%> (-0.11%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...org/apache/pinot/broker/api/RequestStatistics.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL1JlcXVlc3RTdGF0aXN0aWNzLmphdmE=) | `41.66% <0.00%> (-23.49%)` | `0.00 <0.00> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...ller/api/resources/PinotBrokerRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90QnJva2VyUmVzdGxldFJlc291cmNlLmphdmE=) | `60.24% <0.00%> (-0.74%)` | `0.00 <0.00> (ø)` | |
| [...ler/api/resources/PinotSegmentRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90U2VnbWVudFJlc3RsZXRSZXNvdXJjZS5qYXZh) | `3.17% <0.00%> (-15.25%)` | `0.00 <0.00> (ø)` | |
| [...ler/api/resources/TableConfigsRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlQ29uZmlnc1Jlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `60.03% <0.00%> (-2.64%)` | `0.00 <0.00> (ø)` | |
| [...roller/helix/core/PinotTableIdealStateBuilder.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90VGFibGVJZGVhbFN0YXRlQnVpbGRlci5qYXZh) | `79.74% <ø> (ø)` | `0.00 <0.00> (ø)` | |
| [...not/controller/recommender/rules/AbstractRule.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9BYnN0cmFjdFJ1bGUuamF2YQ==) | `80.00% <0.00%> (-20.00%)` | `0.00 <0.00> (ø)` | |
| [...commender/rules/io/params/PartitionRuleParams.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9yZWNvbW1lbmRlci9ydWxlcy9pby9wYXJhbXMvUGFydGl0aW9uUnVsZVBhcmFtcy5qYXZh) | `40.90% <ø> (+2.44%)` | `0.00 <0.00> (ø)` | |
| ... and [469 more](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6c43af7...ed6a6f3](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-834062294
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6890](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8a5e91e) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6c43af70c76c14c07eef26fb97c9759e3f7b6f9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c43af7) will **decrease** coverage by `8.05%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6890 +/- ##
============================================
- Coverage 73.53% 65.48% -8.06%
Complexity 12 12
============================================
Files 1420 1424 +4
Lines 69853 70084 +231
Branches 10083 10126 +43
============================================
- Hits 51366 45892 -5474
- Misses 15088 20915 +5827
+ Partials 3399 3277 -122
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `?` | `?` | |
| unittests | `65.48% <0.00%> (-0.04%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `54.70% <0.00%> (-4.76%)` | `0.00 <0.00> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `60.03% <0.00%> (-2.64%)` | `0.00 <0.00> (ø)` | |
| [...ot/controller/util/ConsumingSegmentInfoReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbnN1bWluZ1NlZ21lbnRJbmZvUmVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...org/apache/pinot/spi/config/table/TableStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/minion/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pache/pinot/common/utils/grpc/GrpcQueryClient.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvZ3JwYy9HcnBjUXVlcnlDbGllbnQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...pinot/minion/exception/TaskCancelledException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vZXhjZXB0aW9uL1Rhc2tDYW5jZWxsZWRFeGNlcHRpb24uamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [...t/core/startree/plan/StarTreeDocIdSetPlanNode.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9zdGFydHJlZS9wbGFuL1N0YXJUcmVlRG9jSWRTZXRQbGFuTm9kZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| ... and [356 more](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6c43af7...8a5e91e](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r628548807
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2117,6 +2117,17 @@ public void toggleQueryQuotaStateForBroker(String brokerInstanceName, String sta
return consumingSegments;
}
+ /**
+ * Utility function to return set of servers corresponding to a given segment.
+ */
+ public Set<String> getServersForSegment(String tableNameWithType, String segmentName) {
+ IdealState idealState = _helixAdmin.getResourceIdealState(_helixClusterName, tableNameWithType);
Review comment:
get the externalview and skip segments that are offline or error states
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,49 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ serversForSegment.removeAll(consumingSegmentInfoList);
Review comment:
Wait. consumingSegmentInfoList is List<ConsumingSegmentInfo> and serversForSegment is Set<String> with server names.
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -586,4 +596,31 @@ private void checkHybridTableConfig(String rawTableName, TableConfig tableConfig
}
}
}
+
+ @GET
+ @Path("/tables/{tableName}/status")
+ @Produces(MediaType.APPLICATION_JSON)
+ @ApiOperation(value = "table status", notes = "Provides status of the table including ingestion status")
+ public String getTableStatus(
+ @ApiParam(value = "Name of the table", required = true) @PathParam("tableName") String tableName,
+ @ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ try {
+ TableType tableType = TableNameBuilder.getTableTypeFromTableName(tableName);
+ if (TableType.OFFLINE == tableType) {
+ // TODO: Support table status for offline table. Currently only supported for realtime.
+ throw new IllegalStateException("Table status for OFFLINE table: " + tableName + " is currently unsupported");
Review comment:
Either UnsuportedOperation or Unimplemented exception.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-834062294
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#6890](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5f54bba) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/6c43af70c76c14c07eef26fb97c9759e3f7b6f9e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c43af7) will **decrease** coverage by `8.13%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #6890 +/- ##
============================================
- Coverage 73.53% 65.40% -8.14%
Complexity 12 12
============================================
Files 1420 1431 +11
Lines 69853 70591 +738
Branches 10083 10204 +121
============================================
- Hits 51366 46167 -5199
- Misses 15088 21123 +6035
+ Partials 3399 3301 -98
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| integration | `?` | `?` | |
| unittests | `65.40% <0.00%> (-0.12%)` | `12.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `54.70% <0.00%> (-4.76%)` | `0.00 <0.00> (ø)` | |
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `60.03% <0.00%> (-2.64%)` | `0.00 <0.00> (ø)` | |
| [...ot/controller/util/ConsumingSegmentInfoReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0NvbnN1bWluZ1NlZ21lbnRJbmZvUmVhZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (ø)` | |
| [...manager/realtime/HLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvSExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `0.00% <ø> (-83.11%)` | `0.00 <0.00> (ø)` | |
| [...manager/realtime/LLRealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvTExSZWFsdGltZVNlZ21lbnREYXRhTWFuYWdlci5qYXZh) | `47.49% <ø> (-25.55%)` | `0.00 <0.00> (ø)` | |
| [...a/manager/realtime/RealtimeSegmentDataManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvUmVhbHRpbWVTZWdtZW50RGF0YU1hbmFnZXIuamF2YQ==) | `50.00% <ø> (+21.42%)` | `0.00 <0.00> (ø)` | |
| [...org/apache/pinot/spi/config/table/TableStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00 <0.00> (?)` | |
| [...va/org/apache/pinot/spi/utils/CommonConstants.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQ29tbW9uQ29uc3RhbnRzLmphdmE=) | `29.82% <0.00%> (-1.66%)` | `0.00 <0.00> (ø)` | |
| [...a/org/apache/pinot/minion/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtbWluaW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9taW5pb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| [.../apache/pinot/common/metrics/BrokerQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9Ccm9rZXJRdWVyeVBoYXNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (ø%)` | |
| ... and [394 more](https://codecov.io/gh/apache/incubator-pinot/pull/6890/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6c43af7...5f54bba](https://codecov.io/gh/apache/incubator-pinot/pull/6890?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [incubator-pinot] mayankshriv commented on pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mayankshriv commented on pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#issuecomment-843582088
@icefury71 @npawar
To avoid creating multiple debug api's on servers, I propose to consolidate the one I am creating for segments as follows:
1. The api consumingSegmentsInfo will be deprecated.
2. We will create a new api called segmentStatusInfo (or another name such as segmentDebugInfo) which will have:
- Consumption info from 1.
- Errors/exceptions related to segment
- Any other info we want to add in future.
3. On the controller side, these can all roll up inside the existing table debug endpoint, if we want to further consolidate.
Please let me know your thoughts.
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r631448893
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +135,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
Review comment:
Looks like we return the first time we find a bad consumer. Don't we want to provide the congestion status of each partition/segment?
Let us say the user finds out that the status is unhealthy (which, btw, they can also find out using metrics). What would they do?
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r631390830
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,49 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ serversForSegment.removeAll(consumingSegmentInfoList);
Review comment:
Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r631549329
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +135,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(RealtimeSegmentDataManager.ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
Good point. Refactored ConsumerState to be in CommonConstants instead.
--
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] icefury71 commented on a change in pull request #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
icefury71 commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r629032169
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2117,6 +2117,17 @@ public void toggleQueryQuotaStateForBroker(String brokerInstanceName, String sta
return consumingSegments;
}
+ /**
+ * Utility function to return set of servers corresponding to a given segment.
+ */
+ public Set<String> getServersForSegment(String tableNameWithType, String segmentName) {
+ IdealState idealState = _helixAdmin.getResourceIdealState(_helixClusterName, tableNameWithType);
Review comment:
The idea here is to get all the servers including those corresponding to segments in ERROR state. This is used to identify if there are any missing entries from the consuming segments API response.
--
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 #6890: Adding a new Controller API to retrieve ingestion status for realtime…
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6890:
URL: https://github.com/apache/incubator-pinot/pull/6890#discussion_r635404113
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/util/ConsumingSegmentInfoReader.java
##########
@@ -131,6 +134,51 @@ private String generateServerURL(String tableNameWithType, String endpoint) {
return String.format("%s/tables/%s/consumingSegmentsInfo", endpoint, tableNameWithType);
}
+ /**
+ * Utility method to derive ingestion status from consuming segment Info. Status is HEALTHY if
+ * consuming segment info specifies CONSUMING state for all active segments across all servers
+ * including replicas.
+ */
+ public TableStatus.IngestionStatus getIngestionStatus(String tableNameWithType, int timeoutMs) {
+ try {
+ ConsumingSegmentsInfoMap consumingSegmentsInfoMap = getConsumingSegmentsInfo(tableNameWithType, timeoutMs);
+ for (Map.Entry<String, List<ConsumingSegmentInfo>> consumingSegmentInfoEntry : consumingSegmentsInfoMap._segmentToConsumingInfoMap
+ .entrySet()) {
+ String segmentName = consumingSegmentInfoEntry.getKey();
+ List<ConsumingSegmentInfo> consumingSegmentInfoList = consumingSegmentInfoEntry.getValue();
+ if (consumingSegmentInfoList == null || consumingSegmentInfoList.isEmpty()) {
+ String errorMessage = "Did not get any response from servers for segment: " + segmentName;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ // Check if any responses are missing
+ Set<String> serversForSegment = _pinotHelixResourceManager.getServersForSegment(tableNameWithType, segmentName);
+ if (serversForSegment.size() != consumingSegmentInfoList.size()) {
+ Set<String> serversResponded =
+ consumingSegmentInfoList.stream().map(c -> c._serverName).collect(Collectors.toSet());
+ serversForSegment.removeAll(serversResponded);
+ String errorMessage =
+ "Not all servers responded for segment: " + segmentName + " Missing servers : " + serversForSegment;
+ return TableStatus.IngestionStatus.newIngestionStatus(TableStatus.IngestionState.UNHEALTHY, errorMessage);
+ }
+
+ for (ConsumingSegmentInfo consumingSegmentInfo : consumingSegmentInfoList) {
+ if (consumingSegmentInfo._consumerState
+ .equals(ConsumerState.NOT_CONSUMING.toString())) {
Review comment:
@npawar , @icefury71 when we change the state to OFFLINE in IS, EV will also be OFFLINE (not ERROR).
--
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