Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17743 )
Change subject: KUDU-1954 [maintenance] Add an optional flush-only thread pool ...................................................................... Patch Set 3: (5 comments) Quick first pass so far, and questions regarding the approach. http://gerrit.cloudera.org:8080/#/c/17743/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17743/3//COMMIT_MSG@7 PS3, Line 7: KUDU-1954 Reading through the RocksDB link https://github.com/facebook/rocksdb/blob/4361d6d16380f619833d58225183cbfbb2c7a1dd/include/rocksdb/options.h#L599-L658 it looks like 'max_background_flushes', as well as guidance around the high priority and low priority pool, is deprecated, in favor of a single 'max_background_jobs'. Or were you referring to a different configuration? In general, I wonder if, for such deployments, we should lower the --memory_pressure_percentage flag for slower deployments. I'm curious if you've tried that, now that Kudu no longer starves compactions when under memory pressure. http://gerrit.cloudera.org:8080/#/c/17743/3//COMMIT_MSG@9 PS3, Line 9: In some special environments, I'm curious whether these environments can be categorized as clusters with slow devices (e.g. HDDs). If so, one question I have is how much this actually improves things, especially if the bottleneck is IO. I also suspect this patch would improve the stability of memory usage, if the compaction threadpool is otherwise blocked on IO. It may not be, if all disks are near saturated, but the fact that Kudu spreads data across disks might help our case here. http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc File src/kudu/util/maintenance_manager.cc: http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc@68 PS3, Line 68: specially nit: "specifically" http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc@71 PS3, Line 71: maintenance_manager_num_flush_threads maintenance_manager_num_threads? http://gerrit.cloudera.org:8080/#/c/17743/3/src/kudu/util/maintenance_manager.cc@535 PS3, Line 535: return {low_io_most_logs_retained_bytes_op, : std::move(notes), : LAUNCH_POOL(low_io_most_logs_retained_bytes_op->type(), flush_pool, general_pool)}; Why bother evaluating which pool to pick here, rather than pushing that burden onto the caller of FindBestOp()? -- To view, visit http://gerrit.cloudera.org:8080/17743 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6dca9d87e7287cc04482a1c18b1e0eefc5f9bcb1 Gerrit-Change-Number: 17743 Gerrit-PatchSet: 3 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Tue, 10 Aug 2021 23:52:19 +0000 Gerrit-HasComments: Yes
