Mike Percy has posted comments on this change. Change subject: WIP: KUDU-1767. Create a client flush interleave test ......................................................................
Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/5256/2/src/kudu/integration-tests/client_flush_interleave-itest.cc File src/kudu/integration-tests/client_flush_interleave-itest.cc: Line 17: > nit: consider adding Included the first two, but gtest is superfluous when inheriting from MiniClusterITestBase. PS2, Line 44: / Check that attempts to scan prior to the ancient history mark fail > The comment looks outdated. Done PS2, Line 71: KuduUpdate* update > nit: consider using unique_ptr to wrap the update op in case of any of the Done PS2, Line 76: for (int i = 0; i < kMax; i++) { : ASSERT_OK(session->Apply(updates[i])); : session->FlushAsync(nullptr); : } > if using unique_ptr, probably you could use deque instead of vector or inse Do you have a specific concern? deques tend to be slower than vectors / arrays because the list links defeat memory locality / prefetching. Using a vector in this manner seems reasonable to me both from a performance and a readability standpoint. How about I just reserve() the capacity in advance? PS2, Line 91: LOG(INFO) > Does it make sense to use VLOG() here instead? Or it's crucial to have tho Done PS2, Line 97: "int_val=$0" > What do those lines look like? My concern is that some portion of anomalie Added a ")" which should do it. PS2, Line 101: /* : KuduScanner scanner(table.get()); : ASSERT_OK(scanner.SetReadMode(KuduScanner::READ_AT_SNAPSHOT)); : Status s = scanner.Open(); : ASSERT_TRUE(s.IsInvalidArgument()) << s.ToString(); : ASSERT_STR_CONTAINS(s.ToString(), "Snapshot timestamp is earlier than the ancient history mark"); : */ > nit: consider removing this commented code. Done -- To view, visit http://gerrit.cloudera.org:8080/5256 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3886f70246c6ef3f94acda6074d02d4fb199cf50 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: David Ribeiro Alves <dral...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
