Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/21570 )
Change subject: KUDU-3371 Parameterize some options of RocksDB ...................................................................... Patch Set 2: (26 comments) http://gerrit.cloudera.org:8080/#/c/21570/1//COMMIT_MSG Commit Message: PS1: > I guess this patch should be rebased on top of https://gerrit.cloudera.org/ Yes, let's merge that one https://gerrit.cloudera.org/#/c/21560/ at first. http://gerrit.cloudera.org:8080/#/c/21570/1//COMMIT_MSG@10 PS1, Line 10: some : options for Kudu > Just curious: how did you find those are 'necessary' ones among many? It's not absolutely, we may remove or add some options in the future. I removed "necessary" word. http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/cfile/block_cache.cc File src/kudu/cfile/block_cache.cc: http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/cfile/block_cache.cc@119 PS1, Line 119: ($0 MB vs. $1 MB). > Substitution parameters are missing -- this might crash. Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/cfile/block_cache.cc@120 PS1, Line 120: performance problems > I'm curious: what sort of performance issues might be there in such a case Typically, the data size is much larger than metadata size, the data size needs more cache than metadata. If the configuration of metadata is larger than that of data, the memory for metadata is considered to be a bit of waste. So it's encouraged to set --block_cache_capacity_mb larger than --log_container_rdb_block_cache_capacity_mb. http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/cfile/block_cache.cc@123 PS1, Line 123: > If it's just a warning (at least it seems so due to LOG(WARNING) above), th Just a typo, it's a warning and should return true. http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc File src/kudu/fs/dir_manager.cc: http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@76 PS1, Line 76: m filter, fo > for details Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@76 PS1, Line 76: Average numb > Average number of bits Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@79 PS1, Line 79: TAG_FLAG(log_container_rdb_bits_per_key, advanced); > Shouldn't all these new flags be tagged as 'experimental'? Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@82 PS1, Line 82: uint32 > Would uint32 suffice here? Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@83 PS1, Line 83: Mi > MiB? Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@87 PS1, Line 87: > Would uint32 suffice here? Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@88 PS1, Line 88: ackground_jobs, 8, > concurrent background jobs Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@89 PS1, Line 89: The max > shared between Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@92 PS1, Line 92: TAG_FLAG(log_ > Would uint32 suffice here? Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@107 PS1, Line 107: b_m > nit: drop 'the' Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@107 PS1, Line 107: ntaine > The amount Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@113 PS1, Line 113: > are Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@114 PS1, Line 114: > nit: 'not empty' or 'non-empty' Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@114 PS1, Line 114: rime > dir Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@120 PS1, Line 120: be used as the log file > Maximum size Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@121 PS1, Line 121: ck > specified Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@125 PS1, Line 125: ize, 8 << > to keep Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@125 PS1, Line 125: og_cont > Maximum number of ... Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@129 PS1, Line 129: > What are the units of the limit? Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@133 PS1, Line 133: > Would int32 suffice here? Done http://gerrit.cloudera.org:8080/#/c/21570/1/src/kudu/fs/dir_manager.cc@135 PS1, Line 135: > the number Done -- To view, visit http://gerrit.cloudera.org:8080/21570 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6a6da6868511fe69af528ca122fcaa98cfc9dac4 Gerrit-Change-Number: 21570 Gerrit-PatchSet: 2 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Sun, 14 Jul 2024 03:49:28 +0000 Gerrit-HasComments: Yes
