Alexey Serbin has posted comments on this change. Change subject: WIP: KUDU-1767. Create a client flush interleave test ......................................................................
Patch Set 2: (7 comments) It looks like a nice test to isolate this problem! 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 #include <vector> #incldue <glog/logging.h> #include <gtest/gtest.h> PS2, Line 44: / Check that attempts to scan prior to the ancient history mark fail The comment looks outdated. PS2, Line 71: KuduUpdate* update nit: consider using unique_ptr to wrap the update op in case of any of the ASSERT_OK() below fail (I suspect that could lead to ASAN warnings) 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 inserts those updates in reverse order and then do pop_back() if using the vector container. PS2, Line 91: LOG(INFO) Does it make sense to use VLOG() here instead? Or it's crucial to have those lines always printed out? PS2, Line 97: "int_val=$0" What do those lines look like? My concern is that some portion of anomalies could be missed here; e.g., consider 'int_val=1' and 'int_val=10' and alike. May be, it's worth to update the check to make sure those cases are also covered. 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. -- 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 <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
