You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "deepthi912 (via GitHub)" <gi...@apache.org> on 2025/06/20 23:50:55 UTC
[PR] Add server status and segments status along in the validDocIds info API - Upsert Tables [pinot]
deepthi912 opened a new pull request, #16165:
URL: https://github.com/apache/pinot/pull/16165
**Context:** When server restarts and segments are in UPDATING state or when the segments are in BAD state, there will be inconsistency in the validDocIds metadata and showing the segment and server status along with the validDocIds of each segment will improve the observability and so we can eliminate those segments in BAD state or ignore the partition when some of the segments are UPDATING.
This will be especially helpful when we compact or merge segments in Upsert Tables.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2162586619
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -525,7 +532,27 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap(
String.format("Table %s segment %s is not a immutable segment", tableNameWithType, segmentName),
Response.Status.BAD_REQUEST);
}
-
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ String serverStatus = "";
+ if (status.equals(ServiceStatus.Status.GOOD)) {
+ _serverMetrics.addMeteredGlobalValue(ServerMeter.READINESS_CHECK_OK_CALLS, 1);
+ serverStatus = "OK";
+ } else {
+ serverStatus = "NOT_READY";
+ }
+ HelixAdmin helixAdmin = _serverInstance.getHelixManager().getClusterManagmentTool();
+ String helixClusterName = _serverInstance.getHelixManager().getClusterName();
+
+ TableViewsUtils.TableView externalView =
+ TableViewsUtils.getTableState(tableNameWithType, TableViewsUtils.EXTERNALVIEW, TableType.REALTIME, helixAdmin,
+ helixClusterName);
+ TableViewsUtils.TableView idealStateView =
+ TableViewsUtils.getTableState(tableNameWithType, TableViewsUtils.IDEALSTATE, TableType.REALTIME, helixAdmin,
+ helixClusterName);
+
Review Comment:
Updated and removed the segment status, in executor we are checking for IDEAL STATE previously, changed to check EXTERNAL VIEW in generator logic as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163020448
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
Yes, this is needed for checking status of each segment here in external view rather than in Ideal state as some segments will be in BAD state. Server status is still needed because some of the UPDATING segments can change the bit map of ONLINE segments, so it is better to wait until server is in READY state.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163123362
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
There could be some segments stuck in BAD state even when server is READY, we would need to avoid those segments, which is why checking external view is important
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163044354
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskExecutor.java:
##########
@@ -96,6 +102,14 @@ protected SegmentConversionResult convert(PinotTaskConfig pinotTaskConfig, File
.build();
}
+ if (!validDocIdsBitmapResponse.getServerStatus().equals("OK")) {
Review Comment:
We can do that on MinionTaskUtils and generation/execution as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2165056602
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,44 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getServerToOnlineSegmentsMapFromEV(String tableNameWithType,
Review Comment:
We may extract a common method for getServerToOnlineSegmentsMapFromEV() and getServerToSegmentsMap() above, as the major difference is where to get segment's status, i.e. IS vs. EV.
(oops, found this just got merged. If this makes sense ^, can be done in separate 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2164882227
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/MinionTaskUtils.java:
##########
@@ -236,6 +237,17 @@ public static RoaringBitmap getValidDocIdFromServerMatchingCrc(String tableNameW
LOGGER.warn(message);
continue;
}
+
+ // skipping servers which are not in READY state. The bitmaps would be inconsistent when
+ // server is NOT READY as UPDATING segments might be updating the ONLINE segments
+ if (!validDocIdsBitmapResponse.getServerStatus().equals(ServiceStatus.Status.GOOD)) {
Review Comment:
(MAJOR) This can cause `NPE` during upgrade. Check if it is not null, then perform this check. Same for other places
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16165:
URL: https://github.com/apache/pinot/pull/16165#issuecomment-2993786502
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/16165?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `58.98876%` with `73 lines` in your changes missing coverage. Please review.
> Project coverage is 33.34%. Comparing base [(`1a476de`)](https://app.codecov.io/gh/apache/pinot/commit/1a476dea72ebf1c15e14ded28ae60c385f71b2c3?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`454d475`)](https://app.codecov.io/gh/apache/pinot/commit/454d4752bba1523ace4d257c49d34fc4bb089abb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> Report is 311 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/16165?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...che/pinot/controller/api/resources/TableViews.java](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&filepath=pinot-controller%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcontroller%2Fapi%2Fresources%2FTableViews.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlVmlld3MuamF2YQ==) | 42.37% | [34 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...che/pinot/common/utils/tables/TableViewsUtils.java](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Futils%2Ftables%2FTableViewsUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvdGFibGVzL1RhYmxlVmlld3NVdGlscy5qYXZh) | 71.11% | [18 Missing and 8 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...mon/restlet/resources/ValidDocIdsMetadataInfo.java](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&filepath=pinot-common%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcommon%2Frestlet%2Fresources%2FValidDocIdsMetadataInfo.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcmVzdGxldC9yZXNvdXJjZXMvVmFsaWREb2NJZHNNZXRhZGF0YUluZm8uamF2YQ==) | 0.00% | [7 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...che/pinot/server/api/resources/TablesResource.java](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&filepath=pinot-server%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fserver%2Fapi%2Fresources%2FTablesResource.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zZXJ2ZXIvYXBpL3Jlc291cmNlcy9UYWJsZXNSZXNvdXJjZS5qYXZh) | 72.72% | [5 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/16165?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (1a476de) and HEAD (454d475). Click for more details.
>
> <details><summary>HEAD has 68 uploads less than BASE</summary>
>
>| Flag | BASE (1a476de) | HEAD (454d475) |
>|------|------|------|
>|java-21|10|1|
>|unittests|6|1|
>|unittests1|3|0|
>|skip-bytebuffers-true|4|0|
>|temurin|15|1|
>|unittests2|3|1|
>|skip-bytebuffers-false|8|0|
>|java-11|5|0|
>|integration|9|0|
>|integration2|3|0|
>|integration1|3|0|
>|custom-integration1|3|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #16165 +/- ##
=============================================
- Coverage 62.90% 33.34% -29.56%
+ Complexity 1386 659 -727
=============================================
Files 2867 2954 +87
Lines 163354 169990 +6636
Branches 24952 25988 +1036
=============================================
- Hits 102755 56682 -46073
- Misses 52847 108758 +55911
+ Partials 7752 4550 -3202
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-11](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-21](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.34% <58.98%> (-29.48%)` | :arrow_down: |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [skip-bytebuffers-true](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.34% <58.98%> (-29.56%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.34% <58.98%> (-29.56%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/16165/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.34% <58.98%> (-0.23%)` | :arrow_down: |
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=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/pinot/pull/16165?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2164907175
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,44 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getExternalViewServerToSegmentsMap(String tableNameWithType,
Review Comment:
Yes, it is intentional to only take only ONLINE segments, exclude ERROR and CONSUMING ones
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "krishan1390 (via GitHub)" <gi...@apache.org>.
krishan1390 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2160256728
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdsMetadataInfo.java:
##########
@@ -32,13 +32,18 @@ public class ValidDocIdsMetadataInfo {
private final ValidDocIdsType _validDocIdsType;
Review Comment:
similar changes are required in ValidDocIdsBitmapResponse too right ? the minion executor calls /validDocIdsBitmap API where too we want to filter response from not ready servers / not loaded segments
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2162586619
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -525,7 +532,27 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap(
String.format("Table %s segment %s is not a immutable segment", tableNameWithType, segmentName),
Response.Status.BAD_REQUEST);
}
-
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ String serverStatus = "";
+ if (status.equals(ServiceStatus.Status.GOOD)) {
+ _serverMetrics.addMeteredGlobalValue(ServerMeter.READINESS_CHECK_OK_CALLS, 1);
+ serverStatus = "OK";
+ } else {
+ serverStatus = "NOT_READY";
+ }
+ HelixAdmin helixAdmin = _serverInstance.getHelixManager().getClusterManagmentTool();
+ String helixClusterName = _serverInstance.getHelixManager().getClusterName();
+
+ TableViewsUtils.TableView externalView =
+ TableViewsUtils.getTableState(tableNameWithType, TableViewsUtils.EXTERNALVIEW, TableType.REALTIME, helixAdmin,
+ helixClusterName);
+ TableViewsUtils.TableView idealStateView =
+ TableViewsUtils.getTableState(tableNameWithType, TableViewsUtils.IDEALSTATE, TableType.REALTIME, helixAdmin,
+ helixClusterName);
+
Review Comment:
Updated and removed the segment status, in executor we are checking for EXTERNAL VIEW, trying to check EXTERNAL VIEW in generator logic as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163020448
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
Yes, this is needed for checking status of each segment here. Server status is still needed because some of the UPDATING segments can change the bit map of ONLINE segments, so it is better to wait until server is in READY state.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "klsince (via GitHub)" <gi...@apache.org>.
klsince commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2162334771
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -525,7 +532,27 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap(
String.format("Table %s segment %s is not a immutable segment", tableNameWithType, segmentName),
Response.Status.BAD_REQUEST);
}
-
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ String serverStatus = "";
+ if (status.equals(ServiceStatus.Status.GOOD)) {
+ _serverMetrics.addMeteredGlobalValue(ServerMeter.READINESS_CHECK_OK_CALLS, 1);
+ serverStatus = "OK";
+ } else {
+ serverStatus = "NOT_READY";
+ }
+ HelixAdmin helixAdmin = _serverInstance.getHelixManager().getClusterManagmentTool();
+ String helixClusterName = _serverInstance.getHelixManager().getClusterName();
+
+ TableViewsUtils.TableView externalView =
+ TableViewsUtils.getTableState(tableNameWithType, TableViewsUtils.EXTERNALVIEW, TableType.REALTIME, helixAdmin,
+ helixClusterName);
+ TableViewsUtils.TableView idealStateView =
+ TableViewsUtils.getTableState(tableNameWithType, TableViewsUtils.IDEALSTATE, TableType.REALTIME, helixAdmin,
+ helixClusterName);
+
Review Comment:
It can be very costly to get both EV and IS when getting validDocIds for just one segment, considering this API can be called by many minion tasks around the same time, when executing tasks like compaction task
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2160515205
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdsMetadataInfo.java:
##########
@@ -32,13 +32,18 @@ public class ValidDocIdsMetadataInfo {
private final ValidDocIdsType _validDocIdsType;
Review Comment:
Right, was about to add it :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "krishan1390 (via GitHub)" <gi...@apache.org>.
krishan1390 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163454919
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -534,15 +547,18 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap(
if (validDocIdSnapshot == null) {
String msg = String.format(
"Found that validDocIds is missing while fetching validDocIds for table %s segment %s while "
- + "reading the validDocIds with validDocIdType %s",
- tableNameWithType, segmentDataManager.getSegmentName(), validDocIdsType);
+ + "reading the validDocIds with validDocIdType %s", tableNameWithType,
+ segmentDataManager.getSegmentName(), validDocIdsType);
LOGGER.warn(msg);
throw new WebApplicationException(msg, Response.Status.NOT_FOUND);
}
-
byte[] validDocIdsBytes = RoaringBitmapUtils.serialize(validDocIdSnapshot);
return new ValidDocIdsBitmapResponse(segmentName, indexSegment.getSegmentMetadata().getCrc(),
- finalValidDocIdsType, validDocIdsBytes);
+ finalValidDocIdsType, validDocIdsBytes, _serverInstance.getInstanceDataManager().getInstanceId(),
+ serverStatus);
+ } catch (Exception e) {
+ LOGGER.error("Failed to get validDocIds for table {}: {}", tableNameWithType, e.getMessage());
+ return null;
Review Comment:
removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163123362
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
There could be some segments stuck in BAD state even when server is READY
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2164879089
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,44 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getExternalViewServerToSegmentsMap(String tableNameWithType,
Review Comment:
Is it intentional to only keep `ONLINE` segment and not `CONSUMING`? If so, let's rename it to `getServerToOnlineSegmentsMapFromEV`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2162590031
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,36 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
Instead of changing all the callers, added a separate one for Upsert Minion tasks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "krishan1390 (via GitHub)" <gi...@apache.org>.
krishan1390 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2160359286
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/tables/TableViewsUtils.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.tables;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.IdealState;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+
+
+public class TableViewsUtils {
+ public static final String IDEALSTATE = "idealstate";
+ public static final String EXTERNALVIEW = "externalview";
+
+ private TableViewsUtils() {
+ }
+
+ public static class TableView {
+ @JsonProperty("OFFLINE")
+ public Map<String, Map<String, String>> _offline;
+ @JsonProperty("REALTIME")
+ public Map<String, Map<String, String>> _realtime;
+ }
+
+ public static TableView getSegmentsView(TableView tableView,
+ List<String> segmentNames) {
+ TableView tableViewResult = new TableView();
+ if (tableView._offline != null) {
+ tableViewResult._offline = getTableTypeSegmentsView(tableView._offline, segmentNames);
+ }
+ if (tableView._realtime != null) {
+ tableViewResult._realtime = getTableTypeSegmentsView(tableView._realtime, segmentNames);
+ }
+ return tableViewResult;
+ }
+
+ public static List<SegmentStatusInfo> getSegmentStatuses(Map<String, Map<String, String>> externalViewMap,
+ Map<String, Map<String, String>> idealStateMap) {
+ return getSegmentStatuses(externalViewMap, idealStateMap, null);
+ }
+
+ public static List<SegmentStatusInfo> getSegmentStatuses(Map<String, Map<String, String>> externalViewMap,
+ Map<String, Map<String, String>> idealStateMap, @Nullable String filterStatus) {
+ List<SegmentStatusInfo> segmentStatusInfoList = new ArrayList<>();
+
+ for (Map.Entry<String, Map<String, String>> entry : idealStateMap.entrySet()) {
+ String segment = entry.getKey();
+ Map<String, String> externalViewEntryValue = externalViewMap.get(segment);
+ Map<String, String> idealViewEntryValue = entry.getValue();
+
+ String computedStatus = computeDisplayStatus(externalViewEntryValue, idealViewEntryValue);
+ if ((filterStatus == null) || (computedStatus.equals(filterStatus))) {
+ segmentStatusInfoList.add(new SegmentStatusInfo(segment, computedStatus));
+ }
+ }
+
+ return segmentStatusInfoList;
+ }
+
+ public static Map<String, String> getSegmentStatusesMap(Map<String, Map<String, String>> externalViewMap,
+ Map<String, Map<String, String>> idealStateMap) {
+ Map<String, String> segmentStatusInfoList = new HashMap<>();
+
+ for (Map.Entry<String, Map<String, String>> entry : idealStateMap.entrySet()) {
+ String segment = entry.getKey();
+ Map<String, String> externalViewEntryValue = externalViewMap.get(segment);
+ Map<String, String> idealViewEntryValue = entry.getValue();
+
+ String computedStatus = computeDisplayStatus(externalViewEntryValue, idealViewEntryValue);
+ segmentStatusInfoList.put(segment, computedStatus);
+ }
+ return segmentStatusInfoList;
+ }
+
+ public static String computeDisplayStatus(Map<String, String> externalView, Map<String, String> idealView) {
+ if (externalView == null) {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.UPDATING;
+ }
+
+ if (isErrorSegment(externalView)) {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.BAD;
+ }
+
+ if (externalView.equals(idealView)) {
+ if (isOnlineOrConsumingSegment(externalView) || isOfflineSegment(externalView)) {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.GOOD;
+ } else {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.UPDATING;
+ }
+ }
+
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.UPDATING;
+ }
+
+ public static Map<String, Map<String, String>> getTableTypeSegmentsView(
+ Map<String, Map<String, String>> tableTypeView, List<String> segmentNames) {
+ Map<String, Map<String, String>> tableTypeViewResult = new HashMap<>();
+ for (String segmentName : segmentNames) {
+ Map<String, String> segmentView = tableTypeView.get(segmentName);
+ if (segmentView != null) {
+ tableTypeViewResult.put(segmentName, segmentView);
+ }
+ }
+ return tableTypeViewResult;
+ }
+
+ public static Map<String, Map<String, String>> getStateMap(TableView view) {
+ if (view != null && view._offline != null && !view._offline.isEmpty()) {
+ return view._offline;
+ } else if (view != null && view._realtime != null && !view._realtime.isEmpty()) {
+ return view._realtime;
+ } else {
+ return new HashMap<>();
+ }
+ }
+
+ public static boolean isErrorSegment(Map<String, String> stateMap) {
+ return stateMap.values().contains(CommonConstants.Helix.StateModel.SegmentStateModel.ERROR);
+ }
+
+ public static boolean isOnlineOrConsumingSegment(Map<String, String> stateMap) {
+ return stateMap.values().stream().allMatch(
+ state -> state.equals(CommonConstants.Helix.StateModel.SegmentStateModel.CONSUMING) || state.equals(
+ CommonConstants.Helix.StateModel.SegmentStateModel.ONLINE));
+ }
+
+ public static boolean isOfflineSegment(Map<String, String> stateMap) {
+ return stateMap.values().contains(CommonConstants.Helix.StateModel.SegmentStateModel.OFFLINE);
+ }
+
+ // we use name "view" to closely match underlying names and to not
+ // confuse with table state of enable/disable
+ public static TableView getTableState(String tableName, String view, @Nullable TableType tableType,
+ HelixAdmin helixAdmin, String helixClusterName)
+ throws Exception {
+ TableView tableView = null;
+ if (view.equalsIgnoreCase(IDEALSTATE)) {
+ tableView = getTableIdealState(tableName, tableType, helixAdmin, helixClusterName);
+ } else if (view.equalsIgnoreCase(EXTERNALVIEW)) {
+ tableView = getTableExternalView(tableName, tableType, helixAdmin, helixClusterName);
+ } else {
+ throw new Exception("Bad view name, expected ideal state of external view");
Review Comment:
lets log the view name in the exception msg
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskExecutor.java:
##########
@@ -96,6 +102,14 @@ protected SegmentConversionResult convert(PinotTaskConfig pinotTaskConfig, File
.build();
}
+ if (!validDocIdsBitmapResponse.getServerStatus().equals("OK")) {
Review Comment:
this check should be done in MinionTaskUtils.getValidDocIdFromServerMatchingCrc() itself right ? that way if server1 isn't ready, we try on server2, etc.
Because if we expect atleast 1 server to be ready for high availability, then we will get valid data and continue the task rather than fail it.
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -525,6 +530,14 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap(
String.format("Table %s segment %s is not a immutable segment", tableNameWithType, segmentName),
Response.Status.BAD_REQUEST);
}
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ String serverStatus = "";
+ if (status.equals(ServiceStatus.Status.GOOD)) {
+ _serverMetrics.addMeteredGlobalValue(ServerMeter.READINESS_CHECK_OK_CALLS, 1);
Review Comment:
lets avoid this meter increment. this metric is used to track the success rate of /health/readiness API
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
Do we still require this method getOnlineServerToSegmentsMap given we're filtering not OK servers in various /validDocIds API ? or can we continue to use getServerToSegmentsMap() ?
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -534,15 +547,18 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap(
if (validDocIdSnapshot == null) {
String msg = String.format(
"Found that validDocIds is missing while fetching validDocIds for table %s segment %s while "
- + "reading the validDocIds with validDocIdType %s",
- tableNameWithType, segmentDataManager.getSegmentName(), validDocIdsType);
+ + "reading the validDocIds with validDocIdType %s", tableNameWithType,
+ segmentDataManager.getSegmentName(), validDocIdsType);
LOGGER.warn(msg);
throw new WebApplicationException(msg, Response.Status.NOT_FOUND);
}
-
byte[] validDocIdsBytes = RoaringBitmapUtils.serialize(validDocIdSnapshot);
return new ValidDocIdsBitmapResponse(segmentName, indexSegment.getSegmentMetadata().getCrc(),
- finalValidDocIdsType, validDocIdsBytes);
+ finalValidDocIdsType, validDocIdsBytes, _serverInstance.getInstanceDataManager().getInstanceId(),
+ serverStatus);
+ } catch (Exception e) {
+ LOGGER.error("Failed to get validDocIds for table {}: {}", tableNameWithType, e.getMessage());
+ return null;
Review Comment:
why return null ? if there is an expected exception we should catch and wrap it in WebApplicationException. For unexpected exceptions it will be better to throw it back.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163123362
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
There could be some segments stuck in BAD state even when server is in READY state, we would need to avoid those segments, which is why checking external view is important
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "krishan1390 (via GitHub)" <gi...@apache.org>.
krishan1390 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2160359286
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/tables/TableViewsUtils.java:
##########
@@ -0,0 +1,239 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.common.utils.tables;
+
+import com.fasterxml.jackson.annotation.JsonProperty;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.model.ExternalView;
+import org.apache.helix.model.IdealState;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.utils.CommonConstants;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
+
+
+public class TableViewsUtils {
+ public static final String IDEALSTATE = "idealstate";
+ public static final String EXTERNALVIEW = "externalview";
+
+ private TableViewsUtils() {
+ }
+
+ public static class TableView {
+ @JsonProperty("OFFLINE")
+ public Map<String, Map<String, String>> _offline;
+ @JsonProperty("REALTIME")
+ public Map<String, Map<String, String>> _realtime;
+ }
+
+ public static TableView getSegmentsView(TableView tableView,
+ List<String> segmentNames) {
+ TableView tableViewResult = new TableView();
+ if (tableView._offline != null) {
+ tableViewResult._offline = getTableTypeSegmentsView(tableView._offline, segmentNames);
+ }
+ if (tableView._realtime != null) {
+ tableViewResult._realtime = getTableTypeSegmentsView(tableView._realtime, segmentNames);
+ }
+ return tableViewResult;
+ }
+
+ public static List<SegmentStatusInfo> getSegmentStatuses(Map<String, Map<String, String>> externalViewMap,
+ Map<String, Map<String, String>> idealStateMap) {
+ return getSegmentStatuses(externalViewMap, idealStateMap, null);
+ }
+
+ public static List<SegmentStatusInfo> getSegmentStatuses(Map<String, Map<String, String>> externalViewMap,
+ Map<String, Map<String, String>> idealStateMap, @Nullable String filterStatus) {
+ List<SegmentStatusInfo> segmentStatusInfoList = new ArrayList<>();
+
+ for (Map.Entry<String, Map<String, String>> entry : idealStateMap.entrySet()) {
+ String segment = entry.getKey();
+ Map<String, String> externalViewEntryValue = externalViewMap.get(segment);
+ Map<String, String> idealViewEntryValue = entry.getValue();
+
+ String computedStatus = computeDisplayStatus(externalViewEntryValue, idealViewEntryValue);
+ if ((filterStatus == null) || (computedStatus.equals(filterStatus))) {
+ segmentStatusInfoList.add(new SegmentStatusInfo(segment, computedStatus));
+ }
+ }
+
+ return segmentStatusInfoList;
+ }
+
+ public static Map<String, String> getSegmentStatusesMap(Map<String, Map<String, String>> externalViewMap,
+ Map<String, Map<String, String>> idealStateMap) {
+ Map<String, String> segmentStatusInfoList = new HashMap<>();
+
+ for (Map.Entry<String, Map<String, String>> entry : idealStateMap.entrySet()) {
+ String segment = entry.getKey();
+ Map<String, String> externalViewEntryValue = externalViewMap.get(segment);
+ Map<String, String> idealViewEntryValue = entry.getValue();
+
+ String computedStatus = computeDisplayStatus(externalViewEntryValue, idealViewEntryValue);
+ segmentStatusInfoList.put(segment, computedStatus);
+ }
+ return segmentStatusInfoList;
+ }
+
+ public static String computeDisplayStatus(Map<String, String> externalView, Map<String, String> idealView) {
+ if (externalView == null) {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.UPDATING;
+ }
+
+ if (isErrorSegment(externalView)) {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.BAD;
+ }
+
+ if (externalView.equals(idealView)) {
+ if (isOnlineOrConsumingSegment(externalView) || isOfflineSegment(externalView)) {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.GOOD;
+ } else {
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.UPDATING;
+ }
+ }
+
+ return CommonConstants.Helix.StateModel.DisplaySegmentStatus.UPDATING;
+ }
+
+ public static Map<String, Map<String, String>> getTableTypeSegmentsView(
+ Map<String, Map<String, String>> tableTypeView, List<String> segmentNames) {
+ Map<String, Map<String, String>> tableTypeViewResult = new HashMap<>();
+ for (String segmentName : segmentNames) {
+ Map<String, String> segmentView = tableTypeView.get(segmentName);
+ if (segmentView != null) {
+ tableTypeViewResult.put(segmentName, segmentView);
+ }
+ }
+ return tableTypeViewResult;
+ }
+
+ public static Map<String, Map<String, String>> getStateMap(TableView view) {
+ if (view != null && view._offline != null && !view._offline.isEmpty()) {
+ return view._offline;
+ } else if (view != null && view._realtime != null && !view._realtime.isEmpty()) {
+ return view._realtime;
+ } else {
+ return new HashMap<>();
+ }
+ }
+
+ public static boolean isErrorSegment(Map<String, String> stateMap) {
+ return stateMap.values().contains(CommonConstants.Helix.StateModel.SegmentStateModel.ERROR);
+ }
+
+ public static boolean isOnlineOrConsumingSegment(Map<String, String> stateMap) {
+ return stateMap.values().stream().allMatch(
+ state -> state.equals(CommonConstants.Helix.StateModel.SegmentStateModel.CONSUMING) || state.equals(
+ CommonConstants.Helix.StateModel.SegmentStateModel.ONLINE));
+ }
+
+ public static boolean isOfflineSegment(Map<String, String> stateMap) {
+ return stateMap.values().contains(CommonConstants.Helix.StateModel.SegmentStateModel.OFFLINE);
+ }
+
+ // we use name "view" to closely match underlying names and to not
+ // confuse with table state of enable/disable
+ public static TableView getTableState(String tableName, String view, @Nullable TableType tableType,
+ HelixAdmin helixAdmin, String helixClusterName)
+ throws Exception {
+ TableView tableView = null;
+ if (view.equalsIgnoreCase(IDEALSTATE)) {
+ tableView = getTableIdealState(tableName, tableType, helixAdmin, helixClusterName);
+ } else if (view.equalsIgnoreCase(EXTERNALVIEW)) {
+ tableView = getTableExternalView(tableName, tableType, helixAdmin, helixClusterName);
+ } else {
+ throw new Exception("Bad view name, expected ideal state of external view");
Review Comment:
lets log the view name in the exception msg
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163020448
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
Yes, this is needed for checking status of each segment here in external view rather than in Ideal state. Server status is still needed because some of the UPDATING segments can change the bit map of ONLINE segments, so it is better to wait until server is in READY state.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #16165:
URL: https://github.com/apache/pinot/pull/16165
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2164697069
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompactmerge/UpsertCompactMergeTaskExecutor.java:
##########
@@ -108,8 +108,9 @@ protected List<SegmentConversionResult> convert(PinotTaskConfig pinotTaskConfig,
// Fetch validDocID snapshot from server and get record-reader for compacted reader.
List<RecordReader> recordReaders = segmentMetadataList.stream().map(x -> {
- RoaringBitmap validDocIds = MinionTaskUtils.getValidDocIdFromServerMatchingCrc(tableNameWithType, x.getName(),
- ValidDocIdsType.SNAPSHOT.name(), MINION_CONTEXT, x.getCrc());
+ RoaringBitmap validDocIds =
Review Comment:
(minor) Seems the changes in this file are unnecessary
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
We want to make the method name more specific to reflect this one checks EV instead of IS. We should also exclude replaced segments
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
+ Map<String, List<String>> serverToSegmentsMap = new TreeMap<>();
+ IdealState idealState = _helixAdmin.getResourceIdealState(_helixClusterName, tableNameWithType);
+ ExternalView externalView = _helixAdmin.getResourceExternalView(_helixClusterName, tableNameWithType);
+ if (idealState == null) {
+ throw new IllegalStateException("Ideal State does not exist for table: " + tableNameWithType);
+ }
+ if (externalView == null) {
+ throw new IllegalStateException("External View state does not exist for table: " + tableNameWithType);
+ }
+
+ Map<String, Map<String, String>> idealStateMap = idealState.getRecord().getMapFields();
+
+ for (Map.Entry<String, Map<String, String>> entry : idealStateMap.entrySet()) {
+ String segmentName = entry.getKey();
+ Map<String, String> externalViewStateMap = externalView.getStateMap(segmentName);
Review Comment:
Need to do a `null` check here
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/MinionTaskUtils.java:
##########
@@ -236,6 +237,17 @@ public static RoaringBitmap getValidDocIdFromServerMatchingCrc(String tableNameW
LOGGER.warn(message);
continue;
}
+
+ // skipping servers which are not in READY state. The bitmaps would be inconsistent when
+ // server is NOT READY as UPDATING segments might be updating the ONLINE segments
+ if (!validDocIdsBitmapResponse.getServerStatus().equals(ServiceStatus.Status.GOOD)) {
+ String message = "Server " + validDocIdsBitmapResponse.getInstanceId() + " is in "
+ + validDocIdsBitmapResponse.getServerStatus() + " state, skipping it for execution for segment: "
+ + validDocIdsBitmapResponse.getSegmentName() + ". Will try other servers.";
+ LOGGER.error(message);
Review Comment:
Make it a warning?
##########
pinot-common/src/main/java/org/apache/pinot/common/restlet/resources/ValidDocIdsBitmapResponse.java:
##########
@@ -19,21 +19,27 @@
package org.apache.pinot.common.restlet.resources;
import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.pinot.common.utils.ServiceStatus;
public class ValidDocIdsBitmapResponse {
private final String _segmentName;
private final String _segmentCrc;
private final ValidDocIdsType _validDocIdsType;
private final byte[] _bitmap;
+ private final String _instanceId;
+ private final ServiceStatus.Status _serverStatus;
Review Comment:
(minor) Keep the order of declaration, constructor, getter consistent for `ValidDocIdsBitmapResponse` and `ValidDocIdsMetadataInfo`
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/MinionTaskUtils.java:
##########
@@ -236,6 +237,17 @@ public static RoaringBitmap getValidDocIdFromServerMatchingCrc(String tableNameW
LOGGER.warn(message);
continue;
}
+
+ // skipping servers which are not in READY state. The bitmaps would be inconsistent when
+ // server is NOT READY as UPDATING segments might be updating the ONLINE segments
+ if (!validDocIdsBitmapResponse.getServerStatus().equals(ServiceStatus.Status.GOOD)) {
Review Comment:
Do we need server to be up for snapshot mode?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2164908297
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/MinionTaskUtils.java:
##########
@@ -236,6 +237,17 @@ public static RoaringBitmap getValidDocIdFromServerMatchingCrc(String tableNameW
LOGGER.warn(message);
continue;
}
+
+ // skipping servers which are not in READY state. The bitmaps would be inconsistent when
+ // server is NOT READY as UPDATING segments might be updating the ONLINE segments
+ if (!validDocIdsBitmapResponse.getServerStatus().equals(ServiceStatus.Status.GOOD)) {
Review Comment:
Good point
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2164907175
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,44 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getExternalViewServerToSegmentsMap(String tableNameWithType,
Review Comment:
Yes, it is intentional to only take only ONLINE segments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2165058680
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,44 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getServerToOnlineSegmentsMapFromEV(String tableNameWithType,
Review Comment:
I will do it later in a separate 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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status in the validDocIds info API - Upsert Tables [pinot]
Posted by "deepthi912 (via GitHub)" <gi...@apache.org>.
deepthi912 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2164804688
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/MinionTaskUtils.java:
##########
@@ -236,6 +237,17 @@ public static RoaringBitmap getValidDocIdFromServerMatchingCrc(String tableNameW
LOGGER.warn(message);
continue;
}
+
+ // skipping servers which are not in READY state. The bitmaps would be inconsistent when
+ // server is NOT READY as UPDATING segments might be updating the ONLINE segments
+ if (!validDocIdsBitmapResponse.getServerStatus().equals(ServiceStatus.Status.GOOD)) {
Review Comment:
Kept it consistent across all the modes for now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "krishan1390 (via GitHub)" <gi...@apache.org>.
krishan1390 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163043427
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3235,6 +3235,35 @@ public Map<String, List<String>> getServerToSegmentsMap(String tableNameWithType
return serverToSegmentsMap;
}
+ /**
+ * Get the servers to segments map for which servers are ONLINE in external view for those segments in IDEAL STATE
+ */
+ public Map<String, List<String>> getOnlineServerToSegmentsMap(String tableNameWithType) {
Review Comment:
yea I agree to wait until server is in READY state.
my question was given we're waiting until server is in READY state, if we still need to check external view ? wouldn't all segments be ONLINE in EV if server is in READY state ?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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
Re: [PR] Add server status and segments status in the validDocIds info API - Upsert Tables [pinot]
Posted by "krishan1390 (via GitHub)" <gi...@apache.org>.
krishan1390 commented on code in PR #16165:
URL: https://github.com/apache/pinot/pull/16165#discussion_r2163454383
##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java:
##########
@@ -525,6 +530,14 @@ public ValidDocIdsBitmapResponse downloadValidDocIdsBitmap(
String.format("Table %s segment %s is not a immutable segment", tableNameWithType, segmentName),
Response.Status.BAD_REQUEST);
}
+ ServiceStatus.Status status = ServiceStatus.getServiceStatus(_instanceId);
+ String serverStatus = "";
+ if (status.equals(ServiceStatus.Status.GOOD)) {
+ _serverMetrics.addMeteredGlobalValue(ServerMeter.READINESS_CHECK_OK_CALLS, 1);
Review Comment:
removed
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/upsertcompaction/UpsertCompactionTaskExecutor.java:
##########
@@ -96,6 +102,14 @@ protected SegmentConversionResult convert(PinotTaskConfig pinotTaskConfig, File
.build();
}
+ if (!validDocIdsBitmapResponse.getServerStatus().equals("OK")) {
Review Comment:
moved to MinionTaskUtils
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
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