You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Song Jiacheng (Code Review)" <ge...@cloudera.org> on 2023/11/20 09:56:34 UTC
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Song Jiacheng has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20716
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Issue a warning on parameter heap_sample_every_n_bytes.
Parameter heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a value greater than 0 will make thread
cache request to page heap for some sample allocations, which
will occupy the lock of page heap. So does the free opeation
of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc.
Almost all the apply threads are waiting for the spin lock, even if
setting the parameter to a higher value like 1M.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of the parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/1
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 1
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Patch Set 6:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@7
PS6, Line 7: Issue a warning
With this patch, no warning is actually issued :) This patch just adds a warning into the description of the flag, but the description already contains information about the overhead of the allocation sampling.
I guess proper summary for this patch could be:
[util] add a warning into 'heap_sample_every_n_bytes' flag description
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@10
PS6, Line 10: thread
: cache request
What is 'thread cache request'?
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@12
PS6, Line 12: occupy
nit: 'acquire' or 'hold'
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@15
PS6, Line 15: (especially for upserts)
Is this essential? Any allocation on the heap will add to the lock contention, no?
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@16
PS6, Line 16: so much performance because of the lock contention of tcmalloc
Even without enabling tcmalloc sampling a system with quite high throughput already shows significant lock contention in tcmalloc -- you could see it in the thread stacks traces captured in the diagnostic logs :)
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@18
PS6, Line 18: 1MB
This is quite low, actually. Empirically, setting it to hundreds of gigabytes makes more sense in a single tablet server of a production system, if trying the sampling at all.
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@20
PS6, Line 20: So give a warning on the parameter heap_sample_every_n_bytes in case
: that some users do not know the terrible influence of it.
Even with the extra warning, users wouldn't know how bad the lock contention in tcmalloc could be before they try it: no quantitative information is provided on that in the description of the flag, before or after this patch.
If trying to add guardrails for people who don't quite understand what they are doing, maybe consider tagging this flag with the 'unsafe' attribute: it's currently tagged with just 'experimental' attribute, but adding 'unsafe' looks reasonable.
In addition, consider issuing a warning into the log if the heap_sample_every_n_bytes flag is set to a positive value -- that could be done in the validator for the flag.
http://gerrit.cloudera.org:8080/#/c/20716/6/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/6/src/kudu/util/flags.cc@100
PS6, Line 100: Warning: Setting it to a value greater than 0 will probably "
: "lead to lock contention in tcmalloc, try setting it manually when a sample "
: "is desired.
Try "setting manually" what?
how about:
WARNING: setting this flag to a positive value increases lock contention in tcmalloc; the lower the value, the higher the lock contention
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 6
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Comment-Date: Tue, 21 Nov 2023 17:27:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#13).
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] Add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread
cache request some sample allocations from page cache, which
will compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput might lose so much performance
because of the lock contention of tcmalloc. Almost all the apply
threads are waiting for the spin lock.
So give a warning on --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/13
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 13
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@9
PS7, Line 9: Parameter heap_sample_every_n_bytes
nit: --heap_sample_every_n_bytes
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@10
PS7, Line 10: to a value greater than 0
nit: a positive value
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@11
PS7, Line 11:
Remove it.
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@12
PS7, Line 12: hold
nit: compete the lock of page heap with other thread?
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@18
PS7, Line 18: the paramete
nit: --heap_sample_every_n_bytes
http://gerrit.cloudera.org:8080/#/c/20716/7/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/7/src/kudu/util/flags.cc@99
PS7, Line 99: Warning: Setting this flag to a positive value increases lock contention "
: "in tcmalloc; the lower the value, the higher the lock contention.
It seems better to set --heap_sample_every_n_bytes more larger to lower the lock contention. Can you give some suggestion how large should this parameter should be set to get better performance?
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 7
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Comment-Date: Thu, 07 Dec 2023 07:55:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#7).
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a value greater than 0 will make thread
cache request some sample allocations from page cache , which
will hold the lock of page heap. So does the free operation
of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc.
Almost all the apply threads are waiting for the spin lock, even if
setting the parameter to a higher value like 1MB.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/7
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 7
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#14).
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] Add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread
cache request some sample allocations from page cache, which
will compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput might lose so much performance
because of the lock contention of tcmalloc. Almost all the apply
threads are waiting for the spin lock.
So add a warning on --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/14
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 14
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
Patch Set 8:
(7 comments)
> Patch Set 7:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@9
PS7, Line 9: Parameter --heap_sample_every_n_byt
> nit: --heap_sample_every_n_bytes
Done
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@10
PS7, Line 10: to a positive value will
> nit: a positive value
Done
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@11
PS7, Line 11: ,
> Remove it.
Done
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@12
PS7, Line 12: comp
> nit: compete the lock of page heap with other thread?
Done
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@18
PS7, Line 18:
> nit: --heap_sample_every_n_bytes
Done
http://gerrit.cloudera.org:8080/#/c/20716/7/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/7/src/kudu/util/flags.cc@100
PS7, Line 100: "in tcmalloc; the lower the value, the higher the lock contention.");
> Is it necessary to remove the former description? 524288 is a recommend val
I have tested 524288 in a high throughput environment, and the stacks show that the apply threads of tserver always stuck in DoSampleAllocation() or Free(). There are 48 apply threads per tserver, but only a few of them could work at the meantime.
Taking one sample once every 512KB allocation is really too often in production environment.
http://gerrit.cloudera.org:8080/#/c/20716/7/src/kudu/util/flags.cc@99
PS7, Line 99: Warning: Setting this flag to a positive value increases lock contention "
: "in tcmalloc; the lower the value, the higher the lock contention.
> It seems better to set --heap_sample_every_n_bytes more larger to lower the
I think it should depend on the memory limit and throughput of the tserver, like 1% of hard limit could be accepted.
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 8
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Mon, 18 Dec 2023 07:22:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
Patch Set 17: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20716/7//COMMIT_MSG@11
PS7, Line 11: h
nit: remove the extra symbol?
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 17
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Thu, 21 Dec 2023 04:06:47 +0000
Gerrit-HasComments: Yes
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Patch Set 4:
(3 comments)
Thank you for the review!
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc@101
PS4, Line 101: leader
> nit: lead
Done
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc@102
PS4, Line 102: wanted
> maybe nit: use 'desired'
Done
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc@546
PS4, Line 546:
> nit: remove extra indentation
Done
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 4
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Comment-Date: Tue, 21 Nov 2023 13:27:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#12).
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] Add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread
cache request some sample allocations from page cache, which
will compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc,
Almost all the apply threads are waiting for the spin lock.
So give a warning on --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/12
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 12
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#16).
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] Add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread cache
request some sample allocations from page cache, which will
compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput might lose so much performance
because of the lock contention of tcmalloc. Almost all the apply
threads are waiting for the spin lock.
So add a warning on --heap_sample_every_n_bytes in case that some
users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/16
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 16
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20716/7/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/7/src/kudu/util/flags.cc@100
PS7, Line 100: "in tcmalloc; the lower the value, the higher the lock contention.");
Is it necessary to remove the former description? 524288 is a recommend value of the official docs of gperftools[1], it's reasonable to leave it there.
1. https://gperftools.github.io/gperftools/tcmalloc.html
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 7
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Fri, 08 Dec 2023 08:13:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#6).
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Issue a warning on parameter heap_sample_every_n_bytes.
Parameter heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a value greater than 0 will make thread
cache request to page heap for some sample allocations, which
will occupy the lock of page heap. So does the free operation
of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc.
Almost all the apply threads are waiting for the spin lock, even if
setting the parameter to a higher value like 1MB.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of it.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/6
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 6
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#5).
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Issue a warning on parameter heap_sample_every_n_bytes.
Parameter heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a value greater than 0 will make thread
cache request to page heap for some sample allocations, which
will occupy the lock of page heap. So does the free operation
of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc.
Almost all the apply threads are waiting for the spin lock, even if
setting the parameter to a higher value like 1MB.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/5
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 5
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
Patch Set 7:
(9 comments)
> Patch Set 6:
>
> (8 comments)
Thank you for the review.
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@7
PS6, Line 7: [util] add a wa
> With this patch, no warning is actually issued :) This patch just adds a wa
Done
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@10
PS6, Line 10: thread
: cache request
> What is 'thread cache request'?
Request is a verb here... And I have fixed the grammar
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@12
PS6, Line 12: hold t
> nit: 'acquire' or 'hold'
Done
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@15
PS6, Line 15: (especially for upserts)
> Is this essential? Any allocation on the heap will add to the lock contenti
Upsert is more likely to read from files, so it lead to more memory allocations.
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@16
PS6, Line 16: so much performance because of the lock contention of tcmalloc
> Even without enabling tcmalloc sampling a system with quite high throughput
Stack traces are actually where I found this problem. I have tested high throughput of upserts with and without sample allocations, there is a threefold difference.
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@18
PS6, Line 18: 1MB
> This is quite low, actually. Empirically, setting it to hundreds of gigaby
Got it, thank you!
http://gerrit.cloudera.org:8080/#/c/20716/6//COMMIT_MSG@20
PS6, Line 20: So give a warning on the parameter heap_sample_every_n_bytes in case
: that some users do not know the terrible influence of thi
> Even with the extra warning, users wouldn't know how bad the lock contentio
Unsafe tag is needed, I think. Done.
http://gerrit.cloudera.org:8080/#/c/20716/6/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/6/src/kudu/util/flags.cc@99
PS6, Line 99: Warning: Setting this flag to a positive value increases lock contention "
: "in tcmalloc;
> Consider removing this part as well: it might be a reasonable choice for a
Done
http://gerrit.cloudera.org:8080/#/c/20716/6/src/kudu/util/flags.cc@100
PS6, Line 100: he lower the value, the higher the lock contention.");
: TAG_FLAG(heap_sample_every_n_bytes, advanced);
: TAG_FLAG(heap_sample_ever
> Try "setting manually" what?
Done
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 7
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Comment-Date: Tue, 21 Nov 2023 18:13:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] Add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread cache
request some sample allocations from page cache, which will
compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput might lose so much performance
because of the lock contention of tcmalloc. Almost all the apply
threads are waiting for the spin lock.
So add a warning on --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Reviewed-on: http://gerrit.cloudera.org:8080/20716
Tested-by: Kudu Jenkins
Reviewed-by: Wang Xixu <14...@qq.com>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
Approvals:
Kudu Jenkins: Verified
Wang Xixu: Looks good to me, but someone else must approve
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 18
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Wang Xixu (Code Review)" <ge...@cloudera.org>.
Wang Xixu has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
Patch Set 17: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 17
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Thu, 21 Dec 2023 02:14:07 +0000
Gerrit-HasComments: No
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/20716/6/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/6/src/kudu/util/flags.cc@99
PS6, Line 99: A value such as 524288 (512KB) is a reasonable choice with relatively "
: "low overhead.
Consider removing this part as well: it might be a reasonable choice for a tiny test/development minicluster, but sampling every 512KB doesn't make any sense in a production system.
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 6
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Comment-Date: Tue, 21 Nov 2023 17:35:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#3).
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Issue a warning on parameter heap_sample_every_n_bytes.
Parameter heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a value greater than 0 will make thread
cache request to page heap for some sample allocations, which
will occupy the lock of page heap. So does the free operation
of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc.
Almost all the apply threads are waiting for the spin lock, even if
setting the parameter to a higher value like 1M.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/3
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 3
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#8).
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread
cache request some sample allocations from page cache, which
will compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc,
Almost all the apply threads are waiting for the spin lock.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/8
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 8
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#9).
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread
cache request some sample allocations from page cache, which
will compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc,
Almost all the apply threads are waiting for the spin lock.
So give a warning on the -- heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/9
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 9
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#15).
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] Add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread cache
request some sample allocations from page cache, which will
compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput might lose so much performance
because of the lock contention of tcmalloc. Almost all the apply
threads are waiting for the spin lock.
So add a warning on --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/15
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 15
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#10).
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread
cache request some sample allocations from page cache, which
will compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc,
Almost all the apply threads are waiting for the spin lock.
So give a warning on the --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/10
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 10
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Song Jiacheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
Patch Set 8:
> Patch Set 7:
>
> (6 comments)
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 8
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Mon, 18 Dec 2023 07:22:12 +0000
Gerrit-HasComments: No
[kudu-CR] [util] Add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#17).
Change subject: [util] Add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] Add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread cache
request some sample allocations from page cache, which will
compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput might lose so much performance
because of the lock contention of tcmalloc. Almost all the apply
threads are waiting for the spin lock.
So add a warning on --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/17
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 17
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] [util] add a warning into 'heap sample every n bytes' flag description.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Yingchun Lai, Kudu Jenkins, Wang Xixu,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#11).
Change subject: [util] add a warning into 'heap_sample_every_n_bytes' flag description.
......................................................................
[util] add a warning into 'heap_sample_every_n_bytes' flag description.
Parameter --heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a positive value will make thread
cache request some sample allocations from page cache, which
will compete the lock of page heap with other threads. So does
the free operation of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc,
Almost all the apply threads are waiting for the spin lock.
So give a warning on --heap_sample_every_n_bytes in case that
some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/11
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 11
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#2).
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Issue a warning on parameter heap_sample_every_n_bytes.
Parameter heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a value greater than 0 will make thread
cache request to page heap for some sample allocations, which
will occupy the lock of page heap. So does the free opeation
of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc.
Almost all the apply threads are waiting for the spin lock, even if
setting the parameter to a higher value like 1M.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/2
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 2
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/20716 )
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Patch Set 4: Code-Review+1
(3 comments)
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc
File src/kudu/util/flags.cc:
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc@101
PS4, Line 101: leader
nit: lead
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc@102
PS4, Line 102: wanted
maybe nit: use 'desired'
http://gerrit.cloudera.org:8080/#/c/20716/4/src/kudu/util/flags.cc@546
PS4, Line 546:
nit: remove extra indentation
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 4
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 21 Nov 2023 09:35:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] Issue a warning on parameter heap sample every n bytes.
Posted by "Song Jiacheng (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/20716
to look at the new patch set (#4).
Change subject: Issue a warning on parameter heap_sample_every_n_bytes.
......................................................................
Issue a warning on parameter heap_sample_every_n_bytes.
Parameter heap_sample_every_n_bytes is actually a parameter of
tcmalloc. Setting it to a value greater than 0 will make thread
cache request to page heap for some sample allocations, which
will occupy the lock of page heap. So does the free operation
of the sample.
A tserver with high throughput(especially for upserts) might lose
so much performance because of the lock contention of tcmalloc.
Almost all the apply threads are waiting for the spin lock, even if
setting the parameter to a higher value like 1MB.
So give a warning on the parameter heap_sample_every_n_bytes in case
that some users do not know the terrible influence of this parameter.
Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
---
M src/kudu/util/flags.cc
1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/16/20716/4
--
To view, visit http://gerrit.cloudera.org:8080/20716
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I00d55126f0cad0ac6cc130576cc45808f96f0a47
Gerrit-Change-Number: 20716
Gerrit-PatchSet: 4
Gerrit-Owner: Song Jiacheng <so...@thinkingdata.cn>
Gerrit-Reviewer: Kudu Jenkins (120)