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 2022/06/27 05:11:17 UTC

[GitHub] [pinot] satishwaghela opened a new pull request, #8978: #8970 Minion tab in Pinot UI

satishwaghela opened a new pull request, #8978:
URL: https://github.com/apache/pinot/pull/8978

   Closes: https://github.com/apache/pinot/issues/8970


-- 
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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1181266877

   > also, please take a look at why the Pinot Integration Test fails in the Github Actions. Seems to be throwing some exceptions from ui code
   
   Seeing this 
   ```
    ERROR in /home/runner/work/pinot/pinot/pinot-controller/src/main/resources/app/pages/TaskQueueTable.tsx
   [INFO] ./app/pages/TaskQueueTable.tsx
   [INFO] [tsl] ERROR in /home/runner/work/pinot/pinot/pinot-controller/src/main/resources/app/pages/TaskQueueTable.tsx(31,31)
   [INFO]       TS2307: Cannot find module '../components/useMinionMetadata'.
   [INFO] Child HtmlWebpackCompiler:
   [INFO]      1 asset
   [INFO]     Entrypoint HtmlWebpackPlugin_0 = __child-HtmlWebpackPlugin_0
   [INFO]     [0] ./node_modules/html-webpack-plugin/lib/loader.js!./app/index.html 1.43 KiB {0} [built]
   [INFO] npm ERR! code ELIFECYCLE
   [INFO] npm ERR! errno 2
   [INFO] npm ERR! pinot-controller-ui@1.0.0 build: `webpack --mode production`
   [INFO] npm ERR! Exit status 2
   [INFO] npm ERR! 
   [INFO] npm ERR! Failed at the pinot-controller-ui@1.0.0 build script.
   [INFO] npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
   ```


-- 
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] [pinot] joshigaurava commented on a diff in pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on code in PR #8978:
URL: https://github.com/apache/pinot/pull/8978#discussion_r915065615


##########
pinot-controller/src/main/resources/app/pages/InstanceDetails.tsx:
##########
@@ -308,7 +315,7 @@ const InstanceDetails = ({ match }: RouteComponentProps<Props>) => {
               </CustomButton>
               <CustomButton
                 onClick={handleDropAction}
-                tooltipTitle="Removes the node from the cluster. Untag & rebalance (to ensure node is not being used by any table), and shutdown instance, before dropping."
+                tooltipTitle={!instanceName.startsWith('Minion_') ? "Removes the node from the cluster. Untag & rebalance (to ensure node is not being used by any table), and shutdown instance, before dropping." : ""}

Review Comment:
   Removes the node from the cluster. Untag and rebalance (to ensure the node is not being used by any table) and shutdown the instance before dropping.
   
   @npawar 



-- 
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] [pinot] joshigaurava commented on a diff in pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on code in PR #8978:
URL: https://github.com/apache/pinot/pull/8978#discussion_r915056558


##########
pinot-controller/src/main/resources/app/utils/PinotMethodUtils.ts:
##########
@@ -746,6 +761,113 @@ const deleteInstance = (instanceName) => {
   });
 };
 
+const getAllPeriodicTaskNames = () => {
+  return getPeriodicTaskNames().then((response)=>{
+    return { columns: ['Task Name'], records: response.data.map(d => [d]) };
+  });
+};
+
+const getAllTaskTypes = async () => {
+  const finalResponse = {
+    columns: ['Task Type', 'Num Tasks in Queue', 'Queue Status'],
+    records: []
+  }
+  await new Promise((resolve, reject) => {
+    getTaskTypes().then(async (response)=>{
+      if (_.isArray(response.data)) {

Review Comment:
   Let's fix this in other files, if required.



-- 
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] [pinot] npawar merged pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar merged PR #8978:
URL: https://github.com/apache/pinot/pull/8978


-- 
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] [pinot] joshigaurava commented on a diff in pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on code in PR #8978:
URL: https://github.com/apache/pinot/pull/8978#discussion_r915053303


##########
pinot-controller/src/main/resources/app/utils/PinotMethodUtils.ts:
##########
@@ -746,6 +761,113 @@ const deleteInstance = (instanceName) => {
   });
 };
 
+const getAllPeriodicTaskNames = () => {
+  return getPeriodicTaskNames().then((response)=>{
+    return { columns: ['Task Name'], records: response.data.map(d => [d]) };
+  });
+};
+
+const getAllTaskTypes = async () => {
+  const finalResponse = {
+    columns: ['Task Type', 'Num Tasks in Queue', 'Queue Status'],
+    records: []
+  }
+  await new Promise((resolve, reject) => {
+    getTaskTypes().then(async (response)=>{
+      if (_.isArray(response.data)) {

Review Comment:
   Let's fix lodash imports in this file. Importing the entire library is not a good idea. Let's change it to `import { a, b, c } from 'lodash';`



-- 
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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1182324709

   @klsince i'll merge this for now. if you do have any feedback after you play with it, we can address those separately


-- 
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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1175690135

   Thanks @satishwaghela . I have last couple of comments
   
   1. After scheduling task using “Schedule Now”, show different message as per response. If response contains null, show message “Could not schedule task for <task type>”. If response contains a task id, show the Task ID from the response as “<Task id> scheduled” and give user an option to dismiss the message (dont auto disappear)
   2. From this page (http://localhost:9000/#/task-queue/MergeRollupTask/tables/githubEvents_OFFLINE) if i click refresh on browser, I see it takes 10-15 seconds to load.
   3. The Start Time is empty for on UI for a task with status IN_PROGRESS, but if I inspect the response, I do see the startTime field.
   ![Screen Shot 2022-07-05 at 6 42 04 PM](https://user-images.githubusercontent.com/19693933/177450866-0f631797-66fd-455f-9df7-2b8f88fa8c49.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.

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] [pinot] joshigaurava commented on a diff in pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on code in PR #8978:
URL: https://github.com/apache/pinot/pull/8978#discussion_r915067462


##########
pinot-controller/src/main/resources/app/pages/InstanceDetails.tsx:
##########
@@ -68,9 +68,16 @@ type Props = {
 const InstanceDetails = ({ match }: RouteComponentProps<Props>) => {
   const classes = useStyles();
   const {instanceName} = match.params;
+  let instanceType;
+  if (instanceName.toLowerCase().startsWith('broker')) {
+    instanceType = 'BROKER';

Review Comment:
   Is it feasible to declare these as enums/constants?



-- 
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] [pinot] codecov-commenter commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1181318840

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8978?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 [#8978](https://codecov.io/gh/apache/pinot/pull/8978?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (486dd34) into [master](https://codecov.io/gh/apache/pinot/commit/c802786ea95cff67b83ff4d24f796b965e565854?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c802786) will **decrease** coverage by `54.38%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #8978       +/-   ##
   =============================================
   - Coverage     69.78%   15.40%   -54.39%     
   + Complexity     4679      170     -4509     
   =============================================
     Files          1808     1783       -25     
     Lines         94235    94134      -101     
     Branches      14052    14140       +88     
   =============================================
   - Hits          65765    14499    -51266     
   - Misses        23908    78599    +54691     
   + Partials       4562     1036     -3526     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `?` | |
   | unittests1 | `?` | |
   | unittests2 | `15.40% <ø> (+0.51%)` | :arrow_up: |
   
   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/pinot/pull/8978?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...src/main/java/org/apache/pinot/sql/FilterKind.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcWwvRmlsdGVyS2luZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...in/java/org/apache/pinot/spi/utils/StringUtil.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvU3RyaW5nVXRpbC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/spi/trace/BaseRecording.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvQmFzZVJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/pinot/spi/trace/NoOpRecording.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdHJhY2UvTm9PcFJlY29yZGluZy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...ava/org/apache/pinot/spi/config/user/RoleType.java](https://codecov.io/gh/apache/pinot/pull/8978/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-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3VzZXIvUm9sZVR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1444 more](https://codecov.io/gh/apache/pinot/pull/8978/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/pinot/pull/8978?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/pinot/pull/8978?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 [c802786...486dd34](https://codecov.io/gh/apache/pinot/pull/8978?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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1173035859

   @joshigaurava @shahsank3t please help review from code perspective. Thanks!


-- 
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] [pinot] joshigaurava commented on a diff in pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on code in PR #8978:
URL: https://github.com/apache/pinot/pull/8978#discussion_r915067201


##########
pinot-controller/src/main/resources/app/pages/InstanceDetails.tsx:
##########
@@ -308,7 +315,7 @@ const InstanceDetails = ({ match }: RouteComponentProps<Props>) => {
               </CustomButton>
               <CustomButton
                 onClick={handleDropAction}
-                tooltipTitle="Removes the node from the cluster. Untag & rebalance (to ensure node is not being used by any table), and shutdown instance, before dropping."
+                tooltipTitle={!instanceName.startsWith('Minion_') ? "Removes the node from the cluster. Untag & rebalance (to ensure node is not being used by any table), and shutdown instance, before dropping." : ""}

Review Comment:
   Is there a difference between line 74
   
   ```
   else if (instanceName.toLowerCase().startsWith('minion')) {
       instanceType = 'MINION';
     }
   ```
   
   and the condition here? Could we just use `instanceType === 'MINION'` 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.

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] [pinot] joshigaurava commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1181884437

   Looks 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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1178073758

   Still seeing high load time when I refresh this page. Seems to be coming from "info" call?
   ![Screen Shot 2022-07-07 at 11 49 40 AM](https://user-images.githubusercontent.com/19693933/177849774-68e874c6-6ea9-4e45-8b5a-7101916883fb.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.

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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1173035468

   Thanks @satishwaghela ! Just played around with this and it's pretty neat! Here's some suggestions:
   
   1. On Homepage, Move MINIONS after SERVERS (Order of these tables should be Controllers, Brokers, Servers, Minions)
   <img width="1393" alt="Screen Shot 2022-07-03 at 12 15 41 AM" src="https://user-images.githubusercontent.com/19693933/177031166-ddd5d2b1-07c6-4aeb-9eff-92aa05a28bff.png">
   
   2. Is this validation coming from UI or backend? If UI, no need for this restriction in tags for Minions, it can be anything.
   <img width="1197" alt="Screen Shot 2022-07-03 at 12 18 12 AM" src="https://user-images.githubusercontent.com/19693933/177031175-c646ed07-fc6a-41e0-bfd1-34393ce2b657.png">
   
   3. After editing the tag for Minion, I see this table appear below, which should not be there
   <img width="1185" alt="Screen Shot 2022-07-03 at 12 18 31 AM" src="https://user-images.githubusercontent.com/19693933/177031190-548fba7b-a2b4-4735-8992-dbf821cf4edb.png">
   
   4. Replace Execution Time with “Elapsed Time”. Calculate the value using System.currentTimeMillis() - startTime. Show value in minutes if < 60. In hours + minutes if > 60
   <img width="1601" alt="Screen Shot 2022-07-03 at 12 27 12 AM" src="https://user-images.githubusercontent.com/19693933/177031204-c850df92-e705-41ba-b1e6-419b38d6f9cc.png">
   
   5. After clicking Schedule Now, show a modal with message “Are you sure you want to schedule this task?” If user clicks YES, then show the Task ID from the response as “<Task id> scheduled” and give user an option to dismiss the message
   <img width="1086" alt="Screen Shot 2022-07-03 at 1 02 11 AM" src="https://user-images.githubusercontent.com/19693933/177031134-64665da6-1f5c-4d75-a3c9-752528420fe4.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.

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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1173035733

   1. These buttons have different tooltip text provided in the design doc
   2. Show every unique table name only once (in this case githubEvents_REALTIME should appear only once)
   3. On this page, when clicking “Resume All” show similar modal to the one as used in “Stop All”. Same for Cleanup All.
   4. After clicking “Stop All” immediately call the API to update “Task Queue Status” field (it should become STOPPED as per the API). Same for “Resume All”, call API to get updated status
   5. After calling Stop or Resume, sometimes going back to homepage just keeps spinning
   <img width="759" alt="Screen Shot 2022-07-03 at 12 32 42 AM" src="https://user-images.githubusercontent.com/19693933/177031273-1de042f7-8f88-4ee7-961f-4fc2b5341041.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.

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] [pinot] npawar commented on pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
npawar commented on PR #8978:
URL: https://github.com/apache/pinot/pull/8978#issuecomment-1178075628

   also, please take a look at why the Pinot Integration Test fails in the Github Actions. Seems to be throwing some exceptions from ui code 


-- 
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] [pinot] joshigaurava commented on a diff in pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on code in PR #8978:
URL: https://github.com/apache/pinot/pull/8978#discussion_r915063545


##########
pinot-controller/src/main/resources/app/pages/TaskDetail.tsx:
##########
@@ -0,0 +1,173 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { useEffect, useState } from 'react';
+import { get, each } from 'lodash';
+import moment from 'moment';
+// import { UnControlled as CodeMirror } from 'react-codemirror2';
+import { Grid, makeStyles } from '@material-ui/core';
+// import SimpleAccordion from '../components/SimpleAccordion';
+import PinotMethodUtils from '../utils/PinotMethodUtils';
+import CustomizedTables from '../components/Table';
+
+// const jsonoptions = {
+//   lineNumbers: true,
+//   mode: 'application/json',
+//   styleActiveLine: true,
+//   gutters: ['CodeMirror-lint-markers'],
+//   theme: 'default',
+//   readOnly: true
+// };
+
+const useStyles = makeStyles(() => ({
+  gridContainer: {
+    padding: 20,
+    backgroundColor: 'white',
+    maxHeight: 'calc(100vh - 70px)',
+    overflowY: 'auto'
+  },
+  operationDiv: {
+    border: '1px #BDCCD9 solid',
+    borderRadius: 4,
+    marginBottom: 20
+  },
+  body: {
+    borderTop: '1px solid #BDCCD9',
+    fontSize: '16px',
+    lineHeight: '3rem',
+    paddingLeft: '15px',
+  },
+  highlightBackground: {
+    border: '1px #4285f4 solid',
+    backgroundColor: 'rgba(66, 133, 244, 0.05)',
+    borderRadius: 4,
+    marginBottom: '20px',
+  },
+  sqlDiv: {
+    border: '1px #BDCCD9 solid',
+    borderRadius: 4,
+    marginBottom: '20px',
+  },
+  queryOutput: {
+    border: '1px solid #BDCCD9',
+    '& .CodeMirror': { height: 532 },
+  },
+}));
+
+const TaskDetail = (props) => {
+  const classes = useStyles();
+  const { taskID, taskType, queueTableName } = props.match.params;
+
+  const [fetching, setFetching] = useState(true);
+  const [taskDebugData, setTaskDebugData] = useState({});
+  const [subtaskTableData, setSubtaskTableData] = useState({ columns: ['Task ID', 'Status', 'Start Time', 'Finish Time', 'Minion Host Name'], records: [] });
+
+  const fetchData = async () => {
+    setFetching(true);
+    const debugRes = await PinotMethodUtils.getTaskDebugData(taskID);
+    const subtaskTableRecords = [];
+    each(get(debugRes, 'data.subtaskInfos', {}), (subTask) => {
+      subtaskTableRecords.push([
+        get(subTask, 'taskId'),
+        get(subTask, 'state'),
+        get(subTask, 'startTime'),
+        get(subTask, 'finishTime'),
+        get(subTask, 'participant'),
+      ])
+    });
+    setSubtaskTableData(prevState => {
+      return { ...prevState, records: subtaskTableRecords };
+    });
+    setTaskDebugData(debugRes.data);
+    setFetching(false);
+  };
+
+  useEffect(() => {
+    fetchData();
+  }, []);
+
+  return (
+    <Grid item xs className={classes.gridContainer}>
+      <div className={classes.highlightBackground}>
+        <Grid container className={classes.body}>
+          <Grid item xs={12}>
+            <strong>Name:</strong> {taskID}
+          </Grid>
+          <Grid item xs={12}>
+            <strong>Status:</strong> {get(taskDebugData, 'taskState', '')}
+          </Grid>
+          <Grid item xs={12}>
+            <strong>Start Time:</strong> {get(taskDebugData, 'startTime', '')}
+          </Grid>
+          <Grid item xs={12}>
+            <strong>Elapsed Time:</strong> {get(taskDebugData, 'startTime') ? PinotMethodUtils.getElapsedTime(moment(get(taskDebugData, 'startTime'), 'YYYY-MM-DD hh:mm:ss')) : ''}
+          </Grid>
+          <Grid item xs={12}>
+            <strong>Finish Time:</strong> {get(taskDebugData, 'subtaskInfos.0.finishTime', '')}
+          </Grid>
+          <Grid item xs={12}>
+            <strong>Number of Sub Tasks:</strong> {get(taskDebugData, 'subtaskCount.total', '')}
+          </Grid>
+        </Grid>
+      </div>
+      <Grid container spacing={2}>
+        {/* <Grid item xs={6}>

Review Comment:
   If the commented out code is not required, let's remove it before merging.



-- 
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] [pinot] joshigaurava commented on a diff in pull request #8978: #8970 Minion tab in Pinot UI

Posted by GitBox <gi...@apache.org>.
joshigaurava commented on code in PR #8978:
URL: https://github.com/apache/pinot/pull/8978#discussion_r915064253


##########
pinot-controller/src/main/resources/app/pages/InstanceListingPage.tsx:
##########
@@ -63,6 +70,7 @@ const InstanceListingPage = () => {
   ) : (
     <Grid item xs className={classes.gridContainer}>
       <Instances instances={instances} clusterName={clusterName} />
+      {/* {isController && periodicTasks.content} */}

Review Comment:
   Is this going to be updated?



##########
pinot-controller/src/main/resources/app/pages/InstanceListingPage.tsx:
##########
@@ -42,6 +43,12 @@ const InstanceListingPage = () => {
   const [instances, setInstances] = useState<DataTable>();
   const [clusterName, setClusterName] = useState('');
 
+  const isController = !!_.get(instances, 'Controller', false);
+
+  // const periodicTasks = usePeriodicTasks({

Review Comment:
   Let's get rid of unused code.



-- 
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