Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10266 )
Change subject: Add validation and docs on setting block_cache_capacity_mb too high ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/10266/5/src/kudu/cfile/block_cache.cc File src/kudu/cfile/block_cache.cc: http://gerrit.cloudera.org:8080/#/c/10266/5/src/kudu/cfile/block_cache.cc@80 PS5, Line 80: nit: extra space http://gerrit.cloudera.org:8080/#/c/10266/5/src/kudu/integration-tests/client-stress-test.cc File src/kudu/integration-tests/client-stress-test.cc: http://gerrit.cloudera.org:8080/#/c/10266/5/src/kudu/integration-tests/client-stress-test.cc@241 PS5, Line 241: // The block cache capacity needs to be disabled. See the group validator > Hmm, this just disables the block cache outright, correct? Reword to say th An alternative approach might be using --force_block_cache_capacity (since it's been introduced already) instead of disabling the block cache at all? http://gerrit.cloudera.org:8080/#/c/10266/5/src/kudu/integration-tests/raft_consensus-itest.cc File src/kudu/integration-tests/raft_consensus-itest.cc: http://gerrit.cloudera.org:8080/#/c/10266/5/src/kudu/integration-tests/raft_consensus-itest.cc@1803 PS5, Line 1803: // We also need to limit the block cache capacity in tandem. See the group > Again, isn't this outright disabling? Maybe, since the --force_block_cache_capacity is introduced, it would be possible to use it here and don't disable the block cache at all? http://gerrit.cloudera.org:8080/#/c/10266/4/src/kudu/util/process_memory.cc File src/kudu/util/process_memory.cc: http://gerrit.cloudera.org:8080/#/c/10266/4/src/kudu/util/process_memory.cc@a182 PS4, Line 182: : : : : : : : > It wasn't printed before this change AFAICT, and now the validator makes it Ah, I see -- that makes sense. -- To view, visit http://gerrit.cloudera.org:8080/10266 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia99fa95c578235beb3a1c72a33a22b4d8ff4800c Gerrit-Change-Number: 10266 Gerrit-PatchSet: 5 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Thu, 03 May 2018 20:23:51 +0000 Gerrit-HasComments: Yes
