You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2017/03/07 18:21:03 UTC

[kudu-CR] [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN

David Ribeiro Alves has uploaded a new change for review.

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

Change subject: [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN
......................................................................

[rpc tests] Reduce flakyness by setting a longer keepalive in TSAN

Some rpc tests have a multi-threaded workload and a fixed keepalive
setting (1 sec). However, in TSAN, threads often take a long time
to create (1+ sec) leading to flakyness due to the connection already
being close when the thread is created and starts doing rpcs.

This patch adds a flags for the keepalive setting and increases
its value for TSAN builds.

Change-Id: Ic93df66e312239fa3de22e5d833c27982e0683d9
---
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
2 files changed, 15 insertions(+), 7 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic93df66e312239fa3de22e5d833c27982e0683d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>

[kudu-CR] [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN

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

Change subject: [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN
......................................................................


Patch Set 1:

why is this messy? we use flags for this kind of thing all the time, right? adding more inheritance for the sake of changing a single value seems like overkill

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic93df66e312239fa3de22e5d833c27982e0683d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN

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

Change subject: [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN
......................................................................


Patch Set 1:

(Sorry about the copy / paste mistake in the above comment)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic93df66e312239fa3de22e5d833c27982e0683d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN

Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has abandoned this change. ( http://gerrit.cloudera.org:8080/6289 )

Change subject: [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN
......................................................................


Abandoned

I no longer have a patch around I can test this on, and it hasn't come up since so abandoning
-- 
To view, visit http://gerrit.cloudera.org:8080/6289
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: Ic93df66e312239fa3de22e5d833c27982e0683d9
Gerrit-Change-Number: 6289
Gerrit-PatchSet: 1
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN

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

Change subject: [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN
......................................................................


Patch Set 1:

This is a little messy. Consider simply making it a constructor parameter in RpcTestBase (possibly with a default) and using a different subclass for the keepalive test if needed.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic93df66e312239fa3de22e5d833c27982e0683d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No

[kudu-CR] [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN

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

Change subject: [rpc tests] Reduce flakyness by setting a longer keepalive in TSAN
......................................................................


Patch Set 1:

#ifdef THREAD_SANITIZER
        FLAGS_rpc_test_connection_keepalive_millis = 10000;
    #endif

 > why is this messy? we use flags for this kind of thing all the
 > time, right? adding more inheritance for the sake of changing a
 > single value seems like overkill

 > why is this messy? we use flags for this kind of thing all the
 > time, right? adding more inheritance for the sake of changing a
 > single value seems like overkill

I'd characterize it as a little messy because it adds a global variable that is not really used and it's causing lint errors. How often will we set the flag on the command line?

Why not just keep it an instance variable and add the new preprocessor-driven logic in the constructor?

    #ifdef THREAD_SANITIZER
        keepalive_time_ms_ = 10000;
    #endif

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic93df66e312239fa3de22e5d833c27982e0683d9
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No