[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch 1.4.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 8f0d3d0857b13924f379b97b9fe4b229f2d5d301
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index 7be9396..5be7572 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -573,6 +573,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -586,8 +593,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const list>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -612,9 +617,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch 1.5.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 4c72e6098fdf2b38e79954c03385bb0ecacbb489
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index 619aecb..bc776b4 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -619,6 +619,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -632,8 +639,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const list>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -658,9 +663,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch 1.6.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit b0a57116c6794f5d0036ed9c3668f27f29155bd7
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index 166e3f4..e73bad1 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -627,6 +627,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -640,8 +647,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const list>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -666,9 +671,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 819b9d8345e701321067f3b14ad2bb78b60d285c
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index c924dde..d2d741a 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -646,6 +646,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -659,8 +666,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const vector>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -685,9 +690,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch 1.8.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit cdd3e2924596eecf605eeb73e9c57f23f6643936
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index 40119d9..c0c689d 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -641,6 +641,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -654,8 +661,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const vector>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -680,9 +685,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch 1.9.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit dcce73d57b4d8866fedb3f287d978a135616afb3
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index e4a19c4..6545eaa 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -658,6 +658,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -671,8 +678,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const vector>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -697,9 +702,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch 1.10.x
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 97251a90d3336bd628c82becca00f545d95b01aa
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index c547696..2f2f624 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -658,6 +658,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -671,8 +678,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const vector>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -697,9 +702,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
[mesos] 01/02: Erased `Info` struct before unmouting volumes in Docker volume isolator.
This is an automated email from the ASF dual-hosted git repository.
qianzhang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 2845330fbd78a80fb7e71c6101724655fa254392
Author: Qian Zhang
AuthorDate: Fri May 15 10:23:51 2020 +0800
Erased `Info` struct before unmouting volumes in Docker volume isolator.
Currently when `DockerVolumeIsolatorProcess::cleanup()` is called, we will
unmount the volume first, and if the unmount operation fails we will NOT
erase the container's `Info` struct from `infos`. This is problematic
because the remaining `Info` in `infos` will cause the reference count of
the volume is greater than 0, but actually the volume is not being used by
any containers. That means we may never get a chance to unmount this volume
on this agent, furthermore if it is an EBS volume, it cannot be used by any
tasks launched on any other agents since a EBS volume can only be attached
to one node at a time. The only workaround would manually unmount the
volume.
So in this patch `DockerVolumeIsolatorProcess::cleanup()` is updated to
erase
container's `Info` struct before unmounting volumes.
Review: https://reviews.apache.org/r/72516
---
.../containerizer/mesos/isolators/docker/volume/isolator.cpp | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
index c547696..2f2f624 100644
--- a/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
+++ b/src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp
@@ -658,6 +658,13 @@ Future DockerVolumeIsolatorProcess::cleanup(
futures.push_back(this->unmount(volume.driver(), volume.name()));
}
+ // Erase the `Info` struct of this container before unmounting the volumes.
+ // This is to ensure the reference count of the volume will not be wrongly
+ // increased if unmounting volumes fail, otherwise next time when another
+ // container using the same volume is destroyed, we would NOT unmount the
+ // volume since its reference count would be larger than 1.
+ infos.erase(containerId);
+
return await(futures)
.then(defer(
PID(this),
@@ -671,8 +678,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
const ContainerID& containerId,
const vector>& futures)
{
- CHECK(infos.contains(containerId));
-
vector messages;
foreach (const Future& future, futures) {
if (!future.isReady()) {
@@ -697,9 +702,6 @@ Future DockerVolumeIsolatorProcess::_cleanup(
LOG(INFO) << "Removed the checkpoint directory at '" << containerDir
<< "' for container " << containerId;
- // Remove all this container's docker volume information from infos.
- infos.erase(containerId);
-
return Nothing();
}
