Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/9446 )
Change subject: IMPALA-6554: Fix a race in DequeueDeferredRpc() ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/9446/4/be/src/runtime/krpc-data-stream-mgr.cc File be/src/runtime/krpc-data-stream-mgr.cc: http://gerrit.cloudera.org:8080/#/c/9446/4/be/src/runtime/krpc-data-stream-mgr.cc@231 PS4, Line 231: Deferred in this case, the "deferred" doesn't make sense since it hasn't yet been differed, right? Since the mem_tracker is no longer baked in and we will use this for cases other than deferred I think we should not include that in this version. http://gerrit.cloudera.org:8080/#/c/9446/4/be/src/runtime/krpc-data-stream-mgr.cc@287 PS4, Line 287: Deferred this case also not deferred http://gerrit.cloudera.org:8080/#/c/9446/4/be/src/runtime/krpc-data-stream-recvr.h File be/src/runtime/krpc-data-stream-recvr.h: http://gerrit.cloudera.org:8080/#/c/9446/4/be/src/runtime/krpc-data-stream-recvr.h@175 PS4, Line 175: mem_tracker_ I think it'd be clearer to also rename this field to deferred_rpc_tracker_ or similar, especially now that it's used as an argument to the shared routine that will do the responding. http://gerrit.cloudera.org:8080/#/c/9446/4/be/src/service/data-stream-service.h File be/src/service/data-stream-service.h: http://gerrit.cloudera.org:8080/#/c/9446/4/be/src/service/data-stream-service.h@62 PS4, Line 62: Deferred as mentioned earlier, since this is used for non-deferred case as well, let's not put deferred given the new refactoring. and remove "deferred" from comment. -- To view, visit http://gerrit.cloudera.org:8080/9446 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4a3b0789633c7b8bc898381d509e2af769f0e069 Gerrit-Change-Number: 9446 Gerrit-PatchSet: 4 Gerrit-Owner: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Thu, 01 Mar 2018 17:39:47 +0000 Gerrit-HasComments: Yes
