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 <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Song Jiacheng <[email protected]> Gerrit-Comment-Date: Tue, 21 Nov 2023 18:13:31 +0000 Gerrit-HasComments: Yes
