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/09/23 18:34:21 UTC

[GitHub] [pinot] klsince opened a new pull request, #9457: track progress from within segment processor framework

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

   Follow up on #9449 to get finer grained progress status from SegmentProcessorFramework as well, as that's usually the step takes most of task time, breaking it down to map/reduce/generation steps for 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.

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] zhtaoxiang commented on a diff in pull request #9457: track progress from within segment processor framework

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


##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/mapper/SegmentMapper.java:
##########
@@ -105,8 +108,12 @@ public SegmentMapper(List<RecordReader> recordReaders, SegmentProcessorConfig pr
    */
   public Map<String, GenericRowFileManager> map()
       throws Exception {
+    Consumer<Object> observer = _processorConfig.getProgressObserver();
+    int totalCount = _recordReaders.size();
+    int count = 1;
     GenericRow reuse = new GenericRow();
     for (RecordReader recordReader : _recordReaders) {
+      observer.accept(String.format("Doing map phase on data from RecordReader: %d/%d", count++, totalCount));

Review Comment:
   It seems to me that it's hard to get the meaning of those two numbers. Maybe change it to `RecordReader %d (%d in total)` or something similar?



-- 
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] klsince commented on pull request #9457: track progress from within segment processor framework

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

   e.g. for those progress status from SegmentProcessorFramework: 
   ```
   {
         "ts": 1663957811428,
         "status": "Doing map phase on data from RecordReader: 1/3"
       },
       {
         "ts": 1663957811434,
         "status": "Doing map phase on data from RecordReader: 2/3"
       },
       {
         "ts": 1663957811434,
         "status": "Doing map phase on data from RecordReader: 3/3"
       },
       {
         "ts": 1663957811434,
         "status": "Doing reduce phase on data from partition: 0, 1/1"
       },
       {
         "ts": 1663957811440,
         "status": "Creating segment of sequentId: 0 with data from partition: 0 in row range: [0, 3)/3"
       },
   ```


-- 
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] klsince commented on a diff in pull request #9457: track progress from within segment processor framework

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


##########
pinot-minion/src/main/java/org/apache/pinot/minion/taskfactory/TaskFactoryRegistry.java:
##########
@@ -127,17 +127,17 @@ private TaskResult runInternal() {
                 _eventObserver.notifyTaskCancelled(pinotTaskConfig);
                 _minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_CANCELLED, 1L);
                 LOGGER.info("Task: {} got cancelled", _taskConfig.getId(), e);
-                return new TaskResult(TaskResult.Status.CANCELED, e.toString());
+                return new TaskResult(TaskResult.Status.CANCELED, StringUtil.getStackTraceAsString(e));
               } catch (FatalException e) {
                 _eventObserver.notifyTaskError(pinotTaskConfig, e);
                 _minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_FATAL_FAILED, 1L);
                 LOGGER.error("Caught fatal exception while executing task: {}", _taskConfig.getId(), e);
-                return new TaskResult(TaskResult.Status.FATAL_FAILED, e.toString());
+                return new TaskResult(TaskResult.Status.FATAL_FAILED, StringUtil.getStackTraceAsString(e));
               } catch (Exception e) {
                 _eventObserver.notifyTaskError(pinotTaskConfig, e);
                 _minionMetrics.addMeteredTableValue(taskType, MinionMeter.NUMBER_TASKS_FAILED, 1L);
                 LOGGER.error("Caught exception while executing task: {}", _taskConfig.getId(), e);
-                return new TaskResult(TaskResult.Status.FAILED, e.toString());
+                return new TaskResult(TaskResult.Status.FAILED, StringUtil.getStackTraceAsString(e));

Review Comment:
   e.toString() like e.getMessage() just returns the error msg (basically the first line of the stack trace).



-- 
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] xiangfu0 closed pull request #9457: track progress from within segment processor framework

Posted by GitBox <gi...@apache.org>.
xiangfu0 closed pull request #9457: track progress from within segment processor framework
URL: https://github.com/apache/pinot/pull/9457


-- 
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] klsince commented on a diff in pull request #9457: track progress from within segment processor framework

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


##########
pinot-core/src/main/java/org/apache/pinot/core/segment/processing/mapper/SegmentMapper.java:
##########
@@ -105,8 +108,12 @@ public SegmentMapper(List<RecordReader> recordReaders, SegmentProcessorConfig pr
    */
   public Map<String, GenericRowFileManager> map()
       throws Exception {
+    Consumer<Object> observer = _processorConfig.getProgressObserver();
+    int totalCount = _recordReaders.size();
+    int count = 1;
     GenericRow reuse = new GenericRow();
     for (RecordReader recordReader : _recordReaders) {
+      observer.accept(String.format("Doing map phase on data from RecordReader: %d/%d", count++, totalCount));

Review Comment:
   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] [pinot] npawar merged pull request #9457: track progress from within segment processor framework

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


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