You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2022/07/06 16:05:42 UTC
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18708
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
IMPALA-11419: Incremental build is broken
IMPALA-11384 broke incremental builds because:
* added a custom target that always rewrites a few generated
header files
** The files are getting included directly/indirectly in most files,
so we always need to recompile a large part of the project
* Didn't remove ${THRIFT_FILE_WE}_constants.cpp/h dependency from
common/thrift/CMakeLists.txt
** These files are not generated anymore, so the build system always
reconstruct all the generated files (because *_constant.cpp/h is
always missing), and then builds every target that depend on them.
IMPALA-11415 fixes a sporadic error during data loading, but it only
covers the root cause, i.e. unnecessary recreation of thrift files.
This patch reverts IMPALA-11415 because:
* to make data-load more parallel
* to not cover similar issues in the future
Testing
* Tested locally that the thrift files are not getting regenerated
Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
---
A bin/cmake_aux/add_override.sh
M common/thrift/CMakeLists.txt
M testdata/bin/create-load-data.sh
3 files changed, 40 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/18708/1
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8302/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Jul 2022 13:58:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Hello Laszlo Gaal (Cloudera), Riza Suminto, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18708
to look at the new patch set (#2).
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
IMPALA-11419: Incremental build is broken
IMPALA-11384 broke incremental builds because:
* added a custom target that always rewrites a few generated
header files
** The files are getting included directly/indirectly in most files,
so we always need to recompile a large part of the project
* Didn't remove ${THRIFT_FILE_WE}_constants.cpp/h dependency from
common/thrift/CMakeLists.txt
** These files are not generated anymore, so the build system always
reconstruct all the generated files (because *_constant.cpp/h is
always missing), and then builds every target that depend on them.
IMPALA-11415 fixes a sporadic error during data loading, but it only
hides the root cause, i.e. unnecessary recreation of thrift files.
This patch reverts IMPALA-11415 because:
* to make data-load more parallel
* to not hide similar issues in the future
Testing
* Tested locally that the thrift files are not getting regenerated
Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
---
A bin/cmake_aux/add_override.sh
M common/thrift/CMakeLists.txt
M testdata/bin/create-load-data.sh
3 files changed, 40 insertions(+), 12 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/18708/2
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Jul 2022 18:40:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
Patch Set 1: Code-Review+1
This looks good to me. Thanks Zoltan for fixing this issue!
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jul 2022 16:46:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10935/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jul 2022 16:25:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Laszlo Gaal (Cloudera) (Code Review)" <ge...@cloudera.org>.
Laszlo Gaal (Cloudera) has posted comments on this change. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
There's a suggestion for the commit message, otherwise the change LGTM. Feel free to carry +2 and submit after considering the edit change.
Thanks a lot for catching this!
http://gerrit.cloudera.org:8080/#/c/18708/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18708/1//COMMIT_MSG@21
PS1, Line 21: covers
nit: suggest rephrasing this as "hide" instead of "cover": I'm afraid "cover" might be misunderstood here as "takes care of" instead of the intended meaning of hiding the real root cause.
http://gerrit.cloudera.org:8080/#/c/18708/1//COMMIT_MSG@25
PS1, Line 25: cover
nit: same here
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Jul 2022 13:43:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Jul 2022 13:58:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
IMPALA-11419: Incremental build is broken
IMPALA-11384 broke incremental builds because:
* added a custom target that always rewrites a few generated
header files
** The files are getting included directly/indirectly in most files,
so we always need to recompile a large part of the project
* Didn't remove ${THRIFT_FILE_WE}_constants.cpp/h dependency from
common/thrift/CMakeLists.txt
** These files are not generated anymore, so the build system always
reconstruct all the generated files (because *_constant.cpp/h is
always missing), and then builds every target that depend on them.
IMPALA-11415 fixes a sporadic error during data loading, but it only
hides the root cause, i.e. unnecessary recreation of thrift files.
This patch reverts IMPALA-11415 because:
* to make data-load more parallel
* to not hide similar issues in the future
Testing
* Tested locally that the thrift files are not getting regenerated
Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Reviewed-on: http://gerrit.cloudera.org:8080/18708
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
A bin/cmake_aux/add_override.sh
M common/thrift/CMakeLists.txt
M testdata/bin/create-load-data.sh
3 files changed, 40 insertions(+), 12 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11419: Incremental build is broken
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18708 )
Change subject: IMPALA-11419: Incremental build is broken
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
Carry +2
http://gerrit.cloudera.org:8080/#/c/18708/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18708/1//COMMIT_MSG@21
PS1, Line 21: hides
> nit: suggest rephrasing this as "hide" instead of "cover": I'm afraid "cove
true, thanks for pointing this out.
http://gerrit.cloudera.org:8080/#/c/18708/1//COMMIT_MSG@25
PS1, Line 25: hide
> nit: same here
Done
--
To view, visit http://gerrit.cloudera.org:8080/18708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ieb0e2007f3fa0cc721bd7b272956ce206ac65b0e
Gerrit-Change-Number: 18708
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Laszlo Gaal (Cloudera) <la...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Jul 2022 13:56:30 +0000
Gerrit-HasComments: Yes