You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Adar Dembo (Code Review)" <ge...@cloudera.org> on 2018/08/07 23:27:32 UTC
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Hello Andrew Wong,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/11152
to review the following change.
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
KUDU-2528: thirdparty downloads should be more robust
Commit 37f3a95d8 added --retry to our curl invocations, but it turns out
that this only retries failures that curl deems to be "transient". And if
curl exits with a non-zero status, the entire script grinds to a halt
because of set -e.
This patch expands the class of retriable errors by parking the curl
statement in a pipeline, which means it is excused from set -e. After that,
the existing fetch_and_expand retry loop takes over.
Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
---
M thirdparty/download-thirdparty.sh
1 file changed, 6 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/11152/1
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11152 )
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 08 Aug 2018 05:01:49 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11152 )
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
KUDU-2528: thirdparty downloads should be more robust
Commit 37f3a95d8 added --retry to our curl invocations, but it turns out
that this only retries failures that curl deems to be "transient". And if
curl exits with a non-zero status, the entire script grinds to a halt
because of set -e.
This patch expands the class of retriable errors by parking the curl
statement in a pipeline, which means it is excused from set -e. After that,
the existing fetch_and_expand retry loop takes over.
Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Reviewed-on: http://gerrit.cloudera.org:8080/11152
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M thirdparty/download-thirdparty.sh
1 file changed, 10 insertions(+), 3 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 3
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11152 )
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 07 Aug 2018 23:57:41 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11152
to look at the new patch set (#2).
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
KUDU-2528: thirdparty downloads should be more robust
Commit 37f3a95d8 added --retry to our curl invocations, but it turns out
that this only retries failures that curl deems to be "transient". And if
curl exits with a non-zero status, the entire script grinds to a halt
because of set -e.
This patch expands the class of retriable errors by parking the curl
statement in a pipeline, which means it is excused from set -e. After that,
the existing fetch_and_expand retry loop takes over.
Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
---
M thirdparty/download-thirdparty.sh
1 file changed, 10 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/11152/2
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11152 )
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11152/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/11152/1/thirdparty/download-thirdparty.sh@76
PS1, Line 76: curl --retry 3 -L -O
> Does it make sense to add --fail option as well? IIRC in case if the serve
I'll remove the file just in case (though I wouldn't expect it to be there).
I don't think --fail makes sense though; it sounds like it yields net less information in the output, plus even if curl "succeeded" and produced a file containing an HTTP error code, we'll fail to unzip/untar it below and will retry anyway.
http://gerrit.cloudera.org:8080/#/c/11152/1/thirdparty/download-thirdparty.sh@78
PS1, Line 78: rm -f "$FILENAME"
> Does it make sense to pause between attempts here? In case of transient er
Sure, I'll throw in a pause.
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 08 Aug 2018 04:28:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/11152 )
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 2
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 08 Aug 2018 14:35:00 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2528: thirdparty downloads should be more robust
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11152 )
Change subject: KUDU-2528: thirdparty downloads should be more robust
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/11152/1/thirdparty/download-thirdparty.sh
File thirdparty/download-thirdparty.sh:
http://gerrit.cloudera.org:8080/#/c/11152/1/thirdparty/download-thirdparty.sh@76
PS1, Line 76: curl --retry 3 -L -O
Does it make sense to add --fail option as well? IIRC in case if the server returns HTTP error code, curl would return 0 masking the problem if run without --fail.
Also, does it make sense to remove the target file (i.e. $FILENAME) in case of an error? With --fail option, the curl would not create the target file in case of HTTP error, but if that were a low-level error then a partially downloaded file would be left.
http://gerrit.cloudera.org:8080/#/c/11152/1/thirdparty/download-thirdparty.sh@78
PS1, Line 78: continue
Does it make sense to pause between attempts here? In case of transient errors curl would do exponential back-off on its own while retrying, but in case of other errors I think we need to take care of that ourselves. I don't think we need some advanced back-off, but at least pause for 5 seconds or alike.
--
To view, visit http://gerrit.cloudera.org:8080/11152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic970afdb7ec23d3e04b2d7a9bf77d32774ca4d57
Gerrit-Change-Number: 11152
Gerrit-PatchSet: 1
Gerrit-Owner: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Wed, 08 Aug 2018 00:17:14 +0000
Gerrit-HasComments: Yes