Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15966 )

Change subject: IMPALA-9793: Impala quickstart cluster with docker-compose
......................................................................


Patch Set 9:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/15966/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15966/9//COMMIT_MSG@26
PS9, Line 26: Instructions for running the quickstart cluster are in
            : docker/quickstart.yml.
> One thing I saw over in Kudu is that they have the docker compose instructi
I moved the stuff from the yml to the README, I think that makes sense.


http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart.yml
File docker/quickstart.yml:

http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart.yml@20
PS9, Line 20: # All filesystem data is stored in Docker volumes. The default 
storage location for tables
            : # is in the impala-quickstart-warehouse volume, i.e. if you 
create a table in Impala, it
            : # will be stored in that volume by default.
> This could be a follow-on change, but we may want to have some documentatio
I added a section to the README describing how to mount the volume in a 
container and copy data from the host.


http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart.yml@51
PS9, Line 51: # To load data in background into Parquet and Kudu formats:
            : #
            : #  docker-compose -f docker/quickstart.yml -f 
docker/quickstart-kudu-minimal.yml \
            : #                     -f docker/quickstart-load-data.yml up -d
> Can the dataload part run as a separate docker-compose command? i.e.
They need to be run in the same command so that docker-compose can resolve the 
dependencies between the files.


http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_client/Dockerfile
File docker/quickstart_client/Dockerfile:

http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_client/Dockerfile@48
PS9, Line 48: RUN groupadd -r impala && useradd --no-log-init -r -g impala 
impala && \
Needed to update the uid/gid to match IMPALA-10373


http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_client/Dockerfile@52
PS9, Line 52: # Copy the Hive install.
> Nit: Not really Hive related. Maybe change this to "Copy the client entrypo
Done


http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_client/data-load-entrypoint.sh
File docker/quickstart_client/data-load-entrypoint.sh:

http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_client/data-load-entrypoint.sh@32
PS9, Line 32: LOading
> Nit: capitalization typo
Done


http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_client/data-load-entrypoint.sh@35
PS9, Line 35:   
TPCDS_TARBALL=tpc-ds-${TPCDS_VERSION}-gcc-4.9.2-ec2-package-ubuntu-18-04.tar.gz
> Nit: The client base image is using Ubuntu 16 by default, and we are downlo
Done - switched the base image to ubuntu 18


http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_conf/hive-site.xml
File docker/quickstart_conf/hive-site.xml:

http://gerrit.cloudera.org:8080/#/c/15966/9/docker/quickstart_conf/hive-site.xml@28
PS9, Line 28:      <property>
            :         <!-- Required for automatic metadata sync. -->
            :         
<name>hive.metastore.notifications.add.thrift.objects</name>
            :         <value>true</value>
            :       </property>
            :       <property>
            :         <!-- Required for automatic metadata sync. -->
            :         <name>hive.metastore.alter.notifications.basic</name>
            :         <value>false</value>
            :       </property>
            :       <property>
            :         <!-- User impala is not authorized to consume 
notifications by default, disable
            :              authentication to work around this. -->
            :          
<name>hive.metastore.event.db.notification.api.auth</name>
            :         <value>false</value>
            :       </property>
> When I'm looking at https://github.com/apache/impala/blob/master/fe/src/tes
Yeah it should be generally similar to what we use for the tests. I used that 
as a reference but removed some of the bits that were obviously irrelevant to 
prune it down. Ideally this config would only override default hive settings 
where we actually need them to get it working.

I removed a couple of these settings that were redundant now.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc0b862af40a368381ada7ec2a355fe4b0aa778c
Gerrit-Change-Number: 15966
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Grant Henke <granthe...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Jan 2021 21:46:50 +0000
Gerrit-HasComments: Yes

Reply via email to