You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by James Peach <jp...@apache.org> on 2018/08/31 16:47:16 UTC
Review Request 68587: Fixed fetcher deadlock with duplicate URIs.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68587/
-----------------------------------------------------------
Review request for mesos, Gilbert Song and Joseph Wu.
Bugs: MESOS-9172
https://issues.apache.org/jira/browse/MESOS-9172
Repository: mesos
Description
-------
If a fetch request contains duplicate URIs that are not already
in the cache, the fetcher would erroneously expect that some other
fetch process is going to download that cache entry. It will then
wait for a Future that will never complete.
The fix is to track whether the cache entry was created in this
fetch, and in that case to simply allow the duplicate URI. In
the fetcher, we check the cache before downloading so that a URIs
can be fetched to distinct output files without being downloaded
multiple times.
Diffs
-----
src/launcher/fetcher.cpp ef8d7ebdcf6d5d15459fd3392cb99774f078347a
src/slave/containerizer/fetcher.cpp 17f5388200c8341936cb4d7f8da67b5f286b727d
src/tests/fetcher_tests.cpp f3ea7092635c88b9dddcc2998a4c7350fb56110c
Diff: https://reviews.apache.org/r/68587/diff/1/
Testing
-------
make check (Fedora 28)
Thanks,
James Peach
Re: Review Request 68587: Fixed fetcher deadlock with duplicate URIs.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68587/#review208192
-----------------------------------------------------------
PASS: Mesos patch 68587 was successfully built and tested.
Reviews applied: `['68586', '68587']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2268/mesos-review-68587
- Mesos Reviewbot Windows
On Aug. 31, 2018, 4:47 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68587/
> -----------------------------------------------------------
>
> (Updated Aug. 31, 2018, 4:47 p.m.)
>
>
> Review request for mesos, Gilbert Song and Joseph Wu.
>
>
> Bugs: MESOS-9172
> https://issues.apache.org/jira/browse/MESOS-9172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If a fetch request contains duplicate URIs that are not already
> in the cache, the fetcher would erroneously expect that some other
> fetch process is going to download that cache entry. It will then
> wait for a Future that will never complete.
>
> The fix is to track whether the cache entry was created in this
> fetch, and in that case to simply allow the duplicate URI. In
> the fetcher, we check the cache before downloading so that a URIs
> can be fetched to distinct output files without being downloaded
> multiple times.
>
>
> Diffs
> -----
>
> src/launcher/fetcher.cpp ef8d7ebdcf6d5d15459fd3392cb99774f078347a
> src/slave/containerizer/fetcher.cpp 17f5388200c8341936cb4d7f8da67b5f286b727d
> src/tests/fetcher_tests.cpp f3ea7092635c88b9dddcc2998a4c7350fb56110c
>
>
> Diff: https://reviews.apache.org/r/68587/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68587: Fixed fetcher deadlock with duplicate URIs.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68587/
-----------------------------------------------------------
(Updated Oct. 17, 2018, 10:21 p.m.)
Review request for mesos, Gilbert Song and Joseph Wu.
Bugs: MESOS-9172
https://issues.apache.org/jira/browse/MESOS-9172
Repository: mesos
Description
-------
If a fetch request contains duplicate URIs that are not already
in the cache, the fetcher would erroneously expect that some other
fetch process is going to download that cache entry. It will then
wait for a Future that will never complete.
The fix is to track whether the cache entry was created in this
fetch, and in that case to simply allow the duplicate URI. In
the fetcher, we check the cache before downloading so that a URIs
can be fetched to distinct output files without being downloaded
multiple times.
Diffs (updated)
-----
src/launcher/fetcher.cpp ef8d7ebdcf6d5d15459fd3392cb99774f078347a
src/slave/containerizer/fetcher.cpp 17f5388200c8341936cb4d7f8da67b5f286b727d
src/tests/fetcher_tests.cpp f3ea7092635c88b9dddcc2998a4c7350fb56110c
Diff: https://reviews.apache.org/r/68587/diff/2/
Changes: https://reviews.apache.org/r/68587/diff/1-2/
Testing
-------
make check (Fedora 28)
Thanks,
James Peach
Re: Review Request 68587: Fixed fetcher deadlock with duplicate URIs.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68587/#review209704
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/containerizer/fetcher.cpp
Lines 409-411 (patched)
<https://reviews.apache.org/r/68587/#comment294254>
The crux here is that URIs with the same `value` but are different in other fields like `executable` or `extract` can lead to multiple items in `entries` pointing to the same cache entry. Add it to the comment?
src/slave/containerizer/fetcher.cpp
Lines 421 (patched)
<https://reviews.apache.org/r/68587/#comment294252>
Using `newEntries.at()` is more idiomatic even though we did check that the entry exists.
- Jiang Yan Xu
On Aug. 31, 2018, 9:47 a.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68587/
> -----------------------------------------------------------
>
> (Updated Aug. 31, 2018, 9:47 a.m.)
>
>
> Review request for mesos, Gilbert Song and Joseph Wu.
>
>
> Bugs: MESOS-9172
> https://issues.apache.org/jira/browse/MESOS-9172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If a fetch request contains duplicate URIs that are not already
> in the cache, the fetcher would erroneously expect that some other
> fetch process is going to download that cache entry. It will then
> wait for a Future that will never complete.
>
> The fix is to track whether the cache entry was created in this
> fetch, and in that case to simply allow the duplicate URI. In
> the fetcher, we check the cache before downloading so that a URIs
> can be fetched to distinct output files without being downloaded
> multiple times.
>
>
> Diffs
> -----
>
> src/launcher/fetcher.cpp ef8d7ebdcf6d5d15459fd3392cb99774f078347a
> src/slave/containerizer/fetcher.cpp 17f5388200c8341936cb4d7f8da67b5f286b727d
> src/tests/fetcher_tests.cpp f3ea7092635c88b9dddcc2998a4c7350fb56110c
>
>
> Diff: https://reviews.apache.org/r/68587/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>
Re: Review Request 68587: Fixed fetcher deadlock with duplicate URIs.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68587/#review208225
-----------------------------------------------------------
Patch looks great!
Reviews applied: [68586, 68587]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Aug. 31, 2018, 4:47 p.m., James Peach wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68587/
> -----------------------------------------------------------
>
> (Updated Aug. 31, 2018, 4:47 p.m.)
>
>
> Review request for mesos, Gilbert Song and Joseph Wu.
>
>
> Bugs: MESOS-9172
> https://issues.apache.org/jira/browse/MESOS-9172
>
>
> Repository: mesos
>
>
> Description
> -------
>
> If a fetch request contains duplicate URIs that are not already
> in the cache, the fetcher would erroneously expect that some other
> fetch process is going to download that cache entry. It will then
> wait for a Future that will never complete.
>
> The fix is to track whether the cache entry was created in this
> fetch, and in that case to simply allow the duplicate URI. In
> the fetcher, we check the cache before downloading so that a URIs
> can be fetched to distinct output files without being downloaded
> multiple times.
>
>
> Diffs
> -----
>
> src/launcher/fetcher.cpp ef8d7ebdcf6d5d15459fd3392cb99774f078347a
> src/slave/containerizer/fetcher.cpp 17f5388200c8341936cb4d7f8da67b5f286b727d
> src/tests/fetcher_tests.cpp f3ea7092635c88b9dddcc2998a4c7350fb56110c
>
>
> Diff: https://reviews.apache.org/r/68587/diff/1/
>
>
> Testing
> -------
>
> make check (Fedora 28)
>
>
> Thanks,
>
> James Peach
>
>