[Impala-ASF-CR] IMPALA-7984: Port UpdateFilter() and PublishFilter() to KRPC

["Michael Ho (Code Review)" <ge...@cloudera.org>]

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 <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Aug 2019 20:30:50 +0000
Gerrit-HasComments: Yes