impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-6225: Part 2: Query profile date-time strings should have ns precision.
Date Fri, 08 Dec 2017 22:31:23 GMT
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/8784
)

Change subject: IMPALA-6225: Part 2: Query profile date-time strings should have ns precision.
......................................................................


Patch Set 3:

(3 comments)

Thanks for the iteration. I think this is almost done. Just a few more comments.

http://gerrit.cloudera.org:8080/#/c/8784/3/tests/common/impala_service.py
File tests/common/impala_service.py:

http://gerrit.cloudera.org:8080/#/c/8784/3/tests/common/impala_service.py@64
PS3, Line 64:   def get_thrift_profile(self, query_id, timeout=10, interval=1):
please add pydoc. This will get used again, I'm sure.


http://gerrit.cloudera.org:8080/#/c/8784/3/tests/common/impala_service.py@73
PS3, Line 73:       LOG.info("Thrift profile for query %s not yet available.")
Is there a more specific exception you can use? Alternately, put the try/except around line
68. I worry that zlib.decompress, and base64, and deserialize can all throw interesting exceptions
that I wouldn't want to swallow.

Also: should this be returning None? or re-throwing? Are you intentionally returning an empty
thrift thingy?


http://gerrit.cloudera.org:8080/#/c/8784/3/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/8784/3/tests/query_test/test_observability.py@125
PS3, Line 125:     service = ImpalaCluster().get_first_impalad().service
I think this maybe should be

    service = self.cluster.impalads[0].service

i.e., you shouldn't be creating a new ImpalaCluster(), but rather using the onet hat your
test has provided you.

"git grep self.cluster.get" is more prominent than "git grep ImpalaCluster\(", though the
latter certainly identifies several classes of this bug.

09:03:15 [130]pannier Impala [maven ?*] ~/src/Impala
$git grep self.cluster.get
bin/remote_data_load.py:        services = dict((s.type, s) for s in self.cluster.get_all_services())
tests/common/failure_injector.py:    self.cluster.get_impala_service().set_process_auto_restart_config(value=True)
tests/common/failure_injector.py:    # self.cluster.get_impala_service().restart()
tests/common/failure_injector.py:    num_impalad_procs = len(self.cluster.get_impala_service().get_all_impalad_processes())
tests/common/failure_injector.py:               self.cluster.get_impala_service().get_all_impalad_processes())
tests/common/failure_injector.py:    state_store = self.cluster.get_impala_service().get_state_store_process()
tests/comparison/cluster.py:    endpoint = self.cluster.get_hadoop_config("dfs.namenode.http-address",
tests/comparison/cluster.py:      port = self.cluster.get_hadoop_config("dfs.https.port",
20102)
tests/comparison/cluster.py:        self.cluster.get_hadoop_config("hive.metastore.warehouse.dir")).path
tests/custom_cluster/test_insert_behaviour.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_insert_behaviour.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_query_concurrency.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_query_expiration.py:    impalad = self.cluster.get_first_impalad()
tests/custom_cluster/test_query_expiration.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_query_expiration.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_s3a_access.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_scratch_disk.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_seq_file_filtering.py:    impalad = self.cluster.get_any_impalad()
tests/custom_cluster/test_session_expiration.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_catalog_hms_failures.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_catalog_hms_failures.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_catalog_hms_failures.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:    worker_impalad = self.cluster.get_different_impalad(impalad)
tests/experiments/test_process_failures.py:    impalad = self.cluster.get_any_impalad()
tests/experiments/test_process_failures.py:    impalad = self.cluster.get_any_impalad()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3421a34cc029ebca551730084c7cbd402d5c109
Gerrit-Change-Number: 8784
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga <zoram@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Zoram Thanga <zoram@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Dec 2017 22:31:23 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message