Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Dockerfile #392

Merged
merged 3 commits into from
Apr 13, 2021
Merged

Improve Dockerfile #392

merged 3 commits into from
Apr 13, 2021

Conversation

janhn
Copy link
Contributor

@janhn janhn commented Apr 8, 2021

  1. Reduces docker image size from ~860MB to ~470MB.

Running a bare chown (or any other form of file modification, e.g. chmod) recursively on a directory tree in a Dockerfile comes with a cost: docker makes a copy of the whole layer to apply the change. Solvable with --chown argument to COPY.

Changes from:

$ docker image ls | grep electrs                                                             
electrs-app [...] 865MB                                                          

$ docker history electrs-app                                                                 
IMAGE          CREATED         CREATED BY                                      SIZE      COMM
...
fae691841c1a   3 minutes ago   /bin/sh -c groupadd -r user     && adduser -…   393MB
1c4b0ea96c3c   3 minutes ago   /bin/sh -c #(nop) COPY dir:7956596a8fde73ca1…   393MB 

to:

$ docker image ls | grep electrs                                                 
electrs-app [...] 472MB                                                          

$ docker history electrs-app                                                                 
IMAGE          CREATED              CREATED BY                                      SIZE
...
7626a2efbb63   About a minute ago   /bin/sh -c #(nop) COPY --chown=user:userdir:…   393MB
bfbfc289f1a8   About a minute ago   /bin/sh -c groupadd -r user     && adduser -…   328kB    
  1. This commit is only a slight improvement in that docker build caches will not get invalidated by e.g. changing Dockerfile or LICENSE, as they shouldn't be, as they don't affect the build/compilation.

Possibly the same could be done for other directories, like doc, internal, but I'm not familiar enough with the code to be confident to make that change. Ideally, the COPY . . statement in the Dockerfile should be rewritten to only copy the necessary minimum to get the build done correctly.

  1. I assume that the only build artifact needed for the final image is the electrs executable, dropping these:
root@e84f0882e5fb:/build/target/release# du -sb *
114651560       build
278254888       deps
522     electrs.d
2       examples
2       incremental
504     libelectrs.d

This further reduces the docker image size to 88.8MB

I haven't done much testing on the functionality though. All I can say is that it starts:

$ docker run electrs-app -v -v
Config { log: StdErrLog { verbosity: Info, quiet: false, show_level: true, timestamp: Off, modules: [], writer: "stderr", color_choice: Never }, network_type: Bitcoin, db_path: "./db/mainnet", daemon_dir: "/home/user/.bitcoin", blocks_dir: "/home/user/.bitcoin/blocks", daemon_rpc_addr: V4(127.0.0.1:8332), electrum_rpc_addr: V4(127.0.0.1:50001), monitoring_addr: V4(127.0.0.1:4224), jsonrpc_import: false, index_batch_size: 10, bulk_index_threads: 16, tx_cache_size: 10485760, txid_limit: 100, server_banner: "Welcome to electrs 0.8.9 (Electrum Rust Server)!", blocktxids_cache_size: 10485760 }
WARN - failed to connect daemon at 127.0.0.1:8332: Connection refused (os error 111)
WARN - failed to connect daemon at 127.0.0.1:8332: Connection refused (os error 111)

@janhn janhn changed the title Deduplicate Dockerfile layers Improve Dockerfile Apr 8, 2021
Copy link
Contributor

@Kixunil Kixunil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, if you've seen my brainfart, sorry about that.

@romanz romanz merged commit c6a6b12 into romanz:master Apr 13, 2021
@romanz
Copy link
Owner

romanz commented Apr 13, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants