Pranay Singh has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8949 )

Change subject: IMPALA-6075: Add Impala daemon metric for catalog version.
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/8949/3/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/8949/3/be/src/service/impala-server.h@1000
PS3, Line 1000: /// Update the metric to store the current version of catalog 
used by impalad.
> comment doesn't seem to reflect the function implementation, plz update
Done


http://gerrit.cloudera.org:8080/#/c/8949/3/be/src/util/impalad-metrics.h
File be/src/util/impalad-metrics.h:

http://gerrit.cloudera.org:8080/#/c/8949/3/be/src/util/impalad-metrics.h@112
PS3, Line 112: Catalog
> nit: catalog (lowercase)
Done


http://gerrit.cloudera.org:8080/#/c/8949/3/be/src/util/impalad-metrics.cc
File be/src/util/impalad-metrics.cc:

http://gerrit.cloudera.org:8080/#/c/8949/3/be/src/util/impalad-metrics.cc@79
PS3, Line 79: const char* ImpaladMetricKeys::CATALOG_VERSION =
            :     "catalog.curr-version";
            : const char* ImpaladMetricKeys::CATALOG_TOPIC_VERSION =
            :     "catalog.curr-topic";
            : const char* ImpaladMetricKeys::CATALOG_SERVICE_ID =
            :     "catalog.curr-serviceid";
> single lines (if they fit)
Done but the above definitions seems to split in to two lines


http://gerrit.cloudera.org:8080/#/c/8949/3/be/src/util/impalad-metrics.cc@249
PS3, Line 249:
> Why not ""?
Done



--
To view, visit http://gerrit.cloudera.org:8080/8949
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id2307eb434561ed74ff058106541c0ebda017d97
Gerrit-Change-Number: 8949
Gerrit-PatchSet: 3
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]>
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Fri, 19 Jan 2018 01:51:36 +0000
Gerrit-HasComments: Yes

Reply via email to