[GitHub] bigtop pull request #344: BIGTOP-3007: add properties to hive-site.xml for z...
Github user asfgit closed the pull request at: https://github.com/apache/bigtop/pull/344 ---
[GitHub] bigtop pull request #344: BIGTOP-3007: add properties to hive-site.xml for z...
Github user jamesbeedy commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/344#discussion_r172086152
--- Diff:
bigtop-packages/src/charm/hive/layer-hive/lib/charms/layer/bigtop_hive.py ---
@@ -36,31 +36,45 @@ def install(self, hbase=None):
# Prep config
roles = ['hive-client', 'hive-metastore', 'hive-server2']
metastore = "thrift://{}:9083".format(hookenv.unit_private_ip())
-if hbase:
-roles.append('hive-hbase')
-hb_connect = "{}:{}".format(hbase['host'],
hbase['master_port'])
-zk_connect = hbase['zk_connect']
-else:
-hb_connect = ""
-zk_connect = ""
--- End diff --
@kwmonroe totally, nice catch. I'll set those bits appropriately.
---
[GitHub] bigtop pull request #344: BIGTOP-3007: add properties to hive-site.xml for z...
Github user kwmonroe commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/344#discussion_r172070816
--- Diff:
bigtop-packages/src/charm/hive/layer-hive/lib/charms/layer/bigtop_hive.py ---
@@ -36,31 +36,45 @@ def install(self, hbase=None):
# Prep config
roles = ['hive-client', 'hive-metastore', 'hive-server2']
metastore = "thrift://{}:9083".format(hookenv.unit_private_ip())
-if hbase:
-roles.append('hive-hbase')
-hb_connect = "{}:{}".format(hbase['host'],
hbase['master_port'])
-zk_connect = hbase['zk_connect']
-else:
-hb_connect = ""
-zk_connect = ""
--- End diff --
Upon closer inspection, there's a side effect of not forcefully setting
this to `""`. With no value, we fall into the default provided by
`cluster.yaml`:
https://github.com/apache/bigtop/blob/master/bigtop-deploy/puppet/hieradata/bigtop/cluster.yaml#L191
which falls up to the hbase config:
https://github.com/apache/bigtop/blob/master/bigtop-deploy/puppet/hieradata/bigtop/cluster.yaml#L132
which makes the namenode (bigtop head node) the default value. This means
on a deployed hive charm, you'll get the following in the config, even when
hbase is not deployed at all:
```
hbase.zookeeper.quorum
ip-172-31-68-129.ec2.internal
http://wiki.apache.org/hadoop/Hive/HBaseIntegration
```
That hostname is for my namenode. This may not be a problem as the hive
unit in this case has no hbase roles defined, but I haven't put "hive plus some
hbase config even when hbase isnt there" to the test to see if anything is
afoul.
---
[GitHub] bigtop pull request #344: BIGTOP-3007: add properties to hive-site.xml for z...
Github user kwmonroe commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/344#discussion_r171912690
--- Diff: bigtop-deploy/puppet/hieradata/bigtop/cluster.yaml ---
@@ -192,6 +192,9 @@ hadoop_hive::common_config::hbase_zookeeper_quorum:
"%{hiera('hadoop_hbase::comm
hadoop_hive::common_config::kerberos_realm:
"%{hiera('kerberos::site::realm')}"
hadoop_hive::common_config::metastore_uris:
"thrift://%{hiera('bigtop::hadoop_head_node')}:9083"
+hadoop_hive::common_config::hive_zookeeper_quorum:
"%{hiera('hadoop_hive::common_config::hive_zookeeper_quorum')}"
--- End diff --
Doh! @jamesbeedy you ninja'd me :) Thx for removing `cluster.yaml`
updates.
---
[GitHub] bigtop pull request #344: BIGTOP-3007: add properties to hive-site.xml for z...
Github user kwmonroe commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/344#discussion_r171910993
--- Diff: bigtop-deploy/puppet/hieradata/bigtop/cluster.yaml ---
@@ -192,6 +192,9 @@ hadoop_hive::common_config::hbase_zookeeper_quorum:
"%{hiera('hadoop_hbase::comm
hadoop_hive::common_config::kerberos_realm:
"%{hiera('kerberos::site::realm')}"
hadoop_hive::common_config::metastore_uris:
"thrift://%{hiera('bigtop::hadoop_head_node')}:9083"
+hadoop_hive::common_config::hive_zookeeper_quorum:
"%{hiera('hadoop_hive::common_config::hive_zookeeper_quorum')}"
--- End diff --
Per the dev list
[discussion](https://mail-archives.apache.org/mod_mbox/bigtop-dev/201803.mbox/%3C85DF9170-CEC9-4462-B554-5CAF892B0570%40oflebbe.de%3E),
we don't need to alter cluster.yaml for these new properties.
---
[GitHub] bigtop pull request #344: BIGTOP-3007: add properties to hive-site.xml for z...
Github user kwmonroe commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/344#discussion_r171911797
--- Diff:
bigtop-packages/src/charm/hive/layer-hive/lib/charms/layer/bigtop_hive.py ---
@@ -69,35 +83,44 @@ def configure_hive(self):
config = hookenv.config()
hive_env = self.dist_config.path('hive_conf') / 'hive-env.sh'
utils.re_edit_in_place(hive_env, {
-r'.*export HADOOP_HEAPSIZE *=.*': 'export HADOOP_HEAPSIZE=%s'
% config['heap'],
--- End diff --
Looks like the rest of the changes to this file are lint fixes. While they
are much appreciated, in the future, please make them in a separate PR. It's a
bit harder to focus on the functional code changes with these formatting
changes.
---
[GitHub] bigtop pull request #344: BIGTOP-3007: add properties to hive-site.xml for z...
Github user kwmonroe commented on a diff in the pull request:
https://github.com/apache/bigtop/pull/344#discussion_r171912318
--- Diff: bigtop-packages/src/charm/hive/layer-hive/reactive/hive.py ---
@@ -78,9 +87,13 @@ def install_hive(hadoop):
else:
hbserver = None
+# Get zookeepers or None
+zks = KV.get('zookeepers', None)
--- End diff --
I'm curious why we're using `unitdata.kv` here. If you look a few lines
higher at the hbase block, we could use the same logic to get zks when
`is_state(zookeeper.ready)` is available.
---
[GitHub] bigtop pull request #344: BIGTOP-3007
GitHub user jamesbeedy opened a pull request: https://github.com/apache/bigtop/pull/344 BIGTOP-3007 In support of hiveserver2 table lock, hive config needs to expose the properties: * hive.support.concurrency * hive.zookeeper.quorum Added hive.zookeeper.quorum and hive.support.concurrency in support of hiveserver2 table lock. Add vars to init.pp. Add zookeeper interface and corresponding reactive bits. Modified install function in lib to facilitate zk_hosts. Update deployment_matrix. You can merge this pull request into a Git repository by running: $ git pull https://github.com/jamesbeedy/bigtop hive_config_add_zookeeper Alternatively you can review and apply these changes as the patch at: https://github.com/apache/bigtop/pull/344.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #344 commit 8e7350573ff8500fd7b68a3902421e4ee601a22b Author: jamesbeedy Date: 2018-02-27T17:19:06Z Add hive configuration properties in support of hiveserver2 In support of hiveserver2 table lock, hive config needs to expose the properties: * hive.support.concurrency * hive.zookeeper.quorum Added hive.zookeeper.quorum and hive.support.concurrency in support of hiveserver2 table lock. Add vars to init.pp. Add zookeeper interface and corresponding reactive bits. Modified install function in lib to facilitate zk_hosts. Update deployment_matrix. ---
