Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16343 )
Change subject: KUDU-1587 part 2: reject write ops if apply queue is overloaded ...................................................................... Patch Set 10: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/16343/10/src/kudu/integration-tests/same_tablet_concurrent_writes-itest.cc File src/kudu/integration-tests/same_tablet_concurrent_writes-itest.cc: http://gerrit.cloudera.org:8080/#/c/16343/10/src/kudu/integration-tests/same_tablet_concurrent_writes-itest.cc@386 PS10, Line 386: ASSERT_NE("", SetCommandLineOptionWithMode("num_inserter_threads", Why can't we use FLAGS_* for these configurations? Or just ignore the gflags altogether and just pass these values directly to Run()? http://gerrit.cloudera.org:8080/#/c/16343/10/src/kudu/kserver/kserver.cc File src/kudu/kserver/kserver.cc: http://gerrit.cloudera.org:8080/#/c/16343/10/src/kudu/kserver/kserver.cc@59 PS10, Line 59: DEFINE_uint32(tablet_apply_pool_overload_threshold_ms, 0, : "The threshold for the queue time of the 'apply' thread pool " : "to enter and exit overloaded state. Once the queue stalls and " : "its queue times become longer than the specified threshold, it " : "enters the overloaded state. Tablet server rejects incoming " : "write requests with some probability when its apply queue is " : "overloaded. The longer the apply queue stays overloaded, the " : "greater the probability of the rejection. In addition, the more " : "row operations a write request has, the greater the probablity " : "of the rejection. The apply queue exits the overloaded state " : "when queue times drop below the specified threshold. Set this " : "flag to 0 to disable the behavior described above."); : TAG_FLAG(tablet_apply_pool_overload_threshold_ms, advanced); This mentions Tablet Servers specifically, but masters also use KuduServers. Will this be a published gflag for masters as well? Is it possible to define this in tablet_service.cc and declare it here? http://gerrit.cloudera.org:8080/#/c/16343/6/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/16343/6/src/kudu/tserver/tablet_server-test.cc@4365 PS6, Line 4365: class OpApplyQueueTest : public TabletServerTestBase { > Could you also add a small test that even with a heavily throttled queue, c Done -- To view, visit http://gerrit.cloudera.org:8080/16343 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d7688d6fa832e606b8efc4549568fa52dfa1931 Gerrit-Change-Number: 16343 Gerrit-PatchSet: 10 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Fri, 28 Aug 2020 23:53:37 +0000 Gerrit-HasComments: Yes
