Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15208 )
Change subject: IMPALA-8013: switch from boost to std locks ...................................................................... Patch Set 7: Code-Review+1 (7 comments) mostly nits http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/exec/hdfs-scan-node-base.h File be/src/exec/hdfs-scan-node-base.h: http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/exec/hdfs-scan-node-base.h@27 PS7, Line 27: #include <boost/thread/shared_mutex.hpp> nit: should this be on line 28, and line 27 should be a newline? http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/bufferpool/buffer-pool-internal.h File be/src/runtime/bufferpool/buffer-pool-internal.h: http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/bufferpool/buffer-pool-internal.h@79 PS7, Line 79: nit: delete new line? http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/coordinator-backend-state.h@23 PS7, Line 23: #include <mutex> nit: same as before, group this with the other std library includes? http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/io/scan-range.cc File be/src/runtime/io/scan-range.cc: http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/io/scan-range.cc@306 PS7, Line 306: std nit: remote std here and in 'std::mutex' http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/query-exec-mgr.h File be/src/runtime/query-exec-mgr.h: http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/query-exec-mgr.h@20 PS7, Line 20: #include <mutex> : #include <unordered_map> i think this was removed in https://github.com/apache/impala/commit/04fd9ae268d89b07e2a692a916bf2ddcfb2e351b - don't think are needed, right? http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/runtime-filter-bank.h File be/src/runtime/runtime-filter-bank.h: http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/runtime/runtime-filter-bank.h@31 PS7, Line 31: #include "util/condition-variable.h" is this used? http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/service/impala-hs2-server.cc File be/src/service/impala-hs2-server.cc: http://gerrit.cloudera.org:8080/#/c/15208/7/be/src/service/impala-hs2-server.cc@1071 PS7, Line 1071: std nit: can remove the std -- To view, visit http://gerrit.cloudera.org:8080/15208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I81d37490d05049919270eb6406fb3d787f78f92f Gerrit-Change-Number: 15208 Gerrit-PatchSet: 7 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Sahil Takiar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 19 Feb 2020 17:00:15 +0000 Gerrit-HasComments: Yes
