dani-garcia / vaultwarden

Unofficial Bitwarden compatible server written in Rust, formerly known as bitwarden_rs
GNU Affero General Public License v3.0
34.71k stars 1.69k forks source link

Reduce healthcheck internal while keep performance #4678

Closed hellodword closed 1 week ago

hellodword commented 1 week ago

Currently, it takes 60 seconds for vaultwarden/server to become healthy:

version: "3"

services:
  vaultwarden:
    image: vaultwarden/server:1.30.5-alpine
    # image: vaultwarden/server:1.30.5
    restart: no
    environment:
      I_REALLY_WANT_VOLATILE_STORAGE: true

  nginx:
    image: nginx
    restart: no
    depends_on:
      vaultwarden:
        condition: service_healthy

By implementing a cache mechanism, we can reduce this interval.

BlackDex commented 1 week ago

This would make the container runtime check for every 3 seconds. That could be an issue for slower less resource systems.

Also, when the log_level is set to debug or trace, this will flood the logs and make it harder to troubleshoot.

If you are really bothered by this i would suggest to use at least docker v25 and a recent docker compose and use start_interval and start_period in your compose file.

Also i see you use I_REALLY_WANT_VOLATILE_STORAGE i must say that is not really save. Not sure if that is only done for this example. It could make all your data disappear when stopping and removing the container.

hellodword commented 1 week ago

This would make the container runtime check for every 3 seconds. That could be an issue for slower less resource systems.

What if we keep the --interval=60s --timeout=10s settings and modify the healthcheck.sh? This way, users who want a quicker healthy status can simply override the interval, even when using older versions of docker compose that do not support start_interval and start_period:

version: "3"

services:
  vaultwarden:
    image: vaultwarden/server:1.30.5-alpine
    restart: no
    environment:
      I_REALLY_WANT_VOLATILE_STORAGE: true
    healthcheck:
      test: ["CMD", "/healthcheck.sh"]
      interval: 3s
      timeout: 2s

  nginx:
    image: nginx
    restart: no
    depends_on:
      vaultwarden:
        condition: service_healthy

Not sure if that is only done for this example.

Thanks, yes, it's just for the example :)

jjlin commented 1 week ago

It doesn't seem like there's any real benefit to this. nginx doesn't depend on vaultwarden, so there's no need to add a dependency there. Both services will start up just fine without each other, and vaultwarden presumably will not be reachable until both services are up anyway, so it's best to let them start up in parallel so they each reach an operational state as soon as possible.

hellodword commented 1 week ago

nginx doesn't depend on vaultwarden, so there's no need to add a dependency there

Actually, Nginx is just an example here. There could be other services in the docker-compose file, and with service_healthy, users can ensure that these services are working properly during docker compose up --build -d.

❯ time docker compose up --build -d
[+] Running 3/3
 ✔ Network vaultwarden-test_default          Created             0.1s 
 ✔ Container vaultwarden-test-vaultwarden-1  Healthy          60.7s 
 ✔ Container vaultwarden-test-nginx-1        Started             60.9s 

real    1m1.035s
user    0m0.255s
sys 0m0.186s
jjlin commented 1 week ago

I think it's a rather niche use case to have other services that actually will not start up properly without vaultwarden being ready, and I would rather there not be a weird caching mechanism just to accommodate that. Anyway, vaultwarden shouldn't take more than 2-3 seconds to start up.

BlackDex commented 1 week ago

I would actually not add those dependencies as @jjlin mentioned. And i agree with @jjlin regarding the healtcheck script customization.

If you really want it to be healthier faster, i suggest to use the options i mentioned in my previous post.

Going to close this. Still thanks for wanting to contribute :), and please do not hesitate to do so in the future.