Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11182 )
Change subject: IMPALA_7127 (continued): initial fetch-from-catalogd implementation ...................................................................... Patch Set 1: (11 comments) http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/catalog/catalog.h File be/src/catalog/catalog.h: http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/catalog/catalog.h@79 PS1, Line 79: > nit: formatting. Done http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/catalog/catalog.cc File be/src/catalog/catalog.cc: http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/catalog/catalog.cc@95 PS1, Line 95: : > nit: can be formatted in 2 lines. Done http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/exec/catalog-op-executor.h File be/src/exec/catalog-op-executor.h: http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/exec/catalog-op-executor.h@55 PS1, Line 55: > nit: formatting. Done http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/exec/catalog-op-executor.cc File be/src/exec/catalog-op-executor.cc: http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/exec/catalog-op-executor.cc@289 PS1, Line 289: const TNetworkAddress& address = > Sure, will do. I don't want to add such assertions on the server side, howe Done http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/exec/catalog-op-executor.cc@295 PS1, Line 295: client.DoRpc(&CatalogServiceClientWrapper::GetPartialCatalogObject, req, resp)); > agreed. Something like OpenCensus's statistics and tags would be very usefu Done http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/service/fe-support.cc File be/src/service/fe-support.cc: http://gerrit.cloudera.org:8080/#/c/11182/1/be/src/service/fe-support.cc@539 PS1, Line 539: if (!status.ok()) { > oh, yea... I was debugging for a while with some LOG statements in here and Done http://gerrit.cloudera.org:8080/#/c/11182/1/common/thrift/CatalogService.thrift File common/thrift/CatalogService.thrift: http://gerrit.cloudera.org:8080/#/c/11182/1/common/thrift/CatalogService.thrift@313 PS1, Line 313: 1: optional Status.TStatus status in the case of "OK", maybe we don't need to send this? At least that's our usual style in Kudu -- missing status is an OK status. http://gerrit.cloudera.org:8080/#/c/11182/1/common/thrift/CatalogService.thrift@313 PS1, Line 313: optional Status.TStatus status : : 2: optional i64 object_version_number > Should these be required always? In the case of a bad status, we might not be able to send back an object version number http://gerrit.cloudera.org:8080/#/c/11182/1/common/thrift/CatalogService.thrift@405 PS1, Line 405: TGetPartialCatalogObjectResponse GetPartialCatalogObject(1: TGetPartialCatalogObjectRequest req); > line too long (99 > 90) Done http://gerrit.cloudera.org:8080/#/c/11182/1/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java: http://gerrit.cloudera.org:8080/#/c/11182/1/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@335 PS1, Line 335: return isReplicaCached ? (short) (hostIdx | ~REPLICA_HOST_IDX_MASK) : (short)hostIdx; > line too long (91 > 90) Done http://gerrit.cloudera.org:8080/#/c/11182/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/11182/1/fe/src/main/java/org/apache/impala/catalog/local/DirectMetaProvider.java@336 PS1, Line 336: // TODO Auto-generated method stub oops, forgot to implement this by passing through 'fds' above. done. -- To view, visit http://gerrit.cloudera.org:8080/11182 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If49207fc592b1cc552fbcc7199568b6833f86901 Gerrit-Change-Number: 11182 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 14 Aug 2018 00:56:10 +0000 Gerrit-HasComments: Yes
