[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

2016-09-09 Thread David Ribeiro Alves (Code Review)
David Ribeiro Alves has posted comments on this change.

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4119/3/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

Line 1803:   // Make a copy so we don't need to synchronize on it while 
iterating.
> I really have no idea what's going on with the locking here... but it's not
yeah, same.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Dan Burkert 
Gerrit-Reviewer: David Ribeiro Alves 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

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

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 3:

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

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

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


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

2016-09-09 Thread David Ribeiro Alves (Code Review)
Hello Kudu Jenkins,

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

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

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

Change subject: java: fix leak of TabletClient objects in client2tablets map
..

java: fix leak of TabletClient objects in client2tablets map

After running YCSB for a week, most of the clients had hit OOMEs. Some
heap dump analysis showed that the client2tablets map had hundreds of
thousands of leaked clients.

It seems that we were neglecting to remove the client from the
client2tablets map upon a disconnect. This fixes the issue and adds a
regression test which reproduced the bug.

This patch was worked on by Todd Lipcon and David Alves.

Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
4 files changed, 72 insertions(+), 5 deletions(-)


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

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


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

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

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 2:

ignore the re-push, accidentally rebased

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
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
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

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

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 1:

Forgot to add: I was under the impression that after a client reconnects, the 
client2tablets entry is reused. I see now that's not the case at all.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
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


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

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

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/4119/1/java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
File java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java:

Line 317: for (int port : tserverPorts) {
I don't think we're managing this list very well right now. We never actually 
remove entries from it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
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: Yes


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

2016-08-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/4119

to review the following change.

Change subject: java: fix leak of TabletClient objects in client2tablets map
..

java: fix leak of TabletClient objects in client2tablets map

After running YCSB for a week, most of the clients had hit OOMEs. Some
heap dump analysis showed that the client2tablets map had hundreds of
thousands of leaked clients.

It seems that we were neglecting to remove the client from the
client2tablets map upon a disconnect. This fixes the issue and adds a
regression test which reproduced the bug.

Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java
M java/kudu-client/src/test/java/org/apache/kudu/client/MiniKuduCluster.java
M 
java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduSession.java
4 files changed, 52 insertions(+), 5 deletions(-)


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

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


[kudu-CR] java: fix leak of TabletClient objects in client2tablets map

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

Change subject: java: fix leak of TabletClient objects in client2tablets map
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I302650f2a6526e7c51537264137a4f00cbbda073
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