Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16312 )
Change subject: [tserver] add test to reproduce KUDU-1587 conditions ...................................................................... Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/16312/1/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/16312/1/src/kudu/tserver/tablet_server-test.cc@4368 PS1, Line 4368: num_data_dirs > Would it also make sense to reduce the number of apply threads to 1? Would Well, I'd rather keep it closer to "real" configurations. That also helps to avoid extra changes introducing max pool size for the apply queue. Or you think it's better to introduce control knobs for the apply queue max_size anyways (maybe, in a separate changelist)? http://gerrit.cloudera.org:8080/#/c/16312/1/src/kudu/tserver/tablet_server-test.cc@4387 PS1, Line 4387: re the > nit: incomplete? Done http://gerrit.cloudera.org:8080/#/c/16312/1/src/kudu/tserver/tablet_server-test.cc@4446 PS1, Line 4446: EXPECT_LT(h->MaxValue(), : kLatencyMs * 1000 * kNumCalls / base::NumCPUs()); : EXPECT_LT(h->ValueAtPercentile(99), : kLatencyMs * 1000 * kNumCalls / (2 * base::NumCPUs())); : EXPECT_LT(h->ValueAtPercentile(50), : kLatencyMs * 1000 * kNumCalls / (4 * base::NumCPUs())); > Making sure I understand these assertions: if we had a single core, we'd ex Well, these are rather simple heuristics. I'm not sure we want to draw some exact lines here based on the parameters for CoDel. I added an extra comment to clarify on this. http://gerrit.cloudera.org:8080/#/c/16312/1/src/kudu/tserver/tablet_server-test.cc@4463 PS1, Line 4463: : EXPECT_LT(h->MaxValue(), kNumCalls / 2); : EXPECT_LT(h->MeanValue(), kNumCalls / 4); > Same here, right? These multipliers will be adjusted depending on the imple These are simple heuristics as well. They depend on the injected latency and the 'overloaded' threshold for the apply queue. I added this as a comment. BTW, the nice feature of the CoDel algorithm is that it has almost no configuration parameters. http://gerrit.cloudera.org:8080/#/c/16312/1/src/kudu/tserver/tablet_server-test.cc@4468 PS1, Line 4468: // Some calls should be rejected by the apply queue, so the maximum queue : // time should be much lower than xxx. > nit: move this above L4436? Or is this referring to the RPC queue? Done -- To view, visit http://gerrit.cloudera.org:8080/16312 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I515a1b26152680ee9b9361afcf84fec39b8f962d Gerrit-Change-Number: 16312 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[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-Comment-Date: Sat, 15 Aug 2020 00:15:27 +0000 Gerrit-HasComments: Yes
