Skip to content

Set the rlimit for NOFILE to 1024 to reduce the subprocess cleanup #3101

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Sep 25, 2019
Merged

Set the rlimit for NOFILE to 1024 to reduce the subprocess cleanup #3101

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
96b7dd4
master: Set our own rlimit for number of fds
cdecker Sep 25, 2019
6e134e3
docker: Use system libraries, don't bring your own
cdecker Sep 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 26 additions & 24 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,29 +48,26 @@ RUN mkdir /opt/litecoin && cd /opt/litecoin \
FROM debian:stretch-slim as builder

ENV LIGHTNINGD_VERSION=master
RUN apt-get update && apt-get install -y --no-install-recommends ca-certificates autoconf automake build-essential git libtool python python3 python3-mako wget gnupg dirmngr git gettext

RUN wget -q https://zlib.net/zlib-1.2.11.tar.gz \
&& tar xvf zlib-1.2.11.tar.gz \
&& cd zlib-1.2.11 \
&& ./configure \
&& make \
&& make install && cd .. && rm zlib-1.2.11.tar.gz && rm -rf zlib-1.2.11

RUN apt-get install -y --no-install-recommends unzip tclsh \
&& wget -q https://www.sqlite.org/2018/sqlite-src-3260000.zip \
&& unzip sqlite-src-3260000.zip \
&& cd sqlite-src-3260000 \
&& ./configure --enable-static --disable-readline --disable-threadsafe --disable-load-extension \
&& make \
&& make install && cd .. && rm sqlite-src-3260000.zip && rm -rf sqlite-src-3260000

RUN wget -q https://gmplib.org/download/gmp/gmp-6.1.2.tar.xz \
&& tar xvf gmp-6.1.2.tar.xz \
&& cd gmp-6.1.2 \
&& ./configure --disable-assembly \
&& make \
&& make install && cd .. && rm gmp-6.1.2.tar.xz && rm -rf gmp-6.1.2
RUN apt-get update && apt-get install -y --no-install-recommends \
ca-certificates \
autoconf \
automake \
build-essential \
git \
libtool \
python \
python3 \
python3-mako \
wget \
gnupg \
dirmngr \
git \
gettext \
unzip \
tclsh \
libsqlite3-dev \
libgmp-dev \
zlib1g-dev

WORKDIR /opt/lightningd
COPY . /tmp/lightning
Expand All @@ -82,7 +79,12 @@ RUN ./configure --prefix=/tmp/lightning_install --enable-static && make -j3 DEVE

FROM debian:stretch-slim as final
COPY --from=downloader /opt/tini /usr/bin/tini
RUN apt-get update && apt-get install -y --no-install-recommends socat inotify-tools \
RUN apt-get update && apt-get install -y --no-install-recommends \
socat \
inotify-tools \
libgmp10 \
libsqlite3-0 \
zlib1g \
&& rm -rf /var/lib/apt/lists/*

ENV LIGHTNINGD_DATA=/root/.lightning
Expand Down
6 changes: 6 additions & 0 deletions lightningd/lightningd.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
#include <lightningd/options.h>
#include <onchaind/onchain_wire.h>
#include <signal.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>
Expand Down Expand Up @@ -629,6 +630,11 @@ int main(int argc, char *argv[])
struct timers *timers;
const char *stop_response;
struct htlc_in_map *unprocessed_htlcs;
struct rlimit nofile = {1024, 1024};

/*~ Make sure that we limit ourselves to something reasonable. Modesty
* is a virtue. */
setrlimit(RLIMIT_NOFILE, &nofile);
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be moderately useful to have this as a configurable option? Or do we need to do this setrlimit ASAP before any command-line processing?

Copy link
Member Author

Choose a reason for hiding this comment

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

No we can make it configurable, but for now this limits us to ~1000 peers, which should be an ok guess for now.

For non-root users we're unlikely to even have a higher limit since it requires system configuration changes. If this becomes an issue we can definitely introduce the option.


/*~ What happens in strange locales should stay there. */
setup_locale();
Expand Down