Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13882 )
Change subject: IMPALA-7984: Port UpdateFilter() and PublishFilter() to KRPC ...................................................................... Patch Set 11: (6 comments) http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/runtime/backend-client.h File be/src/runtime/backend-client.h: http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/runtime/backend-client.h@19 PS11, Line 19: #define IMPALA_BACKEND_CLIENT_H Should this file be deleted after this change ? http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/data-stream-service.cc File be/src/service/data-stream-service.cc: http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/data-stream-service.cc@136 PS11, Line 136: "qs.get() == nullptr"; Is it a case in which it's legitimate for query state to be not found here ? This error message could be clearer: Query State not found for <query_id>. http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/impala-internal-service.h File be/src/service/impala-internal-service.h: http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/impala-internal-service.h@18 PS11, Line 18: #ifndef IMPALA_SERVICE_IMPALA_INTERNAL_SERVICE_H Shouldn't this file be deleted after this change ? http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/impala-internal-service.cc File be/src/service/impala-internal-service.cc: http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/impala-internal-service.cc@36 PS11, Line 36: ImpalaInternalService::ImpalaInternalService() { Same question. Shouldn't this file be deleted after this change ? Also, we can also skip initializing the backend service Thrift server during Impala initialization. http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/service/impala-server.h@342 PS11, Line 342: /// ImpalaInternalService rpcs Stale comment. ImpalaInternalService should not exist anymore after this change. http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/util/bloom-filter.h File be/src/util/bloom-filter.h: http://gerrit.cloudera.org:8080/#/c/13882/11/be/src/util/bloom-filter.h@79 PS11, Line 79: Thrift representation. Stale comment -- To view, visit http://gerrit.cloudera.org:8080/13882 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6b394796d250286510e157ae326882bfc01d387a Gerrit-Change-Number: 13882 Gerrit-PatchSet: 11 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Comment-Date: Thu, 01 Aug 2019 20:30:50 +0000 Gerrit-HasComments: Yes
