You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2017/05/05 04:37:44 UTC

[kudu-CR](branch-1.3.x) KUDU-1993: fixed validation of 'grouped' gflags

Hello Adar Dembo, Kudu Jenkins,

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

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

to review the following change.

Change subject: KUDU-1993: fixed validation of 'grouped' gflags
......................................................................

KUDU-1993: fixed validation of 'grouped' gflags

Added generic implementation for grouped gflags validators.

Use CUSTOM_FLAG_VALIDATOR() macro to register late-phase validator
function for command-line flags. The validation is performed upon call
of HandleCommonFlags() invoked by the top-level ParseCommandLineFlags()
function.

Added unit test to cover the new functionality.

Updated validators for security-related RPC and embedded webserver
flags.

As a workaround for LSAN reports on the leaks in case of ASAN build,
added two scoped leak check disablers into CreateAndStartTimeoutThread.
That does not seem harmful or hiding any potential leaks since it
affects only the timeout thread itself.  Opened separate JIRA to track
the (false positive?) leak warning: KUDU-1995.

Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Reviewed-on: http://gerrit.cloudera.org:8080/6795
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3)
---
M src/kudu/rpc/messenger.cc
M src/kudu/server/webserver_options.cc
M src/kudu/util/CMakeLists.txt
A src/kudu/util/flag_validators-test.cc
A src/kudu/util/flag_validators.cc
A src/kudu/util/flag_validators.h
M src/kudu/util/flags.cc
M src/kudu/util/test_main.cc
8 files changed, 415 insertions(+), 36 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR](branch-1.3.x) KUDU-1993: fixed validation of 'grouped' gflags

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.

Change subject: KUDU-1993: fixed validation of 'grouped' gflags
......................................................................


Patch Set 1: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR](branch-1.3.x) KUDU-1993: fixed validation of 'grouped' gflags

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.

Change subject: KUDU-1993: fixed validation of 'grouped' gflags
......................................................................


KUDU-1993: fixed validation of 'grouped' gflags

Added generic implementation for grouped gflags validators.

Use CUSTOM_FLAG_VALIDATOR() macro to register late-phase validator
function for command-line flags. The validation is performed upon call
of HandleCommonFlags() invoked by the top-level ParseCommandLineFlags()
function.

Added unit test to cover the new functionality.

Updated validators for security-related RPC and embedded webserver
flags.

As a workaround for LSAN reports on the leaks in case of ASAN build,
added two scoped leak check disablers into CreateAndStartTimeoutThread.
That does not seem harmful or hiding any potential leaks since it
affects only the timeout thread itself.  Opened separate JIRA to track
the (false positive?) leak warning: KUDU-1995.

Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Reviewed-on: http://gerrit.cloudera.org:8080/6795
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
(cherry picked from commit e7334c2e6607ac0fa778499eda2df3f4cfcd3fe3)
Reviewed-on: http://gerrit.cloudera.org:8080/6806
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
---
M src/kudu/rpc/messenger.cc
M src/kudu/server/webserver_options.cc
M src/kudu/util/CMakeLists.txt
A src/kudu/util/flag_validators-test.cc
A src/kudu/util/flag_validators.cc
A src/kudu/util/flag_validators.h
M src/kudu/util/flags.cc
M src/kudu/util/test_main.cc
8 files changed, 415 insertions(+), 36 deletions(-)

Approvals:
  Jean-Daniel Cryans: Looks good to me, approved
  Kudu Jenkins: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I3755d62590cdc63a9d501ba69d980cb15f8069a9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: branch-1.3.x
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins