Tim Armstrong 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: (7 comments) 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? Done 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? Done 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? Done 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' Done 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/04fd9ae oh nice catch, thanks 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? Oops, I tried to replace condition_variable and didn't remove this change when I abandoned it. 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 Done -- 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:30:47 +0000 Gerrit-HasComments: Yes
