You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2019/06/13 04:29:34 UTC
[kudu-CR] KUDU-2836: Release memory to OS periodically
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13606
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
KUDU-2836: Release memory to OS periodically
In read-only workloads, memory resered by tcmalloc is barely
released to operation system, memory consumed by Kudu server
increased continuously, and system memory may used up and lead
to OOM.
This patch add a background thread to periodically GC tcmalloc
reserved memory.
Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
---
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/process_memory.h
4 files changed, 50 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/13606/1
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <40...@qq.com>
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13606/3//COMMIT_MSG
Commit Message:
PS3:
> Would it make sense to add a test to verify the behavior is as expected wi
Done
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Sat, 15 Jun 2019 14:55:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 1:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@9
PS1, Line 9: resered
reserved
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@10
PS1, Line 10: operation
operating
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@11
PS1, Line 11: increased
increases
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@11
PS1, Line 11: used
fill
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@13
PS1, Line 13: This patch add a background thread to periodically GC tcmalloc
adds
http://gerrit.cloudera.org:8080/#/c/13606/1/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/13606/1/src/kudu/server/server_base.cc@209
PS1, Line 209: 3600
How did you arrive at this default value?
http://gerrit.cloudera.org:8080/#/c/13606/1/src/kudu/server/server_base.cc@678
PS1, Line 678: while (stop_background_threads_latch_.count() > 0) {
: MonoDelta gc_interval = MonoDelta::FromSeconds(FLAGS_gc_tcmalloc_memory_interval_seconds);
: if (gc_interval.Equals(MonoDelta::FromSeconds(0))) {
: // FLAGS_gc_tcmalloc_memory_interval_seconds is zero, that means Tcmalloc GC periodically
: // is disabled now. We can check this flag some time later.
: stop_background_threads_latch_.WaitFor(MonoDelta::FromSeconds(60));
: continue;
: }
:
: if (!stop_background_threads_latch_.WaitFor(gc_interval)) {
: kudu::process_memory::GcTcmalloc();
: }
: }
How about something like this:
MonoDelta gc_interval;
do {
gc_interval = MonoDelta::FromSeconds(FLAGS_gc_tcmalloc_memory_interval_seconds > 0
? FLAGS_gc_tcmalloc_memory_interval_seconds : 60);
if (FLAGS_gc_tcmalloc_memory_interval_seconds > 0) {
kudu::process_memory::GcTcmalloc();
}
} while (!stop_background_threads_latch.WaitFor(gc_interval));
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 13 Jun 2019 04:49:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13606
to look at the new patch set (#3).
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
KUDU-2836: Release memory to OS periodically
In read-only workloads, memory reserved by tcmalloc is barely
released to operating system, memory consumed by Kudu server
increases continuously, and system memory may fill up and lead
to OOM.
This patch adds a background thread to periodically GC tcmalloc
reserved memory.
Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
---
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/process_memory.h
4 files changed, 47 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/13606/3
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
KUDU-2836: Release memory to OS periodically
In read-only workloads, memory reserved by tcmalloc is barely
released to operating system, memory consumed by Kudu server
increases continuously, and system memory may fill up and lead
to OOM.
This patch adds a background thread to periodically GC tcmalloc
reserved memory.
Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Reviewed-on: http://gerrit.cloudera.org:8080/13606
Reviewed-by: Todd Lipcon <to...@apache.org>
Tested-by: Todd Lipcon <to...@apache.org>
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/disk_reservation-itest.cc
A src/kudu/integration-tests/memory_gc-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/process_memory.h
9 files changed, 202 insertions(+), 20 deletions(-)
Approvals:
Todd Lipcon: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13606
to look at the new patch set (#2).
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
KUDU-2836: Release memory to OS periodically
In read-only workloads, memory reserved by tcmalloc is barely
released to operating system, memory consumed by Kudu server
increases continuously, and system memory may fill up and lead
to OOM.
This patch adds a background thread to periodically GC tcmalloc
reserved memory.
Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
---
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/process_memory.h
4 files changed, 45 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/13606/2
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13606
to look at the new patch set (#4).
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
KUDU-2836: Release memory to OS periodically
In read-only workloads, memory reserved by tcmalloc is barely
released to operating system, memory consumed by Kudu server
increases continuously, and system memory may fill up and lead
to OOM.
This patch adds a background thread to periodically GC tcmalloc
reserved memory.
Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/memory_gc-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/process_memory.h
6 files changed, 219 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/13606/4
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 13 Jun 2019 17:13:09 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has removed a vote on this change.
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 2:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@9
PS1, Line 9: reserve
> reserved
Done
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@10
PS1, Line 10: operating
> operating
Done
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@11
PS1, Line 11: fill
> fill
Done
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@11
PS1, Line 11: increases
> increases
Done
http://gerrit.cloudera.org:8080/#/c/13606/1//COMMIT_MSG@13
PS1, Line 13: This patch adds a background thread to periodically GC tcmalloc
> adds
Done
http://gerrit.cloudera.org:8080/#/c/13606/1/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/13606/1/src/kudu/server/server_base.cc@209
PS1, Line 209: 3600
> How did you arrive at this default value?
Really not consider much about it. It depends on the speed of memory consumed.
http://gerrit.cloudera.org:8080/#/c/13606/1/src/kudu/server/server_base.cc@678
PS1, Line 678: MonoDelta gc_interval;
: do {
: gc_interval = MonoDelta::FromSeconds(FLAGS_gc_tcmalloc_memory_interval_seconds > 0
: ? FLAGS_gc_tcmalloc_memory_interval_seconds : 60);
: if (FLAGS_gc_tcmalloc_memory_interval_seconds > 0) {
: kudu::process_memory::GcTcmalloc();
: }
: } while (!stop_background_threads_latch_.WaitFor(gc_interval));
: }
: #endif
:
: std::string ServerBase::FooterHtml() const {
: r
> How about something like this:
Done
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 13 Jun 2019 05:53:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13606/3//COMMIT_MSG
Commit Message:
PS3:
Would it make sense to add a test to verify the behavior is as expected with regard to the setting for the --tcmalloc_max_free_bytes_percentage ? Probably, it might be done with very active read workload (see TestWorkload in src/kudu/integration_tests/test_workload.h and it usage in various tests) and setting the --gc_tcmalloc_memory_interval_seconds flag to something low (like 5 seconds or alike). The information about current tcmalloc metrics might be retrieved from the /memz page of the embedded webserver.
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 13 Jun 2019 18:08:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13606
to look at the new patch set (#7).
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
KUDU-2836: Release memory to OS periodically
In read-only workloads, memory reserved by tcmalloc is barely
released to operating system, memory consumed by Kudu server
increases continuously, and system memory may fill up and lead
to OOM.
This patch adds a background thread to periodically GC tcmalloc
reserved memory.
Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
---
M src/kudu/integration-tests/CMakeLists.txt
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/disk_reservation-itest.cc
A src/kudu/integration-tests/memory_gc-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/process_memory.h
9 files changed, 202 insertions(+), 20 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/13606/7
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Adar Dembo, Grant Henke, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13606
to look at the new patch set (#6).
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
KUDU-2836: Release memory to OS periodically
In read-only workloads, memory reserved by tcmalloc is barely
released to operating system, memory consumed by Kudu server
increases continuously, and system memory may fill up and lead
to OOM.
This patch adds a background thread to periodically GC tcmalloc
reserved memory.
Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
---
M src/kudu/integration-tests/CMakeLists.txt
A src/kudu/integration-tests/memory_gc-itest.cc
M src/kudu/server/server_base.cc
M src/kudu/server/server_base.h
M src/kudu/util/countdown_latch.h
M src/kudu/util/process_memory.h
6 files changed, 221 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/06/13606/6
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 6:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc
File src/kudu/integration-tests/memory_gc-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@55
PS6, Line 55: void GetMemorySize(ExternalTabletServer* ets, MemoryType type, int64_t* value) {
This is OK, but I wonder if it would be easier to fetch
/metrics?metrics=tcmalloc_pageheap_free_bytes,generic_current_allocated_bytes and parse it with RapidJSON? I think that'll give you the same info but be less fragile (and easier to read) than regex.
Having this function just give two out-parameters for the two types of memory would also probably simplify it
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@87
PS6, Line 87: OverHeadPercent
nit: rename to GetOverheadRatio (a ratio is between 0 and 1, but a percent is between 0 and 100)
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@138
PS6, Line 138: e
typo
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@167
PS6, Line 167: // Limit memory usage once a tserver.
why do we need to test this once per tserver? I think just running this test once (eg limit tserver 1) is sufficient
http://gerrit.cloudera.org:8080/#/c/13606/3/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/13606/3/src/kudu/server/server_base.cc@211
PS3, Line 211: DEFINE_uint64(gc_tcmalloc_memory_interval_seconds, 3600,
is once an hour often enough? from your graphs you uploaded on the JIRA it looked like the memory can grow much faster than this. I was thinking every 30 seconds or so would be OK
http://gerrit.cloudera.org:8080/#/c/13606/3/src/kudu/server/server_base.cc@682
PS3, Line 682: gc_interval = MonoDelta::FromSeconds(FLAGS_gc_tcmalloc_memory_interval_seconds > 0
: ? FLAGS_gc_tcmalloc_memory_interval_seconds : 60);
: if (FLAGS_gc_tcmalloc_memory_interval_seconds > 0)
I think this could be clearer if you rename 'gc_interval' to 'check_interval' and add a comment like:
// If GC is disabled, wake up every 30 seconds anyway to recheck the value of the flag.
check_interval = FLAGS_gc_tcmalloc_memory_interval_secs > 0 ? FLAGS_gc_tcmalloc_memory_interval_secs : 60;
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/server/server_base.cc@211
PS6, Line 211: DEFINE_uint64(gc_tcmalloc_memory_interval_seconds, 3600,
Let's default this to 30 seconds or something. Otherwise I think a lot of memory can accumulate.
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Mon, 17 Jun 2019 16:43:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 7:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc
File src/kudu/integration-tests/memory_gc-itest.cc:
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@55
PS6, Line 55: &generic_current_allocated_bytes));
> This is OK, but I wonder if it would be easier to fetch
Done
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@87
PS6, Line 87: ASSERT_GE(work
> nit: rename to GetOverheadRatio (a ratio is between 0 and 1, but a percent
Done
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@138
PS6, Line 138:
> typo
Done
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/integration-tests/memory_gc-itest.cc@167
PS6, Line 167:
> why do we need to test this once per tserver? I think just running this tes
Done
http://gerrit.cloudera.org:8080/#/c/13606/3/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/13606/3/src/kudu/server/server_base.cc@211
PS3, Line 211: DEFINE_uint64(gc_tcmalloc_memory_interval_seconds, 30,
> is once an hour often enough? from your graphs you uploaded on the JIRA it
Done
http://gerrit.cloudera.org:8080/#/c/13606/3/src/kudu/server/server_base.cc@682
PS3, Line 682: // If GC is disabled, wake up every 60 seconds anyway to recheck the value of the flag.
: check_interval = MonoDelta::FromSeconds(FLAGS_gc_tcmalloc_memory_interval_seconds > 0
: ? FLAGS_gc_tcmalloc_memory_interval_seconds : 60)
> I think this could be clearer if you rename 'gc_interval' to 'check_interva
Done
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
http://gerrit.cloudera.org:8080/#/c/13606/6/src/kudu/server/server_base.cc@211
PS6, Line 211: DEFINE_uint64(gc_tcmalloc_memory_interval_seconds, 30,
> Let's default this to 30 seconds or something. Otherwise I think a lot of m
Done
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 06:39:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 7: Verified+1 Code-Review+2
Test failure seems to be a flake in client-stress-test, unrelated to this change. Thanks for fixing this!
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 07:06:45 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2836: Release memory to OS periodically
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/13606 )
Change subject: KUDU-2836: Release memory to OS periodically
......................................................................
Patch Set 3: Code-Review+1
Could reduce the number of #ifdefs by having the new ServerBase functions no-op when tcmalloc is unavailable, but I don't care that much about it.
--
To view, visit http://gerrit.cloudera.org:8080/13606
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibeb3529ed53c250f69e5faf5b2a067b2abce06c0
Gerrit-Change-Number: 13606
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <40...@qq.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Yingchun Lai <40...@qq.com>
Gerrit-Comment-Date: Thu, 13 Jun 2019 17:09:32 +0000
Gerrit-HasComments: No