You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/07/08 20:03:06 UTC
[GitHub] [incubator-pinot] yashrsharma44 opened a new pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
yashrsharma44 opened a new pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142
Signed-off-by: Yash Sharma <ya...@gmail.com>
## Description
Adds a query parameter - `sortByTime` - for sorting based on creation/modification time of table.
## Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
* [ ] Yes (Please label as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR fix a zero-downtime upgrade introduced earlier?
* [ ] Yes (Please label this as **<code>backward-incompat</code>**, and complete the section below on Release Notes)
Does this PR otherwise need attention when creating release notes? Things to consider:
- New configuration options
- Deprecation of configurations
- Signature changes to public methods/interfaces
- New plugins added or old plugins removed
* [ ] Yes (Please label this PR as **<code>release-notes</code>** and complete the section on Release Notes)
## Release Notes
<!-- If you have tagged this as either backward-incompat or release-notes,
you MUST add text here that you would like to see appear in release notes of the
next release. -->
<!-- If you have a series of commits adding or enabling a feature, then
add this section only in final commit that marks the feature completed.
Refer to earlier release notes to see examples of text.
-->
## Documentation
<!-- If you have introduced a new feature or configuration, please add it to the documentation as well.
See https://docs.pinot.apache.org/developers/developers-and-contributors/update-document
-->
--
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
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142
--
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
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142
--
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
[GitHub] [incubator-pinot] mcvsubbu commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-877831082
Why should sorting be the function of the API? There are external tools available to sort the results and parse them in various ways. If we need to sort by retention and then by table name, would you then extend the API?
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667202408
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time or (2) Last Modified Time
Review comment:
I have added a param for `tableName`, please check if it makes sense.
--
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
[GitHub] [incubator-pinot] mcvsubbu commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-878678074
> @mcvsubbu What external tool are you referring to? IMO listing tables in a cluster sorted with creation time/last modified time is quite common request, and currently there is no easy way to get this info. This is similar to list files within a directory sorted with creation time/last modified time.
A combination of `jq` and `sort`, for instance. You can sort/massage/filter data to your heart's content
--
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-878688331
@mcvsubbu @mqliang Are you suggesting calling `/tables` first, then loop over all the tables and get stats for each individual table, then sort on the client side? This could translate to hundreds of rest calls
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r666497311
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,15 +213,36 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ if (sortOrderByTime == "none") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames);
+ } else {
+ // Sort table names based on (1) Create Time (2) Last Modified Time
+ TableType finalTableType = tableType;
+ Collections.sort(tableNames, (Comparator<String>) (o1, o2) -> {
+ TableConfig t1, t2;
+ if (finalTableType == TableType.REALTIME) {
+ t1 = _pinotHelixResourceManager.getRealtimeTableConfig(o1);
Review comment:
Sure, let me take a look. Thanks for pointing out.
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r666672860
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,15 +213,31 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time
Review comment:
Sure
--
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
[GitHub] [incubator-pinot] codecov-commenter commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7142](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (605fb67) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/4fdf01b0bfd77569d35078974108cbfec49d2a97?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4fdf01b) will **decrease** coverage by `31.88%`.
> The diff coverage is `2.53%`.
> :exclamation: Current head 605fb67 differs from pull request most recent head 96d57aa. Consider uploading reports for the commit 96d57aa to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7142 +/- ##
=============================================
- Coverage 73.48% 41.60% -31.89%
+ Complexity 92 7 -85
=============================================
Files 1495 1497 +2
Lines 73441 73519 +78
Branches 10587 10604 +17
=============================================
- Hits 53971 30587 -23384
- Misses 15948 40341 +24393
+ Partials 3522 2591 -931
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.60% <2.53%> (-0.08%)` | :arrow_down: |
| unittests | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...inot/common/function/scalar/DateTimeFunctions.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0RhdGVUaW1lRnVuY3Rpb25zLmphdmE=) | `6.94% <0.00%> (-93.06%)` | :arrow_down: |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...roller/api/resources/PinotTaskRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFza1Jlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...lix/core/minion/PinotHelixTaskResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL21pbmlvbi9QaW5vdEhlbGl4VGFza1Jlc291cmNlTWFuYWdlci5qYXZh) | `72.00% <0.00%> (-7.42%)` | :arrow_down: |
| [...ot/controller/util/TableIngestionStatusHelper.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL1RhYmxlSW5nZXN0aW9uU3RhdHVzSGVscGVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...transform/function/DateTruncTransformFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vRGF0ZVRydW5jVHJhbnNmb3JtRnVuY3Rpb24uamF2YQ==) | `0.00% <0.00%> (-86.67%)` | :arrow_down: |
| [...he/pinot/segment/local/utils/ReplicationUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9SZXBsaWNhdGlvblV0aWxzLmphdmE=) | `92.30% <ø> (ø)` | |
| [...he/pinot/segment/local/utils/TableConfigUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC91dGlscy9UYWJsZUNvbmZpZ1V0aWxzLmphdmE=) | `47.42% <ø> (-34.69%)` | :arrow_down: |
| [...org/apache/pinot/spi/config/table/TableStatus.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlU3RhdHVzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| ... and [954 more](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4fdf01b...96d57aa](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] mqliang commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
mqliang commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-878691836
> Are you suggesting calling /tables first, then loop over all the tables and get stats for each individual table, then sort on the client side? This could translate to hundreds of rest calls
@Jackie-Jiang Sorry for the confusing, I was assuming there is a `@Path("/tables/stats")` API which will return all data table names as well as their stats info (creation time, modification time), but it turns out it's a `@Path("/tables/{tableName}/stats")` API which will return stats for a single table. So my suggestion is:
* If client want to sort by name, use the @Path("/tables") API and sort the result at client side.
* Add a new `@Path("/tables/stats")` API, which will return all data table names as well as their stats info, if client want to sort by creation/modification time, use this API and parse/sort at client side.
Does it sounds good?
--
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-879289064
@mqliang I think it is good to add the list stats API, and I agree it can solve the sorting problem, but I still suggest adding an option in list tables API to sort on common fields because that will be much easier to use.
Lots of APIs we created are for ease of use. I don't think it is handy to force the user to further process the response from the swagger UI (maybe okay to professional administrator but they are not the only user).
--
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
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
--
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
[GitHub] [incubator-pinot] mcvsubbu edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
mcvsubbu edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-877831082
Why should sorting be the function of the API? There are external tools available to sort the results and parse them in various ways. If we need to sort by retention and then by table name, would you then extend the API? I am not in favor of adding more complexity to the API when there is more flexibility doing such operations outside of Pinot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
--
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
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7142](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (593b372) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/4fdf01b0bfd77569d35078974108cbfec49d2a97?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4fdf01b) will **decrease** coverage by `8.12%`.
> The diff coverage is `2.02%`.
> :exclamation: Current head 593b372 differs from pull request most recent head 40b6819. Consider uploading reports for the commit 40b6819 to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7142 +/- ##
============================================
- Coverage 73.48% 65.36% -8.13%
Complexity 92 92
============================================
Files 1495 1497 +2
Lines 73441 73531 +90
Branches 10587 10608 +21
============================================
- Hits 53971 48060 -5911
- Misses 15948 22078 +6130
+ Partials 3522 3393 -129
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `?` | |
| unittests | `65.36% <2.02%> (-0.08%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `68.88% <ø> (ø)` | |
| [...inot/common/function/scalar/DateTimeFunctions.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0RhdGVUaW1lRnVuY3Rpb25zLmphdmE=) | `98.61% <0.00%> (-1.39%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `0.00% <ø> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `0.00% <ø> (ø)` | |
| [...org/apache/pinot/common/metrics/MinionMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRyaWNzLmphdmE=) | `0.00% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `0.00% <ø> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionTimer.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25UaW1lci5qYXZh) | `0.00% <ø> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `50.19% <0.00%> (-6.22%)` | :arrow_down: |
| [...roller/api/resources/PinotTaskRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFza1Jlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [372 more](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4fdf01b...40b6819](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7142](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1db76ce) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/738a5844c176f0b345f5a15a3424a364c839af7b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (738a584) will **increase** coverage by `0.02%`.
> The diff coverage is `8.82%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7142 +/- ##
============================================
+ Coverage 73.53% 73.55% +0.02%
Complexity 92 92
============================================
Files 1506 1506
Lines 73801 73832 +31
Branches 10644 10655 +11
============================================
+ Hits 54266 54308 +42
+ Misses 15997 15990 -7
+ Partials 3538 3534 -4
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.93% <8.82%> (+0.08%)` | :arrow_up: |
| unittests | `65.24% <0.00%> (-0.03%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.44% <0.00%> (-0.05%)` | :arrow_down: |
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `50.17% <9.67%> (-5.43%)` | :arrow_down: |
| [...data/manager/realtime/SegmentCommitterFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU2VnbWVudENvbW1pdHRlckZhY3RvcnkuamF2YQ==) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | :arrow_down: |
| [...er/api/resources/LLCSegmentCompletionHandlers.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==) | `52.40% <0.00%> (-8.66%)` | :arrow_down: |
| [...altime/ServerSegmentCompletionProtocolHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL3JlYWx0aW1lL1NlcnZlclNlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2xIYW5kbGVyLmphdmE=) | `49.52% <0.00%> (-8.58%)` | :arrow_down: |
| [...e/data/manager/realtime/SplitSegmentCommitter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU3BsaXRTZWdtZW50Q29tbWl0dGVyLmphdmE=) | `53.84% <0.00%> (-7.70%)` | :arrow_down: |
| [.../pinot/core/query/scheduler/PriorityScheduler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUHJpb3JpdHlTY2hlZHVsZXIuamF2YQ==) | `80.82% <0.00%> (-2.74%)` | :arrow_down: |
| [...ation/function/MinMaxRangeAggregationFunction.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9NaW5NYXhSYW5nZUFnZ3JlZ2F0aW9uRnVuY3Rpb24uamF2YQ==) | `87.50% <0.00%> (-1.25%)` | :arrow_down: |
| [...ot/common/protocols/SegmentCompletionProtocol.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vcHJvdG9jb2xzL1NlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2wuamF2YQ==) | `94.78% <0.00%> (-0.95%)` | :arrow_down: |
| ... and [23 more](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [738a584...1db76ce](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667368493
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,31 +202,69 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTables(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "name|creationTime|lastModifiedTime") @QueryParam("sortType") @DefaultValue("name") String sortType,
+ @ApiParam(value = "true|false") @QueryParam("sortAsc") @DefaultValue("true") boolean sortAsc) {
try {
List<String> tableNames;
TableType tableType = null;
if (tableTypeStr != null) {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
}
+ Boolean sortTypeValid = sortType.equalsIgnoreCase("name") || sortType.equalsIgnoreCase("creationTime") || sortType
Review comment:
I have added a check at start - `sortType == null || ...`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r666678584
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,15 +213,31 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time
+ TableType finalTableType = tableType;
+ Collections.sort(tableNames, (Comparator<String>) (o1, o2) -> {
+ TableStats s1, s2;
+ s1 = _pinotHelixResourceManager.getTableStats(o1);
Review comment:
I am using a hashmap, does it make sense?
--
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
[GitHub] [incubator-pinot] mqliang commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
mqliang commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-878681829
> A combination of jq and sort, for instance. You can sort/massage/filter data to your heart's content
I agree:
* For sorting by name, client can use the `@Path("/tables")` API and sort the result.
* For sorting by creation/modification time, client can use the `@Path("/tables/{tableName}/stats")` API, which will return the table name as well as the creation/modification time info, client can then parse/sort the response
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667133484
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
Review comment:
We should treat this the same as `sortType` of "name" for backward-compatibility
```suggestion
if (sortType == null || sortType.equalsIgnoreCase("name")) {
...
}
```
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
Review comment:
Should not use `==` to compare string
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,7 +202,9 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTableData(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
Review comment:
Suggest renaming to `listTables`
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time or (2) Last Modified Time
Review comment:
I feel it is cleaner to read all table stats first, then sort on them per the sort type. We need to add `tableName` to the table stats in order to make it work.
We should make branches for `creationTime` and `lastModifiedTime`, and throw `IllegalArgumentException` when `sortType` is not valid
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time or (2) Last Modified Time
+ TableType finalTableType = tableType;
Review comment:
(nit) not used
--
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667215753
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/table/TableStats.java
##########
@@ -27,11 +27,20 @@
*/
public class TableStats {
public static final String CREATION_TIME_KEY = "creationTime";
+ public static final String LAST_MODIFIED_TIME_KEY = "lastModifiedTime";
private String _creationTime;
+ private String _lastModifiedTime;
+ private String _tableName;
- public TableStats(String creationTime) {
+ public TableStats(String creationTime, String lastModifiedTime, String tableName) {
Review comment:
Move `tableName` as the first parameter
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,31 +202,69 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTables(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "name|creationTime|lastModifiedTime") @QueryParam("sortType") @DefaultValue("name") String sortType,
+ @ApiParam(value = "true|false") @QueryParam("sortAsc") @DefaultValue("true") boolean sortAsc) {
try {
List<String> tableNames;
TableType tableType = null;
if (tableTypeStr != null) {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
}
+ Boolean sortTypeValid = sortType.equalsIgnoreCase("name") || sortType.equalsIgnoreCase("creationTime") || sortType
+ .equalsIgnoreCase("lastModifiedTime");
+ if (!sortTypeValid) {
+ throw new IllegalArgumentException(
+ "sortType expects={name|creationTime|lastModifiedTime} " + "got " + sortType);
+ }
+
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ HashMap<String, TableStats> MapStats = new HashMap<String, TableStats>();
Review comment:
For `name` sort type, we don't need to read this list
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,31 +202,69 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTables(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "name|creationTime|lastModifiedTime") @QueryParam("sortType") @DefaultValue("name") String sortType,
+ @ApiParam(value = "true|false") @QueryParam("sortAsc") @DefaultValue("true") boolean sortAsc) {
try {
List<String> tableNames;
TableType tableType = null;
if (tableTypeStr != null) {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
}
+ Boolean sortTypeValid = sortType.equalsIgnoreCase("name") || sortType.equalsIgnoreCase("creationTime") || sortType
Review comment:
This will cause NPE because sortType is optional
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,31 +202,69 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTables(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "name|creationTime|lastModifiedTime") @QueryParam("sortType") @DefaultValue("name") String sortType,
+ @ApiParam(value = "true|false") @QueryParam("sortAsc") @DefaultValue("true") boolean sortAsc) {
try {
List<String> tableNames;
TableType tableType = null;
if (tableTypeStr != null) {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
}
+ Boolean sortTypeValid = sortType.equalsIgnoreCase("name") || sortType.equalsIgnoreCase("creationTime") || sortType
+ .equalsIgnoreCase("lastModifiedTime");
+ if (!sortTypeValid) {
+ throw new IllegalArgumentException(
+ "sortType expects={name|creationTime|lastModifiedTime} " + "got " + sortType);
+ }
+
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ HashMap<String, TableStats> MapStats = new HashMap<String, TableStats>();
Review comment:
A list here should be enough. We can directly sort on this list, and read the sorted table names from this list
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,31 +202,69 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTables(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "name|creationTime|lastModifiedTime") @QueryParam("sortType") @DefaultValue("name") String sortType,
+ @ApiParam(value = "true|false") @QueryParam("sortAsc") @DefaultValue("true") boolean sortAsc) {
try {
List<String> tableNames;
TableType tableType = null;
if (tableTypeStr != null) {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
}
+ Boolean sortTypeValid = sortType.equalsIgnoreCase("name") || sortType.equalsIgnoreCase("creationTime") || sortType
+ .equalsIgnoreCase("lastModifiedTime");
+ if (!sortTypeValid) {
+ throw new IllegalArgumentException(
+ "sortType expects={name|creationTime|lastModifiedTime} " + "got " + sortType);
+ }
+
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ HashMap<String, TableStats> MapStats = new HashMap<String, TableStats>();
+ for (String tableName : tableNames) {
+ MapStats.put(tableName, _pinotHelixResourceManager.getTableStats(tableName));
+ }
+
Review comment:
Based on the sort type, sort accordingly:
```suggestion
if (sortType == null || sortType.equalsIgnoreCase("name")) {
tableNames.sort(sortAsc ? null : Comparator.reverseOrder());
} else {
boolean sortByCreationTime = sortType.equalsIgnoreCase("creationTime");
Preconditions.checkState(sortByCreationTime || sortType.equalsIgnoreCase("lastModifiedTime"), ...);
List<TableStats> tableStatsList = new ArrayList<>(tableNames.size());
for (String tableName : tableNames) {
tableStatsList.add(_pinotHelixResourceManager.getTableStats(tableName));
}
int sortFactor = sortAsc ? 1 : -1;
if (sortByCreationTime) {
Collections.sort(tableStatsList, (o1, o2) -> o1.getCreationTime().compareTo(o2.getCreationTime()) * sortFactor;
} else {
Collections.sort(tableStatsList, (o1, o2) -> o1.getModifiedTime().compareTo(o2.getModifiedTime()) * sortFactor;
}
// Create a new list from the sorted stats...
}
```
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-880060446
Hi @Jackie-Jiang!
Any suggestions for this PR, or are we good to merge this? Just curious 😄
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876723057
I have a java specific question 😅
- I am trying to import `import org.apache.pinot.common.utils.config.TableConfigUtils;`, and this package with the same name - `import org.apache.pinot.segment.local.utils.TableConfigUtils;`.
* My import contains this function - `toZNRecord(...)` - but the older import does not contain this function. Now since Java doesn't provide imports with same name, any ideas how should I move ahead?
--
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876781392
> I have a java specific question 😅
>
> * I am trying to import `import org.apache.pinot.common.utils.config.TableConfigUtils;`, and this package with the same name - `import org.apache.pinot.segment.local.utils.TableConfigUtils;`.
>
> * My import contains this function - `toZNRecord(...)` - but the older import does not contain this function. Now since Java doesn't provide imports with same name, any ideas how should I move ahead?
You can only import one of them, and use the full class path for the other one when accessing 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
[GitHub] [incubator-pinot] yashrsharma44 commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876709565
I have added some boilerplate code for the sorting feature required, wanted to make sure if the tests are not failing.
Will add the sorting param after tests pass.
--
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667201129
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time or (2) Last Modified Time
Review comment:
Yes, let's add `tableName` to `TableStats`
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667189171
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time or (2) Last Modified Time
Review comment:
Shall I add a param for `tableName` in the tableStats? This I would think, make the sorting much more cleaner.
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667190630
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,7 +202,9 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTableData(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
Review comment:
Sure
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
Review comment:
Sure
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
Review comment:
My bad
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667368703
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,31 +202,69 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTables(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "name|creationTime|lastModifiedTime") @QueryParam("sortType") @DefaultValue("name") String sortType,
+ @ApiParam(value = "true|false") @QueryParam("sortAsc") @DefaultValue("true") boolean sortAsc) {
try {
List<String> tableNames;
TableType tableType = null;
if (tableTypeStr != null) {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
}
+ Boolean sortTypeValid = sortType.equalsIgnoreCase("name") || sortType.equalsIgnoreCase("creationTime") || sortType
+ .equalsIgnoreCase("lastModifiedTime");
+ if (!sortTypeValid) {
+ throw new IllegalArgumentException(
+ "sortType expects={name|creationTime|lastModifiedTime} " + "got " + sortType);
+ }
+
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ HashMap<String, TableStats> MapStats = new HashMap<String, TableStats>();
+ for (String tableName : tableNames) {
+ MapStats.put(tableName, _pinotHelixResourceManager.getTableStats(tableName));
+ }
+
Review comment:
I have refactored the code, please let me know if this makes sense
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r666503887
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,7 +202,8 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTableData(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "asc|desc") @QueryParam("sortByTime") @DefaultValue("none") String sortOrderByTime) {
Review comment:
Sure
--
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
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7142](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1db76ce) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/738a5844c176f0b345f5a15a3424a364c839af7b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (738a584) will **decrease** coverage by `0.05%`.
> The diff coverage is `8.82%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7142 +/- ##
============================================
- Coverage 73.53% 73.47% -0.06%
Complexity 92 92
============================================
Files 1506 1506
Lines 73801 73832 +31
Branches 10644 10655 +11
============================================
- Hits 54266 54247 -19
- Misses 15997 16038 +41
- Partials 3538 3547 +9
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.81% <8.82%> (-0.05%)` | :arrow_down: |
| unittests | `65.24% <0.00%> (-0.03%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `62.44% <0.00%> (-0.05%)` | :arrow_down: |
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `50.17% <9.67%> (-5.43%)` | :arrow_down: |
| [...data/manager/realtime/SegmentCommitterFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU2VnbWVudENvbW1pdHRlckZhY3RvcnkuamF2YQ==) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [...readers/constant/ConstantMVForwardIndexReader.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L3JlYWRlcnMvY29uc3RhbnQvQ29uc3RhbnRNVkZvcndhcmRJbmRleFJlYWRlci5qYXZh) | `14.28% <0.00%> (-28.58%)` | :arrow_down: |
| [...ot/segment/local/customobject/MinMaxRangePair.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9jdXN0b21vYmplY3QvTWluTWF4UmFuZ2VQYWlyLmphdmE=) | `75.86% <0.00%> (-24.14%)` | :arrow_down: |
| [...er/api/resources/LLCSegmentCompletionHandlers.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0xMQ1NlZ21lbnRDb21wbGV0aW9uSGFuZGxlcnMuamF2YQ==) | `52.40% <0.00%> (-8.66%)` | :arrow_down: |
| [...altime/ServerSegmentCompletionProtocolHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VydmVyL3JlYWx0aW1lL1NlcnZlclNlZ21lbnRDb21wbGV0aW9uUHJvdG9jb2xIYW5kbGVyLmphdmE=) | `49.52% <0.00%> (-8.58%)` | :arrow_down: |
| [...e/data/manager/realtime/SplitSegmentCommitter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL21hbmFnZXIvcmVhbHRpbWUvU3BsaXRTZWdtZW50Q29tbWl0dGVyLmphdmE=) | `53.84% <0.00%> (-7.70%)` | :arrow_down: |
| [.../impl/dictionary/LongOffHeapMutableDictionary.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9yZWFsdGltZS9pbXBsL2RpY3Rpb25hcnkvTG9uZ09mZkhlYXBNdXRhYmxlRGljdGlvbmFyeS5qYXZh) | `67.02% <0.00%> (-5.32%)` | :arrow_down: |
| [.../pinot/core/query/scheduler/PriorityScheduler.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9zY2hlZHVsZXIvUHJpb3JpdHlTY2hlZHVsZXIuamF2YQ==) | `80.82% <0.00%> (-2.74%)` | :arrow_down: |
| ... and [17 more](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [738a584...1db76ce](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r666604034
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,15 +213,31 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time
Review comment:
Let's separate the sort type for creation time and last modified time because both of them can be useful. You may add the last modified time to the table stats
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,15 +213,31 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time
+ TableType finalTableType = tableType;
+ Collections.sort(tableNames, (Comparator<String>) (o1, o2) -> {
+ TableStats s1, s2;
+ s1 = _pinotHelixResourceManager.getTableStats(o1);
Review comment:
Don't read table stats in the comparator. Cache them before the sorting.
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667368660
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,31 +202,69 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTables(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "name|creationTime|lastModifiedTime") @QueryParam("sortType") @DefaultValue("name") String sortType,
+ @ApiParam(value = "true|false") @QueryParam("sortAsc") @DefaultValue("true") boolean sortAsc) {
try {
List<String> tableNames;
TableType tableType = null;
if (tableTypeStr != null) {
tableType = TableType.valueOf(tableTypeStr.toUpperCase());
}
+ Boolean sortTypeValid = sortType.equalsIgnoreCase("name") || sortType.equalsIgnoreCase("creationTime") || sortType
+ .equalsIgnoreCase("lastModifiedTime");
+ if (!sortTypeValid) {
+ throw new IllegalArgumentException(
+ "sortType expects={name|creationTime|lastModifiedTime} " + "got " + sortType);
+ }
+
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ HashMap<String, TableStats> MapStats = new HashMap<String, TableStats>();
Review comment:
Sure, I have handled the name case separately. Also switched to `ArrayList`
##########
File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/table/TableStats.java
##########
@@ -27,11 +27,20 @@
*/
public class TableStats {
public static final String CREATION_TIME_KEY = "creationTime";
+ public static final String LAST_MODIFIED_TIME_KEY = "lastModifiedTime";
private String _creationTime;
+ private String _lastModifiedTime;
+ private String _tableName;
- public TableStats(String creationTime) {
+ public TableStats(String creationTime, String lastModifiedTime, String tableName) {
Review comment:
Sure
--
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
[GitHub] [incubator-pinot] yashrsharma44 commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r667190787
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,21 +214,60 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
+ }
+ if (sortType == null) {
+ return JsonUtils.newObjectNode().set("tables", JsonUtils.objectToJsonNode(tableNames)).toString();
}
- Collections.sort(tableNames);
+ if (sortType == "name") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames, sortAsc ? null : Collections.reverseOrder());
+ } else {
+ // Sort table names based on (1) Create Time or (2) Last Modified Time
+ TableType finalTableType = tableType;
Review comment:
Yup :P
--
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
[GitHub] [incubator-pinot] Jackie-Jiang merged pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang merged pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142
--
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
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7142](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1db76ce) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/738a5844c176f0b345f5a15a3424a364c839af7b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (738a584) will **decrease** coverage by `31.71%`.
> The diff coverage is `8.82%`.
[](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7142 +/- ##
=============================================
- Coverage 73.53% 41.81% -31.72%
+ Complexity 92 7 -85
=============================================
Files 1506 1506
Lines 73801 73832 +31
Branches 10644 10655 +11
=============================================
- Hits 54266 30871 -23395
- Misses 15997 40335 +24338
+ Partials 3538 2626 -912
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.81% <8.82%> (-0.05%)` | :arrow_down: |
| unittests | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ntroller/helix/core/PinotHelixResourceManager.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9oZWxpeC9jb3JlL1Bpbm90SGVsaXhSZXNvdXJjZU1hbmFnZXIuamF2YQ==) | `38.81% <0.00%> (-23.68%)` | :arrow_down: |
| [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `26.98% <9.67%> (-28.61%)` | :arrow_down: |
| [...c/main/java/org/apache/pinot/common/tier/Tier.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/data/DateTimeFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EYXRlVGltZUZpZWxkU3BlYy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EaW1lbnNpb25GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/readers/FileFormat.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [957 more](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [738a584...1db76ce](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#discussion_r666495789
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -200,7 +202,8 @@ public String recommendConfig(String inputStr) {
@Produces(MediaType.APPLICATION_JSON)
@Path("/tables")
@ApiOperation(value = "Lists all tables in cluster", notes = "Lists all tables in cluster")
- public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr) {
+ public String listTableData(@ApiParam(value = "realtime|offline") @QueryParam("type") String tableTypeStr,
+ @ApiParam(value = "asc|desc") @QueryParam("sortByTime") @DefaultValue("none") String sortOrderByTime) {
Review comment:
Let's make it 2 parameters, one for the sort order (asc/desc, can also be modeled as a boolean), one for the sort type (name/time etc.)
##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
##########
@@ -210,15 +213,36 @@ public String listTableConfigs(@ApiParam(value = "realtime|offline") @QueryParam
if (tableType == null) {
tableNames = _pinotHelixResourceManager.getAllRawTables();
+ } else if (tableType == TableType.REALTIME) {
+ tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
} else {
- if (tableType == TableType.REALTIME) {
- tableNames = _pinotHelixResourceManager.getAllRealtimeTables();
- } else {
- tableNames = _pinotHelixResourceManager.getAllOfflineTables();
- }
+ tableNames = _pinotHelixResourceManager.getAllOfflineTables();
}
- Collections.sort(tableNames);
+ if (sortOrderByTime == "none") {
+ // Sort table names alphabetically
+ Collections.sort(tableNames);
+ } else {
+ // Sort table names based on (1) Create Time (2) Last Modified Time
+ TableType finalTableType = tableType;
+ Collections.sort(tableNames, (Comparator<String>) (o1, o2) -> {
+ TableConfig t1, t2;
+ if (finalTableType == TableType.REALTIME) {
+ t1 = _pinotHelixResourceManager.getRealtimeTableConfig(o1);
Review comment:
The time info is already lost when converting `ZNRecord` to `TableConfig`. You may refer to `PinotHelixResourceManager.getTableStats()` on how to retrieve the `ZNRecord`
--
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
[GitHub] [incubator-pinot] yashrsharma44 edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
yashrsharma44 edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876709565
<del>I have added some boilerplate code for the sorting feature required, wanted to make sure if the tests are not failing.
Will add the sorting param after tests pass.</del>
Made the PR ready for Review :)
--
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-878677140
@mcvsubbu What external tool are you referring to? IMO listing tables in a cluster sorted with creation time/last modified time is quite common request, and currently there is no easy way to get this info. This is similar to list files within a directory sorted with creation time/last modified time.
--
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
[GitHub] [incubator-pinot] Jackie-Jiang commented on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-878688927
I still don't see why adding sort type a bad thing to the list table api. If not provided, it will just work as is. Adding it can save lots of effort on the user side
--
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
[GitHub] [incubator-pinot] codecov-commenter edited a comment on pull request #7142: Added a query param to /tables for getting sorted table names based on time metadata
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7142:
URL: https://github.com/apache/incubator-pinot/pull/7142#issuecomment-876860754
# [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7142](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (593b372) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/4fdf01b0bfd77569d35078974108cbfec49d2a97?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4fdf01b) will **decrease** coverage by `0.06%`.
> The diff coverage is `3.03%`.
> :exclamation: Current head 593b372 differs from pull request most recent head 40b6819. Consider uploading reports for the commit 40b6819 to get more accurate results
[](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7142 +/- ##
============================================
- Coverage 73.48% 73.42% -0.07%
Complexity 92 92
============================================
Files 1495 1497 +2
Lines 73441 73531 +90
Branches 10587 10608 +21
============================================
+ Hits 53971 53988 +17
- Misses 15948 16023 +75
+ Partials 3522 3520 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| integration | `41.65% <1.01%> (-0.02%)` | :arrow_down: |
| unittests | `65.36% <2.02%> (-0.08%)` | :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=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/java/org/apache/pinot/client/ExecutionStats.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0V4ZWN1dGlvblN0YXRzLmphdmE=) | `68.88% <ø> (ø)` | |
| [...inot/common/function/scalar/DateTimeFunctions.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vc2NhbGFyL0RhdGVUaW1lRnVuY3Rpb25zLmphdmE=) | `98.61% <0.00%> (-1.39%)` | :arrow_down: |
| [...a/org/apache/pinot/common/metrics/MinionGauge.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25HYXVnZS5qYXZh) | `90.00% <ø> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionMeter.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRlci5qYXZh) | `100.00% <ø> (ø)` | |
| [...org/apache/pinot/common/metrics/MinionMetrics.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25NZXRyaWNzLmphdmE=) | `57.14% <ø> (ø)` | |
| [.../apache/pinot/common/metrics/MinionQueryPhase.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25RdWVyeVBoYXNlLmphdmE=) | `100.00% <ø> (ø)` | |
| [...a/org/apache/pinot/common/metrics/MinionTimer.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vbWV0cmljcy9NaW5pb25UaW1lci5qYXZh) | `0.00% <ø> (ø)` | |
| [...che/pinot/controller/api/debug/TableDebugInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvZGVidWcvVGFibGVEZWJ1Z0luZm8uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...roller/api/resources/PinotTaskRestletResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFza1Jlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...t/controller/api/resources/TableDebugResource.java](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlRGVidWdSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [36 more](https://codecov.io/gh/apache/incubator-pinot/pull/7142/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [4fdf01b...40b6819](https://codecov.io/gh/apache/incubator-pinot/pull/7142?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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