You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2019/05/31 19:30:30 UTC

[Impala-ASF-CR] IMPALA-8425: part 1: reduce size of binaries in container

Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13487


Change subject: IMPALA-8425: part 1: reduce size of binaries in container
......................................................................

IMPALA-8425: part 1: reduce size of binaries in container

* Symlink impalad/catalog/statestored inside container.
  This doesn't seem to really save any space - there's
  some kind of deduplication going on.
* Don't include libfesupport.so, which shouldn't be needed.
* strip debug symbols from the binary.
* Only include the libkuduclient.so libraries for Kudu

This shaves ~1.1GB from the image size- 250MB as a result
of the impalad binary changes and the remainder from the
Kudu changs.

Change-Id: I95ff479bedd3b93e6569e72f03f42acd9dba8b14
---
M docker/daemon_entrypoint.sh
M docker/impala_base/Dockerfile
M docker/setup_build_context.py
3 files changed, 32 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/13487/3
-- 
To view, visit http://gerrit.cloudera.org:8080/13487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I95ff479bedd3b93e6569e72f03f42acd9dba8b14
Gerrit-Change-Number: 13487
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8425: part 1: reduce size of binaries in container

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.

Hello Joe McDonnell, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/13487

to look at the new patch set (#4).

Change subject: IMPALA-8425: part 1: reduce size of binaries in container
......................................................................

IMPALA-8425: part 1: reduce size of binaries in container

* Symlink impalad/catalog/statestored inside container.
  This doesn't seem to really save any space - there's
  some kind of deduplication going on.
* Don't include libfesupport.so, which shouldn't be needed.
* strip debug symbols from the binary.
* Only include the libkuduclient.so libraries for Kudu

This shaves ~1.1GB from the image size- 250MB as a result
of the impalad binary changes and the remainder from the
Kudu changs.

Change-Id: I95ff479bedd3b93e6569e72f03f42acd9dba8b14
---
M docker/daemon_entrypoint.sh
M docker/impala_base/Dockerfile
M docker/setup_build_context.py
3 files changed, 34 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/87/13487/4
-- 
To view, visit http://gerrit.cloudera.org:8080/13487
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I95ff479bedd3b93e6569e72f03f42acd9dba8b14
Gerrit-Change-Number: 13487
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-8425: part 1: reduce size of binaries in container

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.

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

Change subject: IMPALA-8425: part 1: reduce size of binaries in container
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13487/4/docker/impala_base/Dockerfile
File docker/impala_base/Dockerfile:

http://gerrit.cloudera.org:8080/#/c/13487/4/docker/impala_base/Dockerfile@30
PS4, Line 30: # Copy build artifacts required for the daemon processes.
            : # Need to have multiple copy commands to preserve directory structure.
            : COPY lib /opt/impala/lib
            : COPY www /opt/impala/www
            : COPY bin /opt/impala/bin
            : # Symlink here instead of in setup_build_context to avoid duplicate binaries.
            : RUN cd /opt/impala/bin && ln -s impalad statestored && ln -s impalad catalogd && \
            : # Create conf directory for later config injection.
            :     mkdir /opt/impala/conf && \
            : # Create logs directory to collect container logs.
            :     mkdir /opt/impala/logs
> I was thinking about the block of COPYs and the RUN that makes some directo
The issue was that COPY runs as root, so the destination files end up being owned by root. And if we do that after switching to impala, we can't modify the files.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95ff479bedd3b93e6569e72f03f42acd9dba8b14
Gerrit-Change-Number: 13487
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Jun 2019 18:08:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8425: part 1: reduce size of binaries in container

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.

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

Change subject: IMPALA-8425: part 1: reduce size of binaries in container
......................................................................


Patch Set 4:

For our test runs we would still have the unstripped binary available (I guess I hadn't confirmed that worked correctly). I don't know about the more general question of whether we want to optionally provide containers with the symbols built-in. It does seem like a lot of overhead to the size of the containers, and you would still have to extract the binary from the container to use it with GDB and a core file. It's nice that that would be an option though.

We still should have enough symbols to get backtraces as-is.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I95ff479bedd3b93e6569e72f03f42acd9dba8b14
Gerrit-Change-Number: 13487
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Jun 2019 00:45:16 +0000
Gerrit-HasComments: No