cf-natali commented on a change in pull request #388:
URL: https://github.com/apache/mesos/pull/388#discussion_r660049235
##########
File path: src/linux/cgroups.cpp
##########
@@ -1425,16 +1424,15 @@ class TasksKiller : public Process<TasksKiller>
const PID<TasksKiller>& pid)
{
// Cancel the freeze operation.
- // TODO(jieyu): Wait until 'future' is in DISCARDED state before
- // starting retry.
future.discard();
- // We attempt to kill the processes before we thaw again,
- // due to a bug in the kernel. See MESOS-1758 for more details.
- // We thaw the cgroup before trying to freeze again to allow any
- // pending signals to be delivered. See MESOS-1689 for details.
- // This is a short term hack until we have PID namespace support.
- return Future<bool>(true)
+ // Wait until the freeze is cancelled, and then attempt to kill the
+ // processes before we thaw again, due to a bug in the kernel. See
+ // MESOS-1758 for more details. We thaw the cgroup before trying to freeze
+ // again to allow any pending signals to be delivered. See MESOS-1689 for
+ // details. This is a short term hack until we have PID namespace support.
+ return future
+ .recover([](const Future<Nothing>&){ return Future<Nothing>(Nothing());
})
Review comment:
Yes, I would assume so.
##########
File path: src/linux/cgroups.cpp
##########
@@ -1425,16 +1424,15 @@ class TasksKiller : public Process<TasksKiller>
const PID<TasksKiller>& pid)
{
// Cancel the freeze operation.
- // TODO(jieyu): Wait until 'future' is in DISCARDED state before
- // starting retry.
future.discard();
- // We attempt to kill the processes before we thaw again,
- // due to a bug in the kernel. See MESOS-1758 for more details.
- // We thaw the cgroup before trying to freeze again to allow any
- // pending signals to be delivered. See MESOS-1689 for details.
- // This is a short term hack until we have PID namespace support.
- return Future<bool>(true)
+ // Wait until the freeze is cancelled, and then attempt to kill the
+ // processes before we thaw again, due to a bug in the kernel. See
+ // MESOS-1758 for more details. We thaw the cgroup before trying to freeze
+ // again to allow any pending signals to be delivered. See MESOS-1689 for
+ // details. This is a short term hack until we have PID namespace support.
+ return future
+ .recover([](const Future<Nothing>&){ return Future<Nothing>(Nothing());
})
Review comment:
See this example:
```
cf@thinkpad:~/src/test/mesos$ cat test_process.cpp
#include <process/process.hpp>
using namespace process;
int
main(int argc, char* argv[])
{
auto promise = Promise<Nothing>();
auto future = promise.future();
future
.recover([](const Future<Nothing>& future) -> Future<Nothing> {
std::cout << "recover" << std::endl;
return Nothing();
})
.then([](const Future<Nothing>& future) {
std::cout << "then" << std::endl;
return Nothing();
});
promise.discard();
future.await();
}
cf@thinkpad:~/src/test/mesos$ /tmp/test_process
recover
then
cf@thinkpad:~/src/test/mesos$
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
