[kudu-CR] Start a background thread to run ResultTracker GC

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 5: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Start a background thread to run ResultTracker GC

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 5:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Start a background thread to run ResultTracker GC

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Start a background thread to run ResultTracker GC

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 4:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Start a background thread to run ResultTracker GC

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Start a background thread to run ResultTracker GC
..

Start a background thread to run ResultTracker GC

Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
---
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/service_if.cc
M src/kudu/server/server_base.cc
5 files changed, 54 insertions(+), 25 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 4
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Start a background thread to run ResultTracker GC

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3961/3/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 426: void ResultTracker::StartGCThread() {
> I suppose that's true. You OK with my just removing the lock then?
Yeah, that's fine with me.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Start a background thread to run ResultTracker GC

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3961/3/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 426: void ResultTracker::StartGCThread() {
> Can't we put this in an Init() style method that is assumed to be single-th
I suppose that's true. You OK with my just removing the lock then?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Start a background thread to run ResultTracker GC

[Adar Dembo (Code Review)]

Adar Dembo has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3961/3/src/kudu/rpc/result_tracker.cc
File src/kudu/rpc/result_tracker.cc:

Line 426: void ResultTracker::StartGCThread() {
Can't we put this in an Init() style method that is assumed to be 
single-threaded, so that gc_thread_ needn't be protected by lock_?

Actually, a concurrent invocation would crash in the CHECK, so what's the point 
of the lock?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Start a background thread to run ResultTracker GC

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 3:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Start a background thread to run ResultTracker GC

[Todd Lipcon (Code Review)]

Hello Kudu Jenkins,

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

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

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

Change subject: Start a background thread to run ResultTracker GC
..

Start a background thread to run ResultTracker GC

Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
---
M src/kudu/rpc/exactly_once_rpc-test.cc
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/service_if.cc
M src/kudu/server/server_base.cc
5 files changed, 53 insertions(+), 25 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] Start a background thread to run ResultTracker GC

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 2:

(1 comment)

will try to write a test. fwiw I've been testing this on a cluster ~2 days now 
and seems stable. Across ~72 tablet servers none is showing a peak memory 
consumption of more than 5MB (and averaging around 2MB).

http://gerrit.cloudera.org:8080/#/c/3961/2//COMMIT_MSG
Commit Message:

Line 7: Start a background thread to run ResultTracker GC
> I was expecting ResultTracker GC to be implemented as an MM op, like other 
yep, it's assumed that it's cheap enough (and doesnt do IO) so it doesn't need 
to be "prioritized" against other actions that do IO and thus might contend 
with each other.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: Yes

[kudu-CR] Start a background thread to run ResultTracker GC

[Todd Lipcon (Code Review)]

Todd Lipcon has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 2:

I looped exactly_once_writes-itest with this patch 1000 times (100% pass): 
http://dist-test.cloudera.org//job?job_id=todd.1471058312.30931

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-HasComments: No

[kudu-CR] Start a background thread to run ResultTracker GC

[Kudu Jenkins (Code Review)]

Kudu Jenkins has posted comments on this change.

Change subject: Start a background thread to run ResultTracker GC
..


Patch Set 1:

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No

[kudu-CR] Start a background thread to run ResultTracker GC

[Todd Lipcon (Code Review)]

Todd Lipcon has uploaded a new change for review.

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

Change subject: Start a background thread to run ResultTracker GC
..

Start a background thread to run ResultTracker GC

Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
---
M src/kudu/rpc/result_tracker.cc
M src/kudu/rpc/result_tracker.h
M src/kudu/rpc/service_if.cc
M src/kudu/server/server_base.cc
4 files changed, 37 insertions(+), 4 deletions(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia34ce95e78920596eb8b9db53643845f637c8e6c
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon