You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "krishna-st (via GitHub)" <gi...@apache.org> on 2026/01/08 07:25:25 UTC
[PR] Add GET /minions/status API endpoint [pinot]
krishna-st opened a new pull request, #17475:
URL: https://github.com/apache/pinot/pull/17475
Add GET /minions/status API endpoint
Implements new REST endpoint to query minion instance status including running task counts and drain state. Supports filtering by status (ONLINE/DRAINED) and limiting results.
- Added `GET /minions/status` endpoint in `PinotMinionRestletResource`
- Queries Helix for minion metadata and running tasks
- Counts RUNNING/INIT tasks per minion
Sample API Output
```json
Request: GET /minions/status
{
"currentMinionCount": 12,
"minionStatus": [
{
"instanceId": "Minion_pinot-minion-minion-0-0.pinot-pinot-minion-headless.cell-pfdz51-managed.svc.cluster.local_9514",
"host": "pinot-minion-minion-0-0.pinot-pinot-minion-headless.cell-abcd-managed.svc.cluster.local",
"port": 9514,
"runningTaskCount": 3,
"status": "ONLINE"
},
{
"instanceId": "Minion_pinot-minion-minion-0-1.pinot-pinot-minion-headless.cell-pfdz51-managed.svc.cluster.local_9514",
"host": "pinot-minion-minion-0-1.pinot-pinot-minion-headless.cell-abcd-managed.svc.cluster.local",
"port": 9514,
"runningTaskCount": 0,
"status": "DRAINED"
},
.......
]
}
Request: GET /minions/status?status=DRAINED&limit=5
{
"currentMinionCount": 2,
"minionStatus": [
{
"instanceId": "Minion_pinot-minion-minion-0-0.pinot-pinot-minion-headless.cell-pfdz51-managed.svc.cluster.local_9514",
"host": "pinot-minion-minion-0-0.pinot-pinot-minion-headless.cell-abcd-managed.svc.cluster.local",
"port": 9514,
"runningTaskCount": 0,
"status": "DRAINED"
},
{
"instanceId": "Minion_pinot-minion-minion-0-1.pinot-pinot-minion-headless.cell-pfdz51-managed.svc.cluster.local_9514",
"host": "pinot-minion-minion-0-1.pinot-pinot-minion-headless.cell-abcd-managed.svc.cluster.local",
"port": 9514,
"runningTaskCount": 3,
"status": "DRAINED"
}
]
}
```
--
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 GET /minions/status API endpoint [pinot]
Posted by "krishna-st (via GitHub)" <gi...@apache.org>.
krishna-st commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2677390804
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
+ runningTaskCounts = getRunningTaskCountsPerMinion();
Review Comment:
Added the flag
--
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 GET /minions/status API endpoint [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2675268018
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
+ runningTaskCounts = getRunningTaskCountsPerMinion();
Review Comment:
How expensive this call could be? if this is not always needed.
Shall we add a flag to provide a `includeTaskCounts=false` option.
--
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 GET /minions/status API endpoint [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2675258768
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
+ runningTaskCounts = getRunningTaskCountsPerMinion();
+ } catch (Exception e) {
+ LOGGER.warn("Failed to get running task counts from task resource manager", e);
+ // Continue with an empty map-task counts will be 0
+ }
+
+ // Build status list for each minion
+ List<MinionStatusResponse.MinionStatus> minionStatusList = new ArrayList<>();
+ for (String minionInstanceId : minionInstances) {
+ InstanceConfig instanceConfig = _helixResourceManager.getHelixInstanceConfig(minionInstanceId);
+ if (instanceConfig == null) {
+ continue;
+ }
+
+ // Determine if minion is drained
+ List<String> tags = instanceConfig.getTags();
+ boolean isDrained = tags != null && tags.contains(Helix.DRAINED_MINION_INSTANCE);
+ String status = isDrained ? "DRAINED" : "ONLINE";
+
+ // Apply status filter if specified
+ if (statusFilter != null && !statusFilter.isEmpty() && !status.equalsIgnoreCase(statusFilter)) {
+ continue;
+ }
+
+ // Get host and port
+ String host = instanceConfig.getHostName();
+ int port = Integer.parseInt(instanceConfig.getPort());
+
+ // Get a running task count for this minion
+ int runningTaskCount = runningTaskCounts.getOrDefault(minionInstanceId, 0);
+
+ MinionStatusResponse.MinionStatus minionStatus =
+ new MinionStatusResponse.MinionStatus(minionInstanceId, host, port, runningTaskCount, status);
+ minionStatusList.add(minionStatus);
+
+ // Apply limit if specified
+ if (limit > 0 && minionStatusList.size() >= limit) {
+ break;
+ }
+ }
+
+ return new MinionStatusResponse(minionStatusList.size(), minionStatusList);
Review Comment:
if currentMinionCount is list size then you just need minionStatusList in the constructor?
Unless you want to use the minion count before the limit, but then you won't break early in the previous loop.
--
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 GET /minions/status API endpoint [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2675268018
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
+ runningTaskCounts = getRunningTaskCountsPerMinion();
Review Comment:
How expensive this call could be? Shall we make it evaluated later based on the matching instances?
--
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 GET /minions/status API endpoint [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2675249653
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
Review Comment:
I feel we should also include helix enable/disable into this so status like `ONLINE`/`OFFLINE`/`DRAINED`
--
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 GET /minions/status API endpoint [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2675249653
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
Review Comment:
I feel we should also include helix enable/disable into this so status could be `ONLINE`/`OFFLINE`/`DRAINED`
--
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 GET /minions/status API endpoint [pinot]
Posted by "krishna-st (via GitHub)" <gi...@apache.org>.
krishna-st commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2677399581
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
+ runningTaskCounts = getRunningTaskCountsPerMinion();
+ } catch (Exception e) {
+ LOGGER.warn("Failed to get running task counts from task resource manager", e);
+ // Continue with an empty map-task counts will be 0
+ }
+
+ // Build status list for each minion
+ List<MinionStatusResponse.MinionStatus> minionStatusList = new ArrayList<>();
+ for (String minionInstanceId : minionInstances) {
+ InstanceConfig instanceConfig = _helixResourceManager.getHelixInstanceConfig(minionInstanceId);
+ if (instanceConfig == null) {
+ continue;
+ }
+
+ // Determine if minion is drained
+ List<String> tags = instanceConfig.getTags();
+ boolean isDrained = tags != null && tags.contains(Helix.DRAINED_MINION_INSTANCE);
+ String status = isDrained ? "DRAINED" : "ONLINE";
+
+ // Apply status filter if specified
+ if (statusFilter != null && !statusFilter.isEmpty() && !status.equalsIgnoreCase(statusFilter)) {
+ continue;
+ }
+
+ // Get host and port
+ String host = instanceConfig.getHostName();
+ int port = Integer.parseInt(instanceConfig.getPort());
+
+ // Get a running task count for this minion
+ int runningTaskCount = runningTaskCounts.getOrDefault(minionInstanceId, 0);
+
+ MinionStatusResponse.MinionStatus minionStatus =
+ new MinionStatusResponse.MinionStatus(minionInstanceId, host, port, runningTaskCount, status);
+ minionStatusList.add(minionStatus);
+
+ // Apply limit if specified
+ if (limit > 0 && minionStatusList.size() >= limit) {
+ break;
+ }
+ }
+
+ return new MinionStatusResponse(minionStatusList.size(), minionStatusList);
Review Comment:
Removed the `limit` filter for now since it would always return the same set of minions. Will revisit the limit filter later
--
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 GET /minions/status API endpoint [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 merged PR #17475:
URL: https://github.com/apache/pinot/pull/17475
--
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 GET /minions/status API endpoint [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #17475:
URL: https://github.com/apache/pinot/pull/17475#issuecomment-3722694232
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/17475?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `0%` with `9 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 55.53%. Comparing base ([`cd08555`](https://app.codecov.io/gh/apache/pinot/commit/cd08555e186d2d61be45b002d3d6bfde47e96165?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`4e395ea`](https://app.codecov.io/gh/apache/pinot/commit/4e395ea5ec42ad73324c036c49d45462f6ed8a2d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 6 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/17475?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...spi/utils/builder/ControllerRequestURLBuilder.java](https://app.codecov.io/gh/apache/pinot/pull/17475?src=pr&el=tree&filepath=pinot-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fspi%2Futils%2Fbuilder%2FControllerRequestURLBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvYnVpbGRlci9Db250cm9sbGVyUmVxdWVzdFVSTEJ1aWxkZXIuamF2YQ==) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/17475?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 (cd08555) and HEAD (4e395ea). Click for more details.
>
> <details><summary>HEAD has 36 uploads less than BASE</summary>
>
>| Flag | BASE (cd08555) | HEAD (4e395ea) |
>|------|------|------|
>|java-21|5|1|
>|unittests1|2|1|
>|unittests|4|1|
>|temurin|10|1|
>|unittests2|2|0|
>|java-11|5|0|
>|integration|6|0|
>|integration2|2|0|
>|integration1|2|0|
>|custom-integration1|2|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #17475 +/- ##
============================================
- Coverage 63.25% 55.53% -7.73%
+ Complexity 1477 702 -775
============================================
Files 3163 2471 -692
Lines 188817 139558 -49259
Branches 28899 22251 -6648
============================================
- Hits 119442 77498 -41944
+ Misses 60117 55517 -4600
+ Partials 9258 6543 -2715
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/17475/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/17475/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/17475/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/17475/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/17475/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/17475/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/17475/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.53% <0.00%> (-7.71%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/17475/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.53% <0.00%> (-7.73%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/17475/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.53% <0.00%> (-7.73%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/17475/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `55.53% <0.00%> (-0.04%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/17475/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
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/17475?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 GET /minions/status API endpoint [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2675264048
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
Review Comment:
fix the indentation
--
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 GET /minions/status API endpoint [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #17475:
URL: https://github.com/apache/pinot/pull/17475#discussion_r2672117327
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
+ runningTaskCounts = getRunningTaskCountsPerMinion();
+ } catch (Exception e) {
+ LOGGER.warn("Failed to get running task counts from task resource manager", e);
+ // Continue with an empty map-task counts will be 0
+ }
+
+ // Build status list for each minion
+ List<MinionStatusResponse.MinionStatus> minionStatusList = new ArrayList<>();
+ for (String minionInstanceId : minionInstances) {
+ InstanceConfig instanceConfig = _helixResourceManager.getHelixInstanceConfig(minionInstanceId);
+ if (instanceConfig == null) {
+ continue;
+ }
+
+ // Determine if minion is drained
+ List<String> tags = instanceConfig.getTags();
+ boolean isDrained = tags != null && tags.contains(Helix.DRAINED_MINION_INSTANCE);
+ String status = isDrained ? "DRAINED" : "ONLINE";
+
+ // Apply status filter if specified
+ if (statusFilter != null && !statusFilter.isEmpty() && !status.equalsIgnoreCase(statusFilter)) {
+ continue;
+ }
+
+ // Get host and port
+ String host = instanceConfig.getHostName();
+ int port = Integer.parseInt(instanceConfig.getPort());
+
+ // Get a running task count for this minion
Review Comment:
Corrected spelling of 'a' to ''. Should read 'Get running task count for this minion'.
```suggestion
// Get running task count for this minion
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/minion/PinotHelixTaskResourceManager.java:
##########
@@ -1340,6 +1343,121 @@ public Map<String, Map<String, Long>> getTaskMetadataLastUpdateTimeMs() {
return MinionTaskMetadataUtils.getAllTaskMetadataLastUpdateTimeMs(propertyStore);
}
+ /**
+ * Gets the status of all minion instances, including their task counts and drain state.
+ *
+ * @param statusFilter Optional filter by status ("ONLINE" or "DRAINED"). If null, returns all minions.
+ * @param limit Maximum number of minions to return. If 0 or negative, returns all minions.
+ * @return MinionStatusResponse containing status information for minion instances
+ */
+ public MinionStatusResponse getMinionStatus(String statusFilter, int limit) {
+ // Validate status filter
+ if (statusFilter != null && !statusFilter.isEmpty()) {
+ if (!"ONLINE".equalsIgnoreCase(statusFilter) && !"DRAINED".equalsIgnoreCase(statusFilter)) {
+ throw new IllegalArgumentException("Invalid status filter. Must be 'ONLINE' or 'DRAINED'");
+ }
+ }
+
+ // Get all instances and filter for minions, then sort them
+ List<String> allInstances = _helixResourceManager.getAllInstances();
+ List<String> minionInstances = allInstances.stream()
+ .filter(InstanceTypeUtils::isMinion)
+ .sorted()
+ .collect(Collectors.toList());
+
+ // Get running task counts per minion (if a task resource manager is provided)
+ Map<String, Integer> runningTaskCounts = new HashMap<>();
+ try {
+ runningTaskCounts = getRunningTaskCountsPerMinion();
+ } catch (Exception e) {
+ LOGGER.warn("Failed to get running task counts from task resource manager", e);
+ // Continue with an empty map-task counts will be 0
Review Comment:
Missing space after hyphen. Should be 'empty map - task counts will be 0'.
```suggestion
// Continue with an empty map - task counts will be 0
```
--
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