[jira] Created: (HADOOP-1462) Better progress reporting from a Task

classic Classic list List threaded Threaded
25 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[jira] Created: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
Better progress reporting from a Task
-------------------------------------

                 Key: HADOOP-1462
                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
             Project: Hadoop
          Issue Type: Improvement
          Components: mapred
            Reporter: Vivek Ratan


The Task code that reports progress updates has the following problems:
1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.

These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.

I propose we make the following changes:

1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:

while (1) {
        if (progress_flag is set) {
                // report progress update
                umbilical::progress(...);
                if (failure), kill task;
                reset progress_flag;
                reset keep_alive_flag; // calling progress() also indicates that we're alive
        }
        else if (keep_alive_flag is set) {
                // let TT know we're alive
                umbilical::progress(same params as last time);
                if (failure), kill task;
                reset keep_alive_flag;
                break;
        }
        else {
                // see if TT is alive
                umbilical::ping();
                if (failure), kill task;
        }
        sleep (1 sec);
}

2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.

3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).

4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.

5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.

6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.

I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12501625 ]

Doug Cutting commented on HADOOP-1462:
--------------------------------------

Why distinguish between keepAlive and progress?  It looks like the actions are the same, so a single flag should suffice, no?  Or have I missed an important distinction?

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12501785 ]

Vivek Ratan commented on HADOOP-1462:
-------------------------------------

Yes, the progress_flag and keel_alive_flag can be combined into one. The thread should do the following:

{code}
while (1) {
  if (progress_flag is set) {
        // report progress update
        umbilical::progress(...);
        if (failure), kill task;
        reset progress_flag;
  }
  else {
        // see if TT is alive
        umbilical::ping();
        if (failure), kill task;
  }
  sleep (1 sec);
}
{code}


> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12501820 ]

Vivek Ratan commented on HADOOP-1462:
-------------------------------------

Some details on synchronization. Access to progress_flag will have to be synchronized, as it is set by MapReduce code throughout, and read&reset by the Progress thread. progress_flag can be a _volatile_ boolean, which will make each read or write atomic (rather than have synchronized setters and getters). However, the Progress thread really needs an atomic read&reset call - it wants to see if the flag is set and reset it as well. So it should probably be of type _AtomicBoolean_, which provides an atomic getAndSet() method. This should still be faster than using synchronized getter and setter methods.

The thread code should now look this:
{code}
while (1) {
  boolean current_progress_flag = progress_flag.getAndSet(false); // may wrap getAndSet() in a non-synchronized getter
  if (current_progress_flag) {
        // report progress update
        umbilical::progress(...);
        if (failure), kill task;
  }
  else {
        // see if TT is alive
        umbilical::ping();
        if (failure), kill task;
  }
  sleep (1 sec);
}
{code}


> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12501988 ]

Doug Cutting commented on HADOOP-1462:
--------------------------------------

This generally looks good to me.  If possible, I'd rather see the 'if (failure) kill task' logic in a single place rather than repeated.  Perhaps this can instead be in a 'try' block outside the 'if'?  Something like:
{code}
while (true) {
  try {
    boolean currentProgressFlag = progressFlag.getAndSet(false);
    if (currentProgressFlag) {
      umbilical::progress(...);
    } else {
      umbilical::ping();
    }
  } catch (IOException e) {
    kill task;
  }
  sleep (1 sec);
}
{code}

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Assigned: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan reassigned HADOOP-1462:
-----------------------------------

    Assignee: Vivek Ratan

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan updated HADOOP-1462:
--------------------------------

    Status: Patch Available  (was: Open)

This patch takes care of the problems listed in this issue:
- RPC calls do not block any more
- Minimal RPC calls are made
- Progress is reported only when it is actually made

In addition, the patch handles some issues reported in HADOOP-1431:
- we make sure we send keep-alive requests and report progress to TaskTracker when in the middle of potentially long-running jobs, which may involve user code. Specifically, progress is reported during the Map sort/merge phase and during the Reduce shuffle/merge phases ( I may have the terminology wrong).


> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan updated HADOOP-1462:
--------------------------------

    Attachment: 1462.patch

Patch file.

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>         Attachments: 1462.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Devaraj Das updated HADOOP-1462:
--------------------------------

    Fix Version/s: 0.14.0
           Status: Open  (was: Patch Available)

Looks good overall (much cleaner & efficient progress reporting!). I have a few comments:
1) The merge APIs needn't do a check for the value of Progressable being null. Instead, it could pass the Progressable object directly to the MergeQueue constructor and that could do progress reporting if it is not null.
2) TaskUmbilical doesn't do 'ping' anymore and hence that method can be removed.
3) In ReduceTask.java there is a direct call to setProgressFlag. This can be replaced with a call to reporter.progress()

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12503326 ]

Vivek Ratan commented on HADOOP-1462:
-------------------------------------

ping is used by the communication thread to ping the TT every sec.
The other two points are good. I've incorporated them. 1462_02.patch contains all the changes, including the ones Devaraj suggested.

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan updated HADOOP-1462:
--------------------------------

    Attachment: 1462_02.patch

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12503641 ]

Hadoop QA commented on HADOOP-1462:
-----------------------------------

+0, new Findbugs warnings

http://issues.apache.org/jira/secure/attachment/12359402/1462_02.patch
applied and successfully tested against trunk revision r546264,
but there appear to be new Findbugs warnings introduced by this patch.

New Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/265/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Test results:   http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/265/testReport/
Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/265/console

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12504142 ]

Devaraj Das commented on HADOOP-1462:
-------------------------------------

As we discussed offline, you need to add progress reporting for the ramfs merges too. The findbugs warnings should also be looked at. The URLs will have the description of what's causing the findbugs warning. Please have a look at those warnings and fix them if appropriate.

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan updated HADOOP-1462:
--------------------------------

    Attachment: 1462_03.patch

A new patch (1462_03.patch) is attached. I've added progress reporting for the ramfs merge, and also made some changes to handle the findbugs results.

The Findbugs warnings on invoking System.exit(): we call exit() when the TT does not have us in its list of IDs or when we fail three times in connecting to it. Same code as before, just moved around a little bit.

The warning about calling Thread.sleep() with a lock held: I cannot figure out why this warning comes about. This is code from before, and I can't see how any changes I made would affect it. I have run the unit tests and the patched jar has also been run on one of the large clusters and everything seems fine.

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch, 1462_03.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan updated HADOOP-1462:
--------------------------------

    Status: Patch Available  (was: Open)

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch, 1462_03.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12504658 ]

Hadoop QA commented on HADOOP-1462:
-----------------------------------

+0, new Findbugs warnings

http://issues.apache.org/jira/secure/attachment/12359666/1462_03.patch
applied and successfully tested against trunk revision r547159,
but there appear to be new Findbugs warnings introduced by this patch.

New Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/281/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
Test results:   http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/281/testReport/
Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/281/console

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch, 1462_03.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Commented: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

    [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12504806 ]

Devaraj Das commented on HADOOP-1462:
-------------------------------------

+1 (though I am not sure whether we have to eliminate the findbugs warnings)

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch, 1462_03.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Doug Cutting updated HADOOP-1462:
---------------------------------

    Status: Open  (was: Patch Available)

SequenceFile.Sorter#setProgessable has a few issues.  First, it's misspelled.  Second, it only works when merging, not when sorting.  Either it should be fixed to work when sorting too, or it should only be specifiable when merging, e.g., as a parameter to the merge method.

My preference would be to add support for it to sorting.  This isn't required for the present issue, but would be nice.  Note that sort might reasonably invoke progress less often, not in the comparator, but perhaps only once per flush, so this should be an easy change.


> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch, 1462_03.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan updated HADOOP-1462:
--------------------------------

    Attachment: 1462_04.patch

My bad about the misspelt function name. That has been corrected. I have also added progress reporting to the sort code in SequenceFile.Sorter, in between calls to sort and flush. Progress reporting can also be added in SortFile::SeqFileComparator::compare(), but I've kept it in SortPass:run() for now. See 1462_04.patch.

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch, 1462_03.patch, 1462_04.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply | Threaded
Open this post in threaded view
|

[jira] Updated: (HADOOP-1462) Better progress reporting from a Task

Markus Jelsma (Jira)
In reply to this post by Markus Jelsma (Jira)

     [ https://issues.apache.org/jira/browse/HADOOP-1462?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Vivek Ratan updated HADOOP-1462:
--------------------------------

    Attachment:     (was: 1462_04.patch)

> Better progress reporting from a Task
> -------------------------------------
>
>                 Key: HADOOP-1462
>                 URL: https://issues.apache.org/jira/browse/HADOOP-1462
>             Project: Hadoop
>          Issue Type: Improvement
>          Components: mapred
>            Reporter: Vivek Ratan
>            Assignee: Vivek Ratan
>             Fix For: 0.14.0
>
>         Attachments: 1462.patch, 1462_02.patch, 1462_03.patch
>
>
> The Task code that reports progress updates has the following problems:
> 1. Some RPC calls are blocking. For example, in MapRunner::run(), the call to RecordReader::next() can result in a blocking RPC call to the Task Tracker (TT) to report progress.
> 2. Some RPC calls are unnecessary. The Ping thread pings the TT once every second, while we also independently send progress updates every second. We don't, for example, need to ping the TT right after we send the progress update.
> 3. In some places, we spawn a thread to send progress updates (in MapOutputBuffer::collect(), for example). If our code gets stuck, the thread will continue sending updates to the TT and we will never be shut down.
> These issues, in some form or another, have been reported in HADOOP-1201 and HADOOP-1431.
> I propose we make the following changes:
> 1. In order to make the RPC calls non-blocking, we need a thread that calls TT. This thread, to be created early on, will make sure we make the most appropriate RPCs. It will have access to two flags: a progress flag that indicates that the Task has made progress since the last RPC, and a keep_alive flag that indicates that we need to let the TT know that we're alive. This thread will also handle pings. It's logic will be something like this:
> while (1) {
> if (progress_flag is set) {
> // report progress update
> umbilical::progress(...);
> if (failure), kill task;
> reset progress_flag;
> reset keep_alive_flag; // calling progress() also indicates that we're alive
> }
> else if (keep_alive_flag is set) {
> // let TT know we're alive
> umbilical::progress(same params as last time);
> if (failure), kill task;
> reset keep_alive_flag;
> break;
> }
> else {
> // see if TT is alive
> umbilical::ping();
> if (failure), kill task;
> }
> sleep (1 sec);
> }
> 2. progress_flag and keep_alive_flag are set by the MapReduce code. Reporter::progress() (in Task.java) sets keep_alive_flag while progress_flag is set whenever Task's taskProgress object has any of its fields changed.
> 3. We do away with Task::reportProgress() as this code is now handled in the Progress thread. Wherever this method is called in our MapReduce kernel code, we should replace it either with Reporter::progress() (if the intent was to let TT know that we're alive) or we simply remove that call (if the intent was to transmit progress changes to the TT).
> 4. TaskUmbilicalProtocol::progress() should return a boolean, and should return the same values that TaskUmbilicalProtocol::ping() does. This will let the Task know whether its ID is known to the TT.
> 5. We no longer need to create a ping thread in TaskTracker::Child. However, we can perhaps create the Progress thread in the same place the Ping thread was created.
> 6. We will need to remove code that creates progress threads. This is in MapTask::MapOutputBuffer::collect(), MapTask::MapOutputBuffer::flush(), and ReduceTask::ReduceCopier(), at the least. Instead, we will need to add code that updates the progress or calls Reporter::progress(). Any of these calls simply update flags. so there's not a lot of performance penalty (at worst, updating progress_flag or keep_alive_flag may need to be done within a synchronized block, but even that may not be necessary since the flags are just boolean values). As per HADOOP-1431, these calls can be made through a ReportingComparator, or from within the generic BuferSorter, or perhaps from some place better.
> I may have missed out on some details, but hopefully the overall idea is clear. Comments welcome.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

12