[kudu-CR] Fix block manager-test running in some builds

2016-08-02 Thread Adar Dembo (Code Review)
Adar Dembo has submitted this change and it was merged.

Change subject: Fix block_manager-test running in some builds
..


Fix block_manager-test running in some builds

Even though the patch for KUDU-1538 passed pre-commit, it failed in the RELEASE
builds post-commit.

The issue appears to be that my trickery with weak symbols only worked in
some cases, depending on the link order, for dynamically linked builds,
since weak symbols are actually only relevant for statically linked ones.
Apparently in the precommit build, the link order was such that the gtest
detection did not work on block_manager-test, and thus things were fine.
But, in the release static build post-commit, the gtest detection worked,
and it broke the test in question.

This patch changes the gtest detection to use dlsym() instead of a weak symbol.
It appears to work more reliably in all types of builds. This also fixes the
test to properly manipulate the block ID sequence in the test that was failing.

Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Reviewed-on: http://gerrit.cloudera.org:8080/3733
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M CMakeLists.txt
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
A src/kudu/util/test_util_prod.cc
A src/kudu/util/test_util_prod.h
7 files changed, 81 insertions(+), 8 deletions(-)

Approvals:
  Adar Dembo: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 


[kudu-CR] Fix block manager-test running in some builds

2016-08-02 Thread Adar Dembo (Code Review)
Adar Dembo has posted comments on this change.

Change subject: Fix block_manager-test running in some builds
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Fix block manager-test running in some builds

2016-08-02 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Fix block_manager-test running in some builds
..

Fix block_manager-test running in some builds

Even though the patch for KUDU-1538 passed pre-commit, it failed in the RELEASE
builds post-commit.

The issue appears to be that my trickery with weak symbols only worked in
some cases, depending on the link order, for dynamically linked builds,
since weak symbols are actually only relevant for statically linked ones.
Apparently in the precommit build, the link order was such that the gtest
detection did not work on block_manager-test, and thus things were fine.
But, in the release static build post-commit, the gtest detection worked,
and it broke the test in question.

This patch changes the gtest detection to use dlsym() instead of a weak symbol.
It appears to work more reliably in all types of builds. This also fixes the
test to properly manipulate the block ID sequence in the test that was failing.

Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
---
M CMakeLists.txt
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
A src/kudu/util/test_util_prod.cc
A src/kudu/util/test_util_prod.h
7 files changed, 81 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/3733/4
-- 
To view, visit http://gerrit.cloudera.org:8080/3733
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] Fix block manager-test running in some builds

2016-08-02 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Fix block_manager-test running in some builds
..


Patch Set 4:

Build Started http://104.196.14.100/job/kudu-gerrit/2701/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] Fix block manager-test running in some builds

2016-08-02 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change.

Change subject: Fix block_manager-test running in some builds
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3733/3/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:

Line 196:   dl
> Does this need to be conditioned on NOT APPLE?
don't think so, given we use 'dl' in kudu_test_main elsewhere in this CMakeLists


http://gerrit.cloudera.org:8080/#/c/3733/3/src/kudu/util/test_util_prod.h
File src/kudu/util/test_util_prod.h:

Line 25: #include "kudu/gutil/macros.h"
> What's this for?
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] Fix block manager-test running in some builds

2016-07-24 Thread Todd Lipcon (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: Fix block_manager-test running in some builds
..

Fix block_manager-test running in some builds

Even though the patch for KUDU-1538 passed pre-commit, it failed in the RELEASE
builds post-commit.

The issue appears to be that my trickery with weak symbols only worked in
some cases, depending on the link order, for dynamically linked builds,
since weak symbols are actually only relevant for statically linked ones.
Apparently in the precommit build, the link order was such that the gtest
detection did not work on block_manager-test, and thus things were fine.
But, in the release static build post-commit, the gtest detection worked,
and it broke the test in question.

This patch changes the gtest detection to use dlsym() instead of a weak symbol.
It appears to work more reliably in all types of builds. This also fixes the
test to properly manipulate the block ID sequence in the test that was failing.

Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
---
M CMakeLists.txt
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
A src/kudu/util/test_util_prod.cc
A src/kudu/util/test_util_prod.h
7 files changed, 83 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/3733/3
-- 
To view, visit http://gerrit.cloudera.org:8080/3733
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] Fix block manager-test running in some builds

2016-07-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Fix block_manager-test running in some builds
..


Patch Set 3:

Build Started http://104.196.14.100/job/kudu-gerrit/2649/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No


[kudu-CR] Fix block manager-test running in some builds

2016-07-24 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Adar Dembo,

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

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

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

Change subject: Fix block_manager-test running in some builds
..

Fix block_manager-test running in some builds

Even though the patch for KUDU-1538 passed pre-commit, it failed in the RELEASE
builds post-commit.

The issue appears to be that my trickery with weak symbols only worked in
some cases, depending on the link order, for dynamically linked builds,
since weak symbols are actually only relevant for statically linked ones.
Apparently in the precommit build, the link order was such that the gtest
detection did not work on block_manager-test, and thus things were fine.
But, in the release static build post-commit, the gtest detection worked,
and it broke the test in question.

This patch changes the gtest detection to use dlsym() instead of a weak symbol.
It appears to work more reliably in all types of builds. This also fixes the
test to properly manipulate the block ID sequence in the test that was failing.

Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
A src/kudu/util/test_util_prod.cc
A src/kudu/util/test_util_prod.h
6 files changed, 76 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/3733/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3733
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins


[kudu-CR] Fix block manager-test running in some builds

2016-07-24 Thread Todd Lipcon (Code Review)
Hello Dan Burkert, Adar Dembo,

I'd like you to do a code review.  Please visit

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

to review the following change.

Change subject: Fix block_manager-test running in some builds
..

Fix block_manager-test running in some builds

Even though the patch for KUDU-1538 passed pre-commit, it failed in the RELEASE
builds post-commit.

The issue appears to be that my trickery with weak symbols only worked in
some cases, depending on the link order, for dynamically linked builds,
since weak symbols are actually only relevant for statically linked ones.
Apparently in the precommit build, the link order was such that the gtest
detection did not work on block_manager-test, and thus things were fine.
But, in the release static build post-commit, the gtest detection worked,
and it broke the test in question.

This patch changes the gtest detection to use dlsym() instead of a weak symbol.
It appears to work more reliably in all types of builds. This also fixes the
test to properly manipulate the block ID sequence in the test that was failing.

Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
---
M src/kudu/fs/block_manager-test.cc
M src/kudu/fs/log_block_manager.cc
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
A src/kudu/util/test_util_prod.cc
A src/kudu/util/test_util_prod.h
6 files changed, 75 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/3733/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3733
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 


[kudu-CR] Fix block manager-test running in some builds

2016-07-24 Thread Kudu Jenkins (Code Review)
Kudu Jenkins has posted comments on this change.

Change subject: Fix block_manager-test running in some builds
..


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2646/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a4025a3d859b139b74be997706bd27d0ba7b292
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No