Adar Dembo has posted comments on this change. Change subject: KUDU-1970: node density integration test ......................................................................
Patch Set 9: (6 comments) http://gerrit.cloudera.org:8080/#/c/6662/9/src/kudu/integration-tests/dense_node-itest.cc File src/kudu/integration-tests/dense_node-itest.cc: PS9, Line 68: DenseNodeTest > in general would it make sense to have this test be parameterized and also Good question. I am interested in how fsync affects certain operations (e.g. CreateTablet() storm on the tserver when creating a table, TSHeartbeat() storm on the master when it starts), but I don't think it's necessary to always run with it on, at least not yet. For now I'll add a gflag that will enable fsync if requested. Maybe once more work is done to improve scalability we'll work it into this test as a parameter. PS9, Line 71: DenseNodeTest > also this test seems to be more of a benchmark that an test. change the nam I'll add some documentation. I was torn on whether to name it "-bench" or "-itest", since it serves as both. I went with "-itest" because there's precedence for that in benchmarks.sh: plenty of tests, itests, and stress tests are benchmarked. PS9, Line 100: // Inject steroids into the MM. > hum 100 seems excessive an unrealistic even for denser nodes, maybe 10 woul The goal isn't to mirror a real deployment; it's to get the MM to flush: 1. As little data as possible, and 2. As often as possible. I actually used 1000 here at one point, but found throughput to be better with 100. When I switch it to 10, I see about half as many blocks generated (though the number of LBM containers remains about the same). PS9, Line 131: for (int i = 1; i < FLAGS_num_columns; i++) { : b.AddColumn(Substitute("i$0", i))->Type(KuduColumnSchema::INT32)->NotNull(); : } > would it make sense to also have larger string columns? seems like this wil But the goal of the test is to maximize metadata, which means smaller columns and rowsets (and more of them) is desirable. PS9, Line 145: us > nit: s/us/the test Done http://gerrit.cloudera.org:8080/#/c/6662/9/src/kudu/integration-tests/external_mini_cluster.h File src/kudu/integration-tests/external_mini_cluster.h: PS9, Line 141: starting > what is "starting" here? writing the instance file? booting up everything? Writing out the instance file. I'll clarify the comment. -- To view, visit http://gerrit.cloudera.org:8080/6662 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie9b5d01557eb41d386ce92f576ed01ec658e8e7d Gerrit-PatchSet: 9 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: David Ribeiro Alves <davidral...@gmail.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
