Performance concerns

[mickaelperrin]

As spotted in #1049, the way certificates are created may have performance concerns when the number of certificates to handle is very important.

The fix in #1049 handles certificates in a reverse order, but sadly this doesn't have any impact because the reload of nginx still wait that all certificates are validated even if new certificates are emitted / renewed.

Ideally:

• the whole validation loop should be triggered only after the DOCKER_GEN_WAIT.
• when a new event is triggered from a container re/start, only related certificates should be handled.

The main challenge with this idea is to handle multiple simultaneous events, maybe some kind of queue management could help.

[buchdag]

The fix in https://github.com/nginx-proxy/acme-companion/issues/1049 handles certificates in a reverse order, but sadly this doesn't have any impact because the reload of nginx still wait that all certificates are validated even if new certificates are emitted / renewed.

To be more specific it handles certificate by reverse container creation time.

Maybe the ability to reload nginx as soon as a certificate is created or renewed rather that at the end of the loop could help ?

the whole validation loop should be triggered only after the DOCKER_GEN_WAIT.

I'm not sure I understand this, could you elaborate ?

when a new event is triggered from a container re/start, only related certificates should be handled.

That would be a major rework of the container logic and inner working. While the idea is interesting, I'll be honest: that's clearly not planned at the moment or on foreseeable future, and neither is implementing queue management.

[mickaelperrin]

Maybe the ability to reload nginx as soon as a certificate is created or renewed rather that at the end of the loop could help ?

Yes definitely, I guess this the easiest fix to drastically improve the performance feeling.

the whole validation loop should be triggered only after the DOCKER_GEN_WAIT.

I'm not sure I understand this, could you elaborate ?

This is related to the idea which states that ideally only certificates that are related to docker events should be checked for renewal. This ensures that certificates related to containers that didn't trigger an event continues to be refreshed regularly.

[buchdag]

This is related to the idea which states that ideally only certificates that are related to docker events should be checked for renewal.

I'll check docker-gen code to be sure but I don't think that's possible, from memory there is no notion of container(s) being related to a given docker event that docker-gen currently have access to.

Even if this was available, I don't think this could be used by acme-companion given the way it work. Each time docker-gen re-generate /app/letsencrypt_data, the generated file is the entirety of the information acme-companion has at its disposal. Any information that's not there just does not exist, and there is no previous state to compare to and infer changes from.

You can view it kinda like using Terraform: on each Docker event docker-gen render a declarative configuration file describing the wanted certificate configuration at this point in time, and the acme-companion process will perform the required operations to reach this state.

[buchdag]

@mickaelperrin could you try out nginxproxy/acme-companion:1147 ?

It should reload nginx as soon as a certificate is created or renewed by default, as discussed above:

diff --git a/app/letsencrypt_service b/app/letsencrypt_service
index 4f58925..0ec9e60 100755
--- a/app/letsencrypt_service
+++ b/app/letsencrypt_service
@@ -479,6 +479,9 @@ function update_cert {
         fi
     done

+    if ! parse_true "${RELOAD_NGINX_ONLY_ONCE:-false}" && parse_true $should_reload_nginx; then
+        reload_nginx
+    fi
 }

 function update_certs {

[buchdag]

Fix from #1166 tested and confirmed to work.