Mike Percy has posted comments on this change. Change subject: KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover. ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/7456/2//COMMIT_MSG Commit Message: Line 7: KUDU-2033 (part 1). Add write and stop/start in a loop to TestLeaderFailover. nit: Avoid period at end of subject line in a commit message http://gerrit.cloudera.org:8080/#/c/7456/2/java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java File java/kudu-client/src/test/java/org/apache/kudu/client/BaseKuduTest.java: PS2, Line 351: Integer Can we return an int instead of an Integer here and below? I can't see any reason to use a boxed primitive here. http://gerrit.cloudera.org:8080/#/c/7456/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestLeaderFailover.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestLeaderFailover.java: Line 42: * This test writes 3 rows. Then in a loop, it kills the leader, then tries to write inner_row rows. Verifying nit: line length should be <= 100 chars Line 49: public void testFailover() throws Exception { Can we keep the existing simple test and add an additional test that uses this "stress test" approach? Line 51: int startRows = 3; style: use final and all caps for constants Line 67: for (int j = i; j < i + innerRows; j++) { I find this logic a bit difficult to follow. I'd suggest keeping a counter like currentRows and just increment that each time you insert a row, then use simple i and j counters as needed to control the number of iterations per loop. something like: int currentRows = 0; for (int i = 0; i < startRows; i++) { session.apply(createBasicSchemaInsert(table, i)); currentRows++; } for (int i = 0; i < numIterations; i++) { // ... for (int j = 0; j < rowsPerIteration; j++) { session.apply(...); currentRows++; } } // ... assertEquals(totalRowsToInsert, countRowsInScan(scanner)); Line 77: assertEquals(i + innerRows, countRowsInScan(scanner)); these assertions should probably be assert eventually -- To view, visit http://gerrit.cloudera.org:8080/7456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie93a15084a4c4c0748dc74c005b8313f443a5ba9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Edward Fancher <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-HasComments: Yes
