Michael Brown has posted comments on this change. Change subject: IMPALA-2013: Reintroduce steps for checking HBase health in run-hbase.sh ......................................................................
Patch Set 5: (9 comments) Under what circumstances is more than one zookeeper needed? http://gerrit.cloudera.org:8080/#/c/4348/5//COMMIT_MSG Commit Message: PS5, Line 11: The original script was problematic for a couple of reasons (most : notably because it queried the wrong Zookeeper node) so we stopped : using it during our mini-cluster HBase start up procedure. Note quite right. I didn't use it because it wasn't clear it was needed anymore, the so-called "HBase race" had been fixed. I didn't notice the master vs. rs discrepancy at that time. http://gerrit.cloudera.org:8080/#/c/4348/5/testdata/bin/check-hbase-nodes.py File testdata/bin/check-hbase-nodes.py: PS5, Line 50: parser.add_argument('--timeout', '-t', action='store' I find action='store' to be redundant since that's the default. PS5, Line 52: 'Default is 30 seconds.')) Don't hardcode 30. Use a format string and read from TIMEOUT_SECONDS. PS5, Line 56: 'e.g, 0.0.0.0:5070. Default is localhost:5070.')) Use a format string and read from HDFS_HOST. PS5, Line 60: 'e.g, 0.0.0.0:2181. Default is localhost:2181.')) Use a format string and read from ZK_HOSTS. PS5, Line 66: '/hbase/master and /hbase/rs.') Use a format string. Maybe use '-n '.join(HBASE_NODES) as part of it? PS5, Line 127: hdfs_client.list('/') What does this do? PS5, Line 125: try: : hdfs_client = InsecureClient('http://' + args.hdfs_host) : hdfs_client.list('/') : except requests.exceptions.ConnectionError as e: : msg = 'Could not connect to HDFS web host http://{0} - {1}'.format(args.hdfs_host, e) : LOGGER.error(msg) : sys.exit(1) : : zk_client = connect_to_zookeeper(args.zookeeper_hosts, args.timeout) : errors = sum([check_hbase_node(zk_client, node, args.timeout) for node in args.nodes]) I suggest you make this a method. The __main__ can get the arguments, the errors, and print the error and exit accordingly. It would be better to have a separate method to make a connection and do the query work. PS5, Line 135: if errors: It would be better to have the explicit numerical > 0 comparison. -- To view, visit http://gerrit.cloudera.org:8080/4348 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9b81f3cfb6ea0ba7b18ce5fcd5d268f515c8b0c3 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: David Knupp <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Ishaan Joshi <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-HasComments: Yes
