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 2020/12/08 02:16:11 UTC

[GitHub] [incubator-pinot] sajjad-moradi opened a new pull request #6331: Add a Controller endpoint to return table creation time

sajjad-moradi opened a new pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331


   ## Description
   Added a GET endpoint on Controller to return table creation time. 
   For one of our clusters, some admin tools need to delete the tables two weeks after the creation time. Therefore the table creation time API is exposed in this PR.
   
   ## Testing Done
   Ran `quick-start-batch.sh` locally and hit the endpoint:
   ```bash
   ➜  ~ curl 'localhost:9000/tables/baseballStats/creationTime?type=offline'
   1607392187128
   ➜  ~ date -r 1607392187
   Mon Dec  7 17:49:47 PST 2020
   ➜  ~ curl 'localhost:9000/tables/baseballStats/creationTime'
   {"code":400,"error":"Illegal table type: null"}
   ```
   Here is the ZooInspector snapshot for the same table which verifies the creation time returned by the endpoint:
   ![image](https://user-images.githubusercontent.com/8548220/101429151-18223e00-38b7-11eb-9a69-b111a6c77012.png)
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] mcvsubbu commented on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-742031284


   @sajjad-moradi  please model it as follows:
   `
   /table/{tableName}/stats
   `
   returning a json map -- one for realtime and one for offline. For now, only creationTime
   `
   {
     "OFFLINE" : {
       "creationTime" : "20201206T2130Z"
     },
     "REALTIME" : {
       "creationTime" : "20201208T2130Z"
     }
   }


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] mcvsubbu commented on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-742007386


   If we are modeling it as a separate call, let us not have one for creatoinTime alone. 
   
   Let us call it `/tables/{tableName}/metadata` or `/tables/{tableName}/info`
   @kishoreg  any preferences here?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-740341902


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=h1) Report
   > Merging [#6331](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=desc) (1b864ca) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `1.09%`.
   > The diff coverage is `45.12%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6331/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6331      +/-   ##
   ==========================================
   - Coverage   66.44%   65.35%   -1.10%     
   ==========================================
     Files        1075     1286     +211     
     Lines       54773    62055    +7282     
     Branches     8168     9005     +837     
   ==========================================
   + Hits        36396    40554    +4158     
   - Misses      15700    18608    +2908     
   - Partials     2677     2893     +216     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `65.35% <45.12%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: |
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `66.66% <0.00%> (+9.52%)` | :arrow_up: |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <0.00%> (-13.29%)` | :arrow_down: |
   | [...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydC5qYXZh) | `10.90% <0.00%> (-51.10%)` | :arrow_down: |
   | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `73.80% <ø> (+0.63%)` | :arrow_up: |
   | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: |
   | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `100.00% <ø> (ø)` | |
   | ... and [1134 more](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=footer). Last update [22d25ff...1b864ca](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] mcvsubbu merged pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu merged pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#discussion_r540366317



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2573,6 +2577,13 @@ private void waitForSegmentsBecomeOnline(String tableNameWithType, Set<String> s
     return onlineSegments;
   }
 
+  public TableStats getTableStats(String tableNameWithType) {
+    String zkPath = ZKMetadataProvider.constructPropertyStorePathForResourceConfig(tableNameWithType);
+    ZNRecord znRecord = ZKMetadataProvider.getZnRecord(_propertyStore, zkPath);
+    String creationTime = SIMPLE_DATE_FORMAT.format(znRecord.getCreationTime());

Review comment:
       It occurred to me that it may be better to return a `long` , in seconds since epoch. Better granularity, less parsing and code on either side (read: easier for automation and scripts)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-740341902


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=h1) Report
   > Merging [#6331](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=desc) (d6aebf2) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **increase** coverage by `7.59%`.
   > The diff coverage is `68.27%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6331/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6331      +/-   ##
   ==========================================
   + Coverage   66.44%   74.04%   +7.59%     
   ==========================================
     Files        1075     1283     +208     
     Lines       54773    61958    +7185     
     Branches     8168     8995     +827     
   ==========================================
   + Hits        36396    45876    +9480     
   + Misses      15700    13118    -2582     
   - Partials     2677     2964     +287     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration | `45.54% <51.88%> (?)` | |
   | unittests | `65.36% <45.12%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `44.44% <0.00%> (-4.40%)` | :arrow_down: |
   | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `78.57% <ø> (+5.40%)` | :arrow_up: |
   | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: |
   | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../pinot/common/function/DateTimePatternHandler.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRGF0ZVRpbWVQYXR0ZXJuSGFuZGxlci5qYXZh) | `83.33% <ø> (ø)` | |
   | [...ot/common/function/FunctionDefinitionRegistry.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25EZWZpbml0aW9uUmVnaXN0cnkuamF2YQ==) | `88.88% <ø> (+44.44%)` | :arrow_up: |
   | [...org/apache/pinot/common/function/FunctionInfo.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vRnVuY3Rpb25JbmZvLmphdmE=) | `100.00% <ø> (ø)` | |
   | ... and [1068 more](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=footer). Last update [22d25ff...d6aebf2](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] mcvsubbu commented on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-740819151


   > can we store creation time in the tableConfig instead of relying on the znode in ZK.
   
   Is the reasoning here to make it easier to move away from zk? Or, do you expect that the metadata is somehow changeable in zk? Or, is it that we can avoid a new API (I like that) and just get the table config?
   
   Another way can be to get the metadata along with the `GET` API on tables?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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] sajjad-moradi commented on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-741113214


   > > can we store creation time in the tableConfig instead of relying on the znode in ZK.
   > 
   > Is the reasoning here to make it easier to move away from zk? Or, do you expect that the metadata is somehow changeable in zk? Or, is it that we can avoid a new API (I like that) and just get the table config?
   > 
   > Another way can be to get the metadata along with the `GET` API on tables?
   
   So Subbu's suggestion for GET api on tables will be something like:
   ```
   {
     "OFFLINE" : {
     ...
     },
     "REALTIME" : {
     ...
     },
     "METADATA" : {
       "REALTIME" : {
         "creationTime" : "20201206T2130Z"
       },
       "OFFLINE" : {
         "creationTime" : "20201206T2130Z"
       }
     }
   }
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] mcvsubbu edited a comment on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu edited a comment on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-742031284


   @sajjad-moradi  please model it as follows:
   `
   /table/{tableName}/stats
   `
   returning a json map -- one for realtime and one for offline. For now, only creationTime
   `
   {
     "OFFLINE" : {
       "creationTime" : "20201206T2130Z"
     },
     "REALTIME" : {
       "creationTime" : "20201208T2130Z"
     }
   }
   `


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] mcvsubbu commented on a change in pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#discussion_r540372776



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2573,6 +2577,13 @@ private void waitForSegmentsBecomeOnline(String tableNameWithType, Set<String> s
     return onlineSegments;
   }
 
+  public TableStats getTableStats(String tableNameWithType) {
+    String zkPath = ZKMetadataProvider.constructPropertyStorePathForResourceConfig(tableNameWithType);
+    ZNRecord znRecord = ZKMetadataProvider.getZnRecord(_propertyStore, zkPath);
+    String creationTime = SIMPLE_DATE_FORMAT.format(znRecord.getCreationTime());

Review comment:
       I take that back. I think at this stage it is more useful to be able to readily read it on the console. We can leave it at this, but please do add seconds as well.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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 #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-741259630


   > > > can we store creation time in the tableConfig instead of relying on the znode in ZK.
   > > 
   > > 
   > > Is the reasoning here to make it easier to move away from zk? Or, do you expect that the metadata is somehow changeable in zk? Or, is it that we can avoid a new API (I like that) and just get the table config?
   > > Another way can be to get the metadata along with the `GET` API on tables?
   > 
   > So Subbu's suggestion for GET api on tables will be something like:
   > 
   > ```
   > {
   >   "OFFLINE" : {
   >   ...
   >   },
   >   "REALTIME" : {
   >   ...
   >   },
   >   "METADATA" : {
   >     "REALTIME" : {
   >       "creationTime" : "20201206T2130Z"
   >     },
   >     "OFFLINE" : {
   >       "creationTime" : "20201206T2130Z"
   >     }
   >   }
   > }
   > ```
   
   I like sending out metadata along with the table API. What do you say, @kishoreg ? This also allows us to add other metadata later on if needed.
   
   I am not sure I prefer including  metadata in the table config znode


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-io commented on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-740341902


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=h1) Report
   > Merging [#6331](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=desc) (d6aebf2) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `1.08%`.
   > The diff coverage is `45.12%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6331/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master    #6331      +/-   ##
   ==========================================
   - Coverage   66.44%   65.36%   -1.09%     
   ==========================================
     Files        1075     1283     +208     
     Lines       54773    61958    +7185     
     Branches     8168     8995     +827     
   ==========================================
   + Hits        36396    40500    +4104     
   - Misses      15700    18574    +2874     
   - Partials     2677     2884     +207     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | unittests | `65.36% <45.12%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...e/pinot/broker/api/resources/PinotBrokerDebug.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYXBpL3Jlc291cmNlcy9QaW5vdEJyb2tlckRlYnVnLmphdmE=) | `0.00% <0.00%> (-79.32%)` | :arrow_down: |
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `71.42% <ø> (-28.58%)` | :arrow_down: |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `33.96% <0.00%> (-32.71%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ava/org/apache/pinot/client/AbstractResultSet.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Fic3RyYWN0UmVzdWx0U2V0LmphdmE=) | `66.66% <0.00%> (+9.52%)` | :arrow_up: |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `35.55% <0.00%> (-13.29%)` | :arrow_down: |
   | [...inot/client/JsonAsyncHttpPinotClientTransport.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0pzb25Bc3luY0h0dHBQaW5vdENsaWVudFRyYW5zcG9ydC5qYXZh) | `10.90% <0.00%> (-51.10%)` | :arrow_down: |
   | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `73.80% <ø> (+0.63%)` | :arrow_up: |
   | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: |
   | [...pinot/common/function/AggregationFunctionType.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZnVuY3Rpb24vQWdncmVnYXRpb25GdW5jdGlvblR5cGUuamF2YQ==) | `100.00% <ø> (ø)` | |
   | ... and [1131 more](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=footer). Last update [22d25ff...d6aebf2](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] codecov-io edited a comment on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-740341902


   # [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=h1) Report
   > Merging [#6331](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=desc) (dd8cb46) into [master](https://codecov.io/gh/apache/incubator-pinot/commit/1beaab59b73f26c4e35f3b9bc856b03806cddf5a?el=desc) (1beaab5) will **decrease** coverage by `21.02%`.
   > The diff coverage is `51.75%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-pinot/pull/6331/graphs/tree.svg?width=650&height=150&src=pr&token=4ibza2ugkz)](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #6331       +/-   ##
   ===========================================
   - Coverage   66.44%   45.42%   -21.03%     
   ===========================================
     Files        1075     1286      +211     
     Lines       54773    62054     +7281     
     Branches     8168     9005      +837     
   ===========================================
   - Hits        36396    28188     -8208     
   - Misses      15700    31546    +15846     
   + Partials     2677     2320      -357     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration | `45.42% <51.75%> (?)` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [...ot/broker/broker/AllowAllAccessControlFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL0FsbG93QWxsQWNjZXNzQ29udHJvbEZhY3RvcnkuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [.../helix/BrokerUserDefinedMessageHandlerFactory.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvYnJva2VyL2hlbGl4L0Jyb2tlclVzZXJEZWZpbmVkTWVzc2FnZUhhbmRsZXJGYWN0b3J5LmphdmE=) | `52.83% <0.00%> (-13.84%)` | :arrow_down: |
   | [...org/apache/pinot/broker/queryquota/HitCounter.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9IaXRDb3VudGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...che/pinot/broker/queryquota/MaxHitRateTracker.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9NYXhIaXRSYXRlVHJhY2tlci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...ache/pinot/broker/queryquota/QueryQuotaEntity.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcXVlcnlxdW90YS9RdWVyeVF1b3RhRW50aXR5LmphdmE=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [...ker/routing/instanceselector/InstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL0luc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `100.00% <ø> (ø)` | |
   | [...ceselector/StrictReplicaGroupInstanceSelector.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtYnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9icm9rZXIvcm91dGluZy9pbnN0YW5jZXNlbGVjdG9yL1N0cmljdFJlcGxpY2FHcm91cEluc3RhbmNlU2VsZWN0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [.../main/java/org/apache/pinot/client/Connection.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY2xpZW50cy9waW5vdC1qYXZhLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY2xpZW50L0Nvbm5lY3Rpb24uamF2YQ==) | `22.22% <0.00%> (-26.62%)` | :arrow_down: |
   | [...not/common/assignment/InstancePartitionsUtils.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vYXNzaWdubWVudC9JbnN0YW5jZVBhcnRpdGlvbnNVdGlscy5qYXZh) | `64.28% <ø> (-8.89%)` | :arrow_down: |
   | [.../apache/pinot/common/exception/QueryException.java](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vZXhjZXB0aW9uL1F1ZXJ5RXhjZXB0aW9uLmphdmE=) | `90.27% <ø> (+5.55%)` | :arrow_up: |
   | ... and [1283 more](https://codecov.io/gh/apache/incubator-pinot/pull/6331/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=footer). Last update [22d25ff...1b864ca](https://codecov.io/gh/apache/incubator-pinot/pull/6331?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] kishoreg commented on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-742004044


   I liked the idea at first glance but most users follow this pattern
   - do a get, edit it and post it again. 
   
   I think a separate call like what you did in this PR is better but better to model it as a table stat/info call. We can expand it to add more stuff later like num segments, size etc. We need this for UI as well. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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] sajjad-moradi commented on a change in pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on a change in pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#discussion_r540421954



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2573,6 +2577,13 @@ private void waitForSegmentsBecomeOnline(String tableNameWithType, Set<String> s
     return onlineSegments;
   }
 
+  public TableStats getTableStats(String tableNameWithType) {
+    String zkPath = ZKMetadataProvider.constructPropertyStorePathForResourceConfig(tableNameWithType);
+    ZNRecord znRecord = ZKMetadataProvider.getZnRecord(_propertyStore, zkPath);
+    String creationTime = SIMPLE_DATE_FORMAT.format(znRecord.getCreationTime());

Review comment:
       Sure. Second is added now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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 #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-742006292


   > I liked the idea at first glance but most users follow this pattern
   > 
   > * do a get, edit it and post it again.
   > 
   > I think a separate call like what you did in this PR is better but better to model it as a table stat/info call. We can expand it to add more stuff later like num segments, size etc. We need this for UI as well.
   
   In any case, we don't accept the json that has elements "OFFLINE" and "REALTIME". So, in this case, doing a get/edit will not work


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] kishoreg commented on pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
kishoreg commented on pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#issuecomment-740318065


   can we store creation time in the tableConfig instead of relying on the znode in ZK. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org

[GitHub] [incubator-pinot] sajjad-moradi commented on a change in pull request #6331: Add a Controller endpoint to return table creation time

Posted by GitBox <gi...@apache.org>.
sajjad-moradi commented on a change in pull request #6331:
URL: https://github.com/apache/incubator-pinot/pull/6331#discussion_r540421954



##########
File path: pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
##########
@@ -2573,6 +2577,13 @@ private void waitForSegmentsBecomeOnline(String tableNameWithType, Set<String> s
     return onlineSegments;
   }
 
+  public TableStats getTableStats(String tableNameWithType) {
+    String zkPath = ZKMetadataProvider.constructPropertyStorePathForResourceConfig(tableNameWithType);
+    ZNRecord znRecord = ZKMetadataProvider.getZnRecord(_propertyStore, zkPath);
+    String creationTime = SIMPLE_DATE_FORMAT.format(znRecord.getCreationTime());

Review comment:
       Sure. Seconds are added now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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