You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "cypherean (via GitHub)" <gi...@apache.org> on 2023/12/24 14:40:37 UTC
[PR] Consistency in API response for live broker [pinot]
cypherean opened a new pull request, #12201:
URL: https://github.com/apache/pinot/pull/12201
Instructions:
1. The PR has to be tagged with at least one of the following labels (*):
1. `feature`
2. `bugfix`
3. `performance`
4. `ui`
5. `backward-incompat`
6. `release-notes` (**)
2. Remove these instructions before publishing the PR.
(*) Other labels to consider:
- `testing`
- `dependencies`
- `docker`
- `kubernetes`
- `observability`
- `security`
- `code-style`
- `extension-point`
- `refactor`
- `cleanup`
(**) Use `release-notes` label for scenarios like:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins 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] Consistency in API response for live broker [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1529293061
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java:
##########
@@ -170,9 +171,12 @@ public List<String> getLiveBrokersForTable(
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
})
- public Map<String, List<InstanceInfo>> getLiveBrokers(@Context HttpHeaders headers) {
+ public Map<String, List<InstanceInfo>> getLiveBrokers(@Context HttpHeaders headers,
+ @ApiParam(value = "Table name list(with or without type)", allowMultiple = true)
+ @QueryParam("tableNameList") List<String> tableNameList) {
Review Comment:
Suggest naming the parameter `"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] Consistency in API response for live broker [pinot]
Posted by "cypherean (via GitHub)" <gi...@apache.org>.
cypherean commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1536580339
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java:
##########
@@ -170,9 +171,12 @@ public List<String> getLiveBrokersForTable(
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
})
- public Map<String, List<InstanceInfo>> getLiveBrokers(@Context HttpHeaders headers) {
+ public Map<String, List<InstanceInfo>> getLiveBrokers(@Context HttpHeaders headers,
+ @ApiParam(value = "Table name list(with or without type)", allowMultiple = true)
+ @QueryParam("tableNameList") List<String> tableNameList) {
Review Comment:
@Jackie-Jiang addressed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] Consistency in API response for live broker [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1439256245
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java:
##########
@@ -159,9 +161,11 @@ public List<String> getLiveBrokersForTable(
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
})
- public Map<String, List<InstanceInfo>> getLiveBrokers() {
+ public Map<String, List<InstanceInfo>> getLiveBrokers(
+ @ApiParam(value = "Table name (with or without type)", required = false) @DefaultValue("")
+ @QueryParam("tableName") String tableName) {
try {
- return _pinotHelixResourceManager.getTableToLiveBrokersMapping();
+ return _pinotHelixResourceManager.getTableToLiveBrokersMapping(Optional.of(tableName));
Review Comment:
This might not give `Optional.empty()` when tableName is empty which should be the desired behaviour here. You can modify it to - `tableName.equals("") ? Optional.empty() : Optional.of(tableName)` ?
--
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] Consistency in API response for live broker [pinot]
Posted by "cypherean (via GitHub)" <gi...@apache.org>.
cypherean commented on PR #12201:
URL: https://github.com/apache/pinot/pull/12201#issuecomment-1868537068
@Jackie-Jiang comments on linked PR #11850 -
> These are not common functions, and I find them quite hard to read. Can we keep it simple? It is okay if we need to repeat some code
done
> Query param should be tableName
done
> Shall we keep it consistent with other APIs where optional field is represented as @Nullable
the default value for optional query params is "", and setting a custom default value also only takes a string
> I think we want to put hosts when table exists, even if it is empty
done
> This tableName can be raw (without type suffix). The returned map should contain the actual table name with type suffix
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] Consistency in API response for live broker [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1508366878
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3921,7 +3921,16 @@ public TableStats getTableStats(String tableNameWithType) {
* Returns map of tableName to list of live brokers
* @return Map of tableName to list of ONLINE brokers serving the table
*/
- public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() {
+ public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() throws TableNotFoundException {
+ return getTableToLiveBrokersMapping(null);
+ }
+
+ /**
+ * Returns map of arg tableName to list of live brokers
+ * @return Map of arg tableName to list of ONLINE brokers serving the table
+ */
+ public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping(@Nullable Object nullableTableName)
Review Comment:
```suggestion
public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping(@Nullable String tableName)
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3933,6 +3942,29 @@ public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() {
Map<String, List<InstanceInfo>> result = new HashMap<>();
ZNRecord znRecord = ev.getRecord();
+
+ if (nullableTableName != null && !nullableTableName.toString().isEmpty()) {
Review Comment:
```suggestion
if (StringUtils.isEmpty(tableName)) {
```
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3933,6 +3942,29 @@ public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() {
Map<String, List<InstanceInfo>> result = new HashMap<>();
ZNRecord znRecord = ev.getRecord();
+
+ if (nullableTableName != null && !nullableTableName.toString().isEmpty()) {
+ List<String> tableNameWithType = getExistingTableNamesWithType(nullableTableName.toString(), null);
+ if (tableNameWithType.isEmpty()) {
+ throw new ControllerApplicationException(LOGGER, String.format("Table=%s not found", nullableTableName),
+ Response.Status.NOT_FOUND);
+ }
+ tableNameWithType.forEach(tableName -> {
+ Map<String, String> brokersToState = znRecord.getMapField(tableName);
+ List<InstanceInfo> hosts = new ArrayList<>();
+ for (Map.Entry<String, String> brokerEntry : brokersToState.entrySet()) {
+ if ("ONLINE".equalsIgnoreCase(brokerEntry.getValue())
+ && instanceConfigMap.containsKey(brokerEntry.getKey())) {
+ InstanceConfig instanceConfig = instanceConfigMap.get(brokerEntry.getKey());
+ hosts.add(new InstanceInfo(instanceConfig.getInstanceName(), instanceConfig.getHostName(),
+ Integer.parseInt(instanceConfig.getPort())));
+ }
+ }
Review Comment:
Consider extracting a common method
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3921,7 +3921,16 @@ public TableStats getTableStats(String tableNameWithType) {
* Returns map of tableName to list of live brokers
* @return Map of tableName to list of ONLINE brokers serving the table
*/
- public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() {
+ public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() throws TableNotFoundException {
Review Comment:
(minor) reformat this change
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java:
##########
@@ -165,9 +166,11 @@ public List<String> getLiveBrokersForTable(
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
})
- public Map<String, List<InstanceInfo>> getLiveBrokers() {
+ public Map<String, List<InstanceInfo>> getLiveBrokers(
+ @ApiParam(value = "Table name (with or without type)", required = false) @DefaultValue("")
Review Comment:
Are we going to ask for liver brokers on multiple tables (e.g. for JOIN queries)? If so, suggest taking `tableName` as a list. Search for `allowMultiple = true` to find examples of using list as api param
--
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] Consistency in API response for live broker [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1453945061
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3933,6 +3942,28 @@ public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() {
Map<String, List<InstanceInfo>> result = new HashMap<>();
ZNRecord znRecord = ev.getRecord();
+
+ if (nullableTableName != null && !nullableTableName.toString().isEmpty()) {
+ List<String> tableNameWithType = getExistingTableNamesWithType(nullableTableName.toString(), null);
+ if (tableNameWithType.isEmpty()) {
+ throw new TableNotFoundException(String.format("Table=%s not found", nullableTableName));
Review Comment:
should we make it consistent?
```suggestion
throw new ControllerApplicationException(LOGGER, String.format("Table=%s not found", nullableTableName), Response.Status.NOT_FOUND);
```
--
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] Consistency in API response for live broker [pinot]
Posted by "cypherean (via GitHub)" <gi...@apache.org>.
cypherean commented on PR #12201:
URL: https://github.com/apache/pinot/pull/12201#issuecomment-1971105562
@Jackie-Jiang @walterddr please take a look
--
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] Consistency in API response for live broker [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang merged PR #12201:
URL: https://github.com/apache/pinot/pull/12201
--
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] Consistency in API response for live broker [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #12201:
URL: https://github.com/apache/pinot/pull/12201#issuecomment-1868540142
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/12201?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`fc26d6d`)](https://app.codecov.io/gh/apache/pinot/commit/fc26d6d8975b4cd46e26e460236a30e8b1eb2cde?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 0.11% compared to head [(`5bd97a6`)](https://app.codecov.io/gh/apache/pinot/pull/12201?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 46.44%.
> Report is 727 commits behind head on master.
> :exclamation: Current head 5bd97a6 differs from pull request most recent head 95ce4fd. Consider uploading reports for the commit 95ce4fd to get more accurate results
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #12201 +/- ##
=============================================
+ Coverage 0.11% 46.44% +46.33%
- Complexity 0 944 +944
=============================================
Files 2191 1809 -382
Lines 118003 95158 -22845
Branches 17868 15352 -2516
=============================================
+ Hits 137 44199 +44062
+ Misses 117846 47807 -70039
- Partials 20 3152 +3132
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration1temurin11](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1temurin17](https://app.codecov.io/gh/apache/pinot/pull/12201/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/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.44% <ø> (?)` | |
| [skip-bytebuffers-false](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.44% <ø> (?)` | |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.44% <ø> (?)` | |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.44% <ø> (?)` | |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `46.44% <ø> (?)` | |
| [unittests1temurin11](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2temurin11](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2temurin17](https://app.codecov.io/gh/apache/pinot/pull/12201/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unittests2temurin20](https://app.codecov.io/gh/apache/pinot/pull/12201/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/12201?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).
--
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] Consistency in API response for live broker [pinot]
Posted by "tibrewalpratik17 (via GitHub)" <gi...@apache.org>.
tibrewalpratik17 commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1439256245
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java:
##########
@@ -159,9 +161,11 @@ public List<String> getLiveBrokersForTable(
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
})
- public Map<String, List<InstanceInfo>> getLiveBrokers() {
+ public Map<String, List<InstanceInfo>> getLiveBrokers(
+ @ApiParam(value = "Table name (with or without type)", required = false) @DefaultValue("")
+ @QueryParam("tableName") String tableName) {
try {
- return _pinotHelixResourceManager.getTableToLiveBrokersMapping();
+ return _pinotHelixResourceManager.getTableToLiveBrokersMapping(Optional.of(tableName));
Review Comment:
This will not give `Optional.empty()` when tableName is empty which might be the desired behaviour here. You can modify it to - `tableName.equals("") ? Optional.empty() : Optional.of(tableName)` ?
--
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] Consistency in API response for live broker [pinot]
Posted by "cypherean (via GitHub)" <gi...@apache.org>.
cypherean commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1439270762
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableInstances.java:
##########
@@ -159,9 +161,11 @@ public List<String> getLiveBrokersForTable(
@ApiResponses(value = {
@ApiResponse(code = 200, message = "Success"), @ApiResponse(code = 500, message = "Internal server error")
})
- public Map<String, List<InstanceInfo>> getLiveBrokers() {
+ public Map<String, List<InstanceInfo>> getLiveBrokers(
+ @ApiParam(value = "Table name (with or without type)", required = false) @DefaultValue("")
+ @QueryParam("tableName") String tableName) {
try {
- return _pinotHelixResourceManager.getTableToLiveBrokersMapping();
+ return _pinotHelixResourceManager.getTableToLiveBrokersMapping(Optional.of(tableName));
Review Comment:
The check `if (optionalTableName.isPresent() && !optionalTableName.get().isEmpty())` in `getTableToLiveBrokersMapping` checks if the tableName string is empty.
Added a test case for the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] Consistency in API response for live broker [pinot]
Posted by "walterddr (via GitHub)" <gi...@apache.org>.
walterddr commented on code in PR #12201:
URL: https://github.com/apache/pinot/pull/12201#discussion_r1442360960
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotRunningQueryResource.java:
##########
@@ -153,7 +154,8 @@ public Map<String, Map<String, String>> getRunningQueries(
@ApiParam(value = "Timeout for brokers to return running queries") @QueryParam("timeoutMs") @DefaultValue("3000")
int timeoutMs, @Context HttpHeaders httpHeaders) {
try {
- Map<String, List<InstanceInfo>> tableBrokers = _pinotHelixResourceManager.getTableToLiveBrokersMapping();
+ Map<String, List<InstanceInfo>> tableBrokers = _pinotHelixResourceManager
+ .getTableToLiveBrokersMapping(Optional.empty());
Review Comment:
no need to change here. add a new overload in PinotHelixResourceManager since it is a public API
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -3977,7 +3977,8 @@ public TableStats getTableStats(String tableNameWithType) {
* Returns map of tableName to list of live brokers
* @return Map of tableName to list of ONLINE brokers serving the table
*/
- public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() {
+ public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping(Optional<String> optionalTableName)
Review Comment:
keep both
```suggestion
public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping() {
return getTableToLiveBrokersMapping(null);
}
public Map<String, List<InstanceInfo>> getTableToLiveBrokersMapping(@Nullable String optionalTableName) {
..
}
```
--
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