You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Henry Robinson (Code Review)" <ge...@cloudera.org> on 2017/01/12 01:28:31 UTC

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil 

Henry Robinson has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/5688

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................

IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors.

* Provide our own implementation of since-removed MonotonicNanos()
* Provide our own implementation of since-removed
  Barrier_CompareAndSwap().
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Comment out non-compiling deprecated function definitions in numbers.h
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
---
M be/src/common/atomic.h
M be/src/exec/hash-table.inline.h
M be/src/gutil/macros.h
M be/src/gutil/strings/numbers.h
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
10 files changed, 55 insertions(+), 40 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5688/1
-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 7: Code-Review+1

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 7:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/416/

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has uploaded a new patch set (#6).

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................

IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors.

* Provide our own implementation of since-removed MonotonicNanos()
* Provide our own implementation of since-removed
  Barrier_CompareAndSwap().
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Comment out non-compiling deprecated function definitions in numbers.h
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
---
M be/src/common/atomic.h
M be/src/exec/hash-table.inline.h
M be/src/gutil/macros.h
M be/src/gutil/strings/numbers.h
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.h
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
11 files changed, 91 insertions(+), 44 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5688/6
-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has uploaded a new patch set (#5).

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................

IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors.

* Provide our own implementation of since-removed MonotonicNanos()
* Provide our own implementation of since-removed
  Barrier_CompareAndSwap().
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Comment out non-compiling deprecated function definitions in numbers.h
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
---
M be/src/common/atomic.h
M be/src/exec/hash-table.inline.h
M be/src/gutil/macros.h
M be/src/gutil/strings/numbers.h
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.h
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
11 files changed, 91 insertions(+), 44 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5688/5
-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5688/4/be/src/common/atomic.h
File be/src/common/atomic.h:

Line 103:     base::subtle::MemoryBarrier();
Dan Hecht - you might want to take a look at this (Barrier_CompareAndSwap() has been removed from gutil).


-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 6: Code-Review+1

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5688/6/be/src/util/time.h
File be/src/util/time.h:

PS6, Line 31: 1000000000ll
nit: Maybe 1'000'000'000LL?


PS6, Line 40: 1e9
NANOS_PER_SEC?


-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 7: Verified-1

Build failed: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/416/

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, Jim Apple, Tim Armstrong,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5688

to look at the new patch set (#8).

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................

IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors.

* Provide our own implementation of since-removed MonotonicNanos()
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Comment out non-compiling deprecated function definitions in numbers.h
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
---
M be/src/exec/hash-table.inline.h
M be/src/gutil/macros.h
M be/src/gutil/strings/numbers.h
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream-v2-test.cc
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.h
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
12 files changed, 91 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5688/8
-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 9: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5688/9/be/src/util/time.h
File be/src/util/time.h:

Line 36:   return ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
are you sure impala doesn't have a commit that introduced GetMonoTimeNanos() that we should just cherry-pick?  It might have been done that way to get it working on macos. If we don't, this is fine (though I don't see why kudu would have deleted that code).


http://gerrit.cloudera.org:8080/#/c/5688/9/cmake_modules/kudu_cmake_fns.txt
File cmake_modules/kudu_cmake_fns.txt:

Line 64:   endif()
what do these two changes do and why are they needed now?


-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has uploaded a new patch set (#4).

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................

IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors.

* Provide our own implementation of since-removed MonotonicNanos()
* Provide our own implementation of since-removed
  Barrier_CompareAndSwap().
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Comment out non-compiling deprecated function definitions in numbers.h
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
---
M be/src/common/atomic.h
M be/src/exec/hash-table.inline.h
M be/src/gutil/macros.h
M be/src/gutil/strings/numbers.h
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
10 files changed, 56 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5688/4
-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5688/9//COMMIT_MSG
Commit Message:

Line 32: a7c3f30 - Remove AMD Opteron Rev E workaround from atomicops
probably worth our taking this patch into kudu gutil as well


-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 9: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 6: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5688/6/be/src/common/atomic.h
File be/src/common/atomic.h:

Line 104:     return base::subtle::Release_CompareAndSwap(&value_, old_val, new_val) == old_val;
It looks like Barrier_CompareAndSwap was added by Dan to gutils in the below commit. I'm not sure of the subtleties here, so I'll defer to him about whether it's equivalent.

 commit 152c586f398269856169c271c771484793fdfee9
Author: Dan Hecht <dh...@cloudera.com>
Date:   Wed Feb 17 16:33:35 2016 -0800

    Improve AtomicInt abstraction and implementation


-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 8: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 8: Code-Review+1

Rebase and fix a test-compilation bug.

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 8:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/417/

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 8: Code-Review+2

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, Jim Apple, Dan Hecht, Tim Armstrong,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5688

to look at the new patch set (#9).

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................

IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors. Future upgrades to gutil/ should redo the work in
this commit.

* Remove kudu/ include prefix with command:

git grep -l "include \"kudu/" | xargs sed -i 's/include \"kudu\//include
\"/g'

* Change KUDU_GUTIL_* guards to be GUTIL_*

git grep -l KUDU_GUTIL | xargs sed -i 's/KUDU_GUTIL/GUTIL/g'

* Replace glog/logging.h with common/logging.h

git grep -l "glog/logging" | xargs sed -i 's/glog\/logging/common\/logging/g'

* Provide our own implementation of since-removed MonotonicNanos()
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Replay overwritten parts of following commits:

a7c3f30 - Remove AMD Opteron Rev E workaround from atomicops
54194af - IMPALA-4631: don't use floating point operations for time unit
conversions
152c586 - Improve AtomicInt abstraction and implementation

* Comment out non-compiling deprecated function definitions in numbers.h
* Overwrite changes from 92fafa "Use more efficient gutil implementation
  of Log2Ceiling" in favour of implementing them in Impala code only.
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
---
M be/src/exec/hash-table.inline.h
M be/src/gutil/atomic_refcount.h
M be/src/gutil/atomicops-internals-x86.cc
M be/src/gutil/atomicops-internals-x86.h
M be/src/gutil/atomicops.h
M be/src/gutil/auxiliary/atomicops-internals-arm-generic.h
M be/src/gutil/auxiliary/atomicops-internals-arm-v6plus.h
M be/src/gutil/auxiliary/atomicops-internals-windows.h
M be/src/gutil/basictypes.h
M be/src/gutil/bind.h
M be/src/gutil/bind_helpers.h
M be/src/gutil/bind_internal.h
M be/src/gutil/bits.cc
M be/src/gutil/bits.h
M be/src/gutil/callback.h
M be/src/gutil/callback_forward.h
M be/src/gutil/callback_internal.cc
M be/src/gutil/callback_internal.h
M be/src/gutil/casts.h
M be/src/gutil/charmap.h
M be/src/gutil/cpu.cc
M be/src/gutil/cycleclock-inl.h
M be/src/gutil/endian.h
M be/src/gutil/fixedarray.h
M be/src/gutil/gscoped_ptr.h
M be/src/gutil/hash/builtin_type_hash.h
M be/src/gutil/hash/city.cc
M be/src/gutil/hash/city.h
M be/src/gutil/hash/hash.cc
M be/src/gutil/hash/hash.h
M be/src/gutil/hash/hash128to64.h
M be/src/gutil/hash/jenkins.cc
M be/src/gutil/hash/jenkins.h
M be/src/gutil/hash/jenkins_lookup2.h
M be/src/gutil/hash/legacy_hash.h
M be/src/gutil/hash/string_hash.h
M be/src/gutil/int128.cc
M be/src/gutil/int128.h
M be/src/gutil/macros.h
M be/src/gutil/manual_constructor.h
M be/src/gutil/map-util.h
M be/src/gutil/mathlimits.cc
M be/src/gutil/once.cc
M be/src/gutil/once.h
M be/src/gutil/paranoid.h
M be/src/gutil/port.h
M be/src/gutil/raw_scoped_refptr_mismatch_checker.h
M be/src/gutil/ref_counted.cc
M be/src/gutil/ref_counted.h
M be/src/gutil/ref_counted_memory.cc
M be/src/gutil/ref_counted_memory.h
M be/src/gutil/singleton.h
M be/src/gutil/spinlock.cc
M be/src/gutil/spinlock.h
M be/src/gutil/spinlock_internal.cc
M be/src/gutil/spinlock_internal.h
M be/src/gutil/spinlock_linux-inl.h
M be/src/gutil/stl_util.h
M be/src/gutil/stringprintf.cc
M be/src/gutil/stringprintf.h
M be/src/gutil/strings/ascii_ctype.cc
M be/src/gutil/strings/charset.cc
M be/src/gutil/strings/charset.h
M be/src/gutil/strings/escaping.cc
M be/src/gutil/strings/escaping.h
M be/src/gutil/strings/fastmem.h
M be/src/gutil/strings/human_readable.cc
M be/src/gutil/strings/human_readable.h
M be/src/gutil/strings/join.cc
M be/src/gutil/strings/join.h
M be/src/gutil/strings/memutil.cc
M be/src/gutil/strings/memutil.h
M be/src/gutil/strings/numbers.cc
M be/src/gutil/strings/numbers.h
M be/src/gutil/strings/serialize.cc
M be/src/gutil/strings/serialize.h
M be/src/gutil/strings/split.cc
M be/src/gutil/strings/split.h
M be/src/gutil/strings/split_internal.h
M be/src/gutil/strings/strcat.cc
M be/src/gutil/strings/strcat.h
M be/src/gutil/strings/string_util-test.cc
M be/src/gutil/strings/stringpiece.cc
M be/src/gutil/strings/stringpiece.h
M be/src/gutil/strings/strip.cc
M be/src/gutil/strings/strip.h
M be/src/gutil/strings/substitute.cc
M be/src/gutil/strings/substitute.h
M be/src/gutil/strings/util.cc
M be/src/gutil/strings/util.h
M be/src/gutil/strtoint.cc
M be/src/gutil/strtoint.h
M be/src/gutil/synchronization_profiling.h
M be/src/gutil/sysinfo.cc
M be/src/gutil/threading/thread_collision_warner.cc
M be/src/gutil/threading/thread_collision_warner.h
M be/src/gutil/type_traits.h
M be/src/gutil/walltime.cc
M be/src/gutil/walltime.h
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream-v2-test.cc
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.h
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
108 files changed, 492 insertions(+), 472 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5688/9
-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors. Future upgrades to gutil/ should redo the work in
this commit.

* Remove kudu/ include prefix with command:

git grep -l "include \"kudu/" | xargs sed -i 's/include \"kudu\//include
\"/g'

* Change KUDU_GUTIL_* guards to be GUTIL_*

git grep -l KUDU_GUTIL | xargs sed -i 's/KUDU_GUTIL/GUTIL/g'

* Replace glog/logging.h with common/logging.h

git grep -l "glog/logging" | xargs sed -i 's/glog\/logging/common\/logging/g'

* Provide our own implementation of since-removed MonotonicNanos()
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Replay overwritten parts of following commits:

a7c3f30 - Remove AMD Opteron Rev E workaround from atomicops
54194af - IMPALA-4631: don't use floating point operations for time unit
conversions
152c586 - Improve AtomicInt abstraction and implementation

* Comment out non-compiling deprecated function definitions in numbers.h
* Overwrite changes from 92fafa "Use more efficient gutil implementation
  of Log2Ceiling" in favour of implementing them in Impala code only.
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Reviewed-on: http://gerrit.cloudera.org:8080/5688
Reviewed-by: Dan Hecht <dh...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/exec/hash-table.inline.h
M be/src/gutil/atomic_refcount.h
M be/src/gutil/atomicops-internals-x86.cc
M be/src/gutil/atomicops-internals-x86.h
M be/src/gutil/atomicops.h
M be/src/gutil/auxiliary/atomicops-internals-arm-generic.h
M be/src/gutil/auxiliary/atomicops-internals-arm-v6plus.h
M be/src/gutil/auxiliary/atomicops-internals-windows.h
M be/src/gutil/basictypes.h
M be/src/gutil/bind.h
M be/src/gutil/bind_helpers.h
M be/src/gutil/bind_internal.h
M be/src/gutil/bits.cc
M be/src/gutil/bits.h
M be/src/gutil/callback.h
M be/src/gutil/callback_forward.h
M be/src/gutil/callback_internal.cc
M be/src/gutil/callback_internal.h
M be/src/gutil/casts.h
M be/src/gutil/charmap.h
M be/src/gutil/cpu.cc
M be/src/gutil/cycleclock-inl.h
M be/src/gutil/endian.h
M be/src/gutil/fixedarray.h
M be/src/gutil/gscoped_ptr.h
M be/src/gutil/hash/builtin_type_hash.h
M be/src/gutil/hash/city.cc
M be/src/gutil/hash/city.h
M be/src/gutil/hash/hash.cc
M be/src/gutil/hash/hash.h
M be/src/gutil/hash/hash128to64.h
M be/src/gutil/hash/jenkins.cc
M be/src/gutil/hash/jenkins.h
M be/src/gutil/hash/jenkins_lookup2.h
M be/src/gutil/hash/legacy_hash.h
M be/src/gutil/hash/string_hash.h
M be/src/gutil/int128.cc
M be/src/gutil/int128.h
M be/src/gutil/macros.h
M be/src/gutil/manual_constructor.h
M be/src/gutil/map-util.h
M be/src/gutil/mathlimits.cc
M be/src/gutil/once.cc
M be/src/gutil/once.h
M be/src/gutil/paranoid.h
M be/src/gutil/port.h
M be/src/gutil/raw_scoped_refptr_mismatch_checker.h
M be/src/gutil/ref_counted.cc
M be/src/gutil/ref_counted.h
M be/src/gutil/ref_counted_memory.cc
M be/src/gutil/ref_counted_memory.h
M be/src/gutil/singleton.h
M be/src/gutil/spinlock.cc
M be/src/gutil/spinlock.h
M be/src/gutil/spinlock_internal.cc
M be/src/gutil/spinlock_internal.h
M be/src/gutil/spinlock_linux-inl.h
M be/src/gutil/stl_util.h
M be/src/gutil/stringprintf.cc
M be/src/gutil/stringprintf.h
M be/src/gutil/strings/ascii_ctype.cc
M be/src/gutil/strings/charset.cc
M be/src/gutil/strings/charset.h
M be/src/gutil/strings/escaping.cc
M be/src/gutil/strings/escaping.h
M be/src/gutil/strings/fastmem.h
M be/src/gutil/strings/human_readable.cc
M be/src/gutil/strings/human_readable.h
M be/src/gutil/strings/join.cc
M be/src/gutil/strings/join.h
M be/src/gutil/strings/memutil.cc
M be/src/gutil/strings/memutil.h
M be/src/gutil/strings/numbers.cc
M be/src/gutil/strings/numbers.h
M be/src/gutil/strings/serialize.cc
M be/src/gutil/strings/serialize.h
M be/src/gutil/strings/split.cc
M be/src/gutil/strings/split.h
M be/src/gutil/strings/split_internal.h
M be/src/gutil/strings/strcat.cc
M be/src/gutil/strings/strcat.h
M be/src/gutil/strings/string_util-test.cc
M be/src/gutil/strings/stringpiece.cc
M be/src/gutil/strings/stringpiece.h
M be/src/gutil/strings/strip.cc
M be/src/gutil/strings/strip.h
M be/src/gutil/strings/substitute.cc
M be/src/gutil/strings/substitute.h
M be/src/gutil/strings/util.cc
M be/src/gutil/strings/util.h
M be/src/gutil/strtoint.cc
M be/src/gutil/strtoint.h
M be/src/gutil/synchronization_profiling.h
M be/src/gutil/sysinfo.cc
M be/src/gutil/threading/thread_collision_warner.cc
M be/src/gutil/threading/thread_collision_warner.h
M be/src/gutil/type_traits.h
M be/src/gutil/walltime.cc
M be/src/gutil/walltime.h
M be/src/runtime/buffered-tuple-stream-test.cc
M be/src/runtime/buffered-tuple-stream-v2-test.cc
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.h
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
108 files changed, 492 insertions(+), 472 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Dan Hecht: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5688/6/be/src/common/atomic.h
File be/src/common/atomic.h:

Line 104:     return base::subtle::Release_CompareAndSwap(&value_, old_val, new_val) == old_val;
> It looks like Barrier_CompareAndSwap was added by Dan to gutils in the belo
I reverted to the original (Dan's commit) just to be safe.


http://gerrit.cloudera.org:8080/#/c/5688/6/be/src/util/time.h
File be/src/util/time.h:

PS6, Line 31: 1000000000ll
> nit: Maybe 1'000'000'000LL?
Tried it, but it confused my Emacs' cc-mode (which parsed it as an unterminated string). I didn't spend much time trying to fix it.


PS6, Line 40: 1e9
> NANOS_PER_SEC?
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Hello Jim Apple, Tim Armstrong,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/5688

to look at the new patch set (#7).

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................

IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Meant to be taken as a whole with the previous commit. This patch makes
the necessary code changes to Impala and the gutil/ library to fix all
compilation errors.

* Provide our own implementation of since-removed MonotonicNanos()
* Reinstate COMPILE_FLAGS argument to ADD_EXPORTABLE_LIBRARY,
  used by gutil.
* Comment out non-compiling deprecated function definitions in numbers.h
* Couple of misc fixes.

Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
---
M be/src/exec/hash-table.inline.h
M be/src/gutil/macros.h
M be/src/gutil/strings/numbers.h
M be/src/runtime/bufferpool/suballocator.cc
M be/src/runtime/collection-value-builder.h
M be/src/util/bit-util-test.cc
M be/src/util/bit-util.h
M be/src/util/error-util.h
M be/src/util/time.h
M cmake_modules/kudu_cmake_fns.txt
10 files changed, 89 insertions(+), 43 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/88/5688/7
-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 9:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/420/

-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4758: (2/2) Impala-side changes to build with latest gutil

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4758: (2/2) Impala-side changes to build with latest gutil
......................................................................


Patch Set 9:

(2 comments)

Thanks for the quick reviews!

http://gerrit.cloudera.org:8080/#/c/5688/9/be/src/util/time.h
File be/src/util/time.h:

Line 36:   return ts.tv_sec * NANOS_PER_SEC + ts.tv_nsec;
> are you sure impala doesn't have a commit that introduced GetMonoTimeNanos(
I think we did add it for MacOS (see https://github.com/apache/incubator-impala/commit/aeadd326a982af0f2ac8d55e425248a8bc7babfe), and that added GetMonoTimeNanos() for both platforms. I took the view that it's easier to maintain Impala-side dependencies for now, and that the MacOS port is effectively dead at this point.


http://gerrit.cloudera.org:8080/#/c/5688/9/cmake_modules/kudu_cmake_fns.txt
File cmake_modules/kudu_cmake_fns.txt:

Line 64:   endif()
> what do these two changes do and why are they needed now?
The first change adds custom compile flags to this target. This is needed to suppress some spurious warnings.

The second change isn't strictly needed here, but since gutil is the first user of ADD_EXPORTABLE_LIBRARY I figured I'd include the fix at the earliest point. This allows CMake to properly handle transitive dependencies, so we don't need to 'flatten' all the deps of, e.g., kudu_util when linking against it.


-- 
To view, visit http://gerrit.cloudera.org:8080/5688
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4ac21d7d6401f21fcdfdd1132b8f322bfba4bb80
Gerrit-PatchSet: 9
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes