You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2018/06/19 22:32:39 UTC
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Hello Henry Robinson, Impala Public Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/10758
to review the following change.
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/cache.cc
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/env_posix.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/locks.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M be/src/kudu/util/net/socket.cc
M be/src/kudu/util/subprocess.cc
11 files changed, 124 insertions(+), 40 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/1
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.
The original commit message is below:
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/10758
Reviewed-by: Michael Ho <kw...@cloudera.com>
Tested-by: Lars Volker <lv...@cloudera.com>
---
M CMakeLists.txt
M LICENSE.txt
M be/src/common/kudu_version.cc
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M bin/rat_exclude_files.txt
10 files changed, 150 insertions(+), 32 deletions(-)
Approvals:
Michael Ho: Looks good to me, approved
Lars Volker: Verified
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 13
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h
File be/src/kudu/util/kudu_export.h:
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h@17
PS9, Line 17:
Is this file auto-generated during build on kudu side ?
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 17:41:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has removed Henry Robinson from this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Removed reviewer Henry Robinson.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 7:
This should be ready for review now.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 20:14:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 12: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 12
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Jul 2018 21:35:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/CMakeLists.txt
File be/src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/CMakeLists.txt@213
PS1, Line 213: if(NOT NO_NVM_SUPPORT)
> After this change we should probably start continuously cherry-picking chan
It seems that the original code is conflating NO_NVM_SUPPORT with NO_APPLE.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Jun 2018 17:39:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h
File be/src/kudu/util/kudu_export.h:
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h@17
PS9, Line 17:
> No, I this is just manually added by us.
Sorry, I may not have been clear. I noticed that this file doesn't exist in the Kudu code base either but it exists in various build directories. I didn't dig too much into the mechanism but I am just wondering if you happen to know why we need to copy it manually.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 19:36:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#11).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.
The original commit message is below:
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M LICENSE.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M bin/rat_exclude_files.txt
9 files changed, 143 insertions(+), 32 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/11
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 11
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h
File be/src/kudu/util/kudu_export.h:
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h@17
PS9, Line 17:
> Is this file auto-generated during build on kudu side ?
No, I this is just manually added by us.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 17:45:30 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#10).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.
The original commit message is below:
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
7 files changed, 79 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/10
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#8).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.
The original commit message is below:
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
7 files changed, 75 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/8
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 8
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has removed Henry Robinson from this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Removed reviewer Henry Robinson.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/7/be/src/kudu/util/kudu_export.h
File be/src/kudu/util/kudu_export.h:
PS7:
Nothing in this file has changed.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Jul 2018 18:42:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/CMakeLists.txt
File be/src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/CMakeLists.txt@213
PS1, Line 213: if(NOT NO_NVM_SUPPORT)
> Can this be applied to Kudu code base so we don't have to reapply this patc
After this change we should probably start continuously cherry-picking changes from Kudu instead of waiting for things to get bad again.
This flag was added only for us and is manually set in the top-level CMakeLists.txt to remove support for libvmem (together with NO_TESTS). Do you think Kudu would have a use for having such a flag themselves?
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Fri, 22 Jun 2018 00:48:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 9: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h
File be/src/kudu/util/kudu_export.h:
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h@17
PS9, Line 17:
> I understand. The file seems to be generated by Kudu's build through src/ku
A note would be good but feel free to merge it as-is. Just checking to see if we somehow auto-generate it on Impala side too. Guess not.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 01:52:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 10: Code-Review+2
Rebased and addressed the final comment. Carrying Michael's +2.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 10
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 22:04:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h
File be/src/kudu/util/kudu_export.h:
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h@17
PS9, Line 17:
> Sorry, I may not have been clear. I noticed that this file doesn't exist in
I understand. The file seems to be generated by Kudu's build through src/kudu/client/CMakeLists.txt. Do you think we should add a note to our copy to indicate this?
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jul 2018 19:55:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 12: Code-Review+2
I looked at the diff between v10 and v12.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 12
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Jul 2018 20:43:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#12).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.
The original commit message is below:
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M LICENSE.txt
M be/src/common/kudu_version.cc
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
M bin/rat_exclude_files.txt
10 files changed, 150 insertions(+), 32 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/12
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 12
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#9).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
NOTE: This commit is part of a set of changes for IMPALA-7006. It
contains pieces of a previous commit that need to be cherry picked
again after rebasing the code in be/src/kudu/{util,security,rpc}.
The original commit message is below:
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
7 files changed, 75 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/9
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#7).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
7 files changed, 75 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/7
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 7
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#2).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/env_posix.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
8 files changed, 77 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/2
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 2
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#6).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/env_posix.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
8 files changed, 76 insertions(+), 32 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/6
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 6
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Henry Robinson, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10758
to look at the new patch set (#3).
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
IMPALA-4669: [KUTIL] Add kudu_util library to the build.
A few miscellaneous changes to allow kudu_util to compile with Impala.
Add kudu_version.cc to substitute for the version.cc file that is
automatically built during the full Kudu build.
Set LZ4_DISABLE_DEPRECATE_WARNINGS to allow Kudu's compressor utility to
use deprecated names for LZ4 methods.
Add NO_NVM_SUPPORT flag to Kudu build (plan to upstream this later) to
disable building with nvm support, removing a library dependency.
Also remove imported FindOpenSSL.cmake in favour of the standard one provided
by cmake itself.
Finally, a few changes to allow compilation on RHEL5:
* Only use sched_getcpu() if supported
* Only include magic.h if available
* Workaround for kernels that don't have SOCK_NONBLOCK
* Workaround for kernels that don't have O_CLOEXEC (ignore the flag)
* Provide non-working implementation of fallocate()
* Disable inclusion of linux/fiemap.h - although this exists on RHEL5,
it does not compile due to other #includes in env_posix.cc. We disable
the path this is used for, since Impala does not call that code.
* Use Kudu's implementation of pipe(2), preadv(2) and pwritev(2) where
it doesn't exist.
In most cases these changes simply force kutil to revert to a different
implementation that was already written for OSX support - this patch
generalises the logic to provide the implementation whenever the
required function doesn't exist.
This patch compiles on RHEL5.5 and 6.0, SLES11 and 12, Ubuntu 12.04 and
14.04 and Debian 7.0 and 8.0.
Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Reviewed-on: http://gerrit.cloudera.org:8080/5715
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M CMakeLists.txt
M be/src/kudu/util/CMakeLists.txt
M be/src/kudu/util/compression/compression_codec.cc
M be/src/kudu/util/env_posix.cc
M be/src/kudu/util/flags.cc
A be/src/kudu/util/kudu_export.h
M be/src/kudu/util/logging.cc
M be/src/kudu/util/minidump.cc
8 files changed, 77 insertions(+), 33 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/10758/3
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 3
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h
File be/src/kudu/util/kudu_export.h:
http://gerrit.cloudera.org:8080/#/c/10758/9/be/src/kudu/util/kudu_export.h@17
PS9, Line 17:
> A note would be good but feel free to merge it as-is. Just checking to see
I will push a note in a final rebase once the last change has been reviewed, since it will require me to carry all +2s.
// This file is generated during Kudu's build. Instead of recreating the necessary steps
// in Impala's build process, we copy it into our repository. See
// kudu/client/CMakeLists.txt in Kudu's repository for details.
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 9
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 05:58:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4669: [KUTIL] Add kudu util library to the build.
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/10758 )
Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build.
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/CMakeLists.txt
File be/src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/CMakeLists.txt@213
PS1, Line 213: if(NOT NO_NVM_SUPPORT)
Can this be applied to Kudu code base so we don't have to reapply this patch every time ? Same below.
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/cache.cc
File be/src/kudu/util/cache.cc:
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/cache.cc@563
PS1, Line 563: !defined(NO_NVM_SUPPORT)
Please see comments in CMakeList.txt.
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/locks.h
File be/src/kudu/util/locks.h:
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/locks.h@183
PS1, Line 183: !defined(HAVE_SCHED_GETCPU)
Can HAVE_SCHED_GETCPU be added to Kudu code base and remove defined(__APPLE__) branch of the #if statement ?
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/net/socket.cc
File be/src/kudu/util/net/socket.cc:
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/net/socket.cc@131
PS1, Line 131: && defined(SOCK_NONBLOCK) && defined(SOCK_CLOEXEC)
This also seems applicable to Kudu too and makes the build more robust across different platforms.
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/subprocess.cc
File be/src/kudu/util/subprocess.cc:
http://gerrit.cloudera.org:8080/#/c/10758/1/be/src/kudu/util/subprocess.cc@298
PS1, Line 298: !defined(HAVE_PIPE2)
Also applicable to Kudu code base. May be we can replace (__APPLE__) with (HAVE_PIPE2).
--
To view, visit http://gerrit.cloudera.org:8080/10758
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I451f02d3e4669e8a548b92fb1445cb2b322659a2
Gerrit-Change-Number: 10758
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Jun 2018 22:57:57 +0000
Gerrit-HasComments: Yes