sablierapp / sablier

Start your containers on demand, shut them down automatically when there's no activity. Docker, Docker Swarm Mode and Kubernetes compatible.
https://sablierapp.dev/
GNU Affero General Public License v3.0
1.36k stars 46 forks source link

Blocking strategy response slower than expected #282

Open Derkades opened 6 months ago

Derkades commented 6 months ago

Describe the bug I have configured sablier with nginx and whoami for testing. A response takes slightly over 5 seconds when the whoami container is not running. Even when starting the container manually, then issuing a request a few seconds later, it still takes 5 seconds. But the next request is instant again. I feel like there is some unnecessary delay somewhere.

Context

acouvreur commented 3 months ago

Hello @Derkades is this issue still happening ?

Derkades commented 3 months ago

Hi, yes it appears this is still an issue with the latest :beta

acouvreur commented 3 months ago

The first time sablier receives a request for a given container, it needs to check for its readiness.

So even if it's already running, the first request will be longer than the next ones.

Maybe I could pre-register all running containers, that could speed up things actually.

Would you be able to create a reproducible scenario with a docker compose file so I can experience this issue ?

Derkades commented 3 months ago

Yes, thank you for your interest in solving this issue! This is my compose file:

services:
  sablier:
    image: ghcr.io/acouvreur/sablier:beta
    volumes:
      - type: bind
        source: ./sablier.yaml
        target: /etc/sablier/sablier.yml
      - type: bind
        source: /var/run/docker.sock
        target: /var/run/docker.sock
    ports: ['127.0.0.1:10000:10000']

  whoami:
    image: containous/whoami:v1.5.0
    labels:
      - sablier.enable=true
      - sablier.group=my-group

  nginx:
    image: nginxinc/nginx-unprivileged
    volumes:
      - type: bind
        source: ./nginx.conf
        target: /etc/nginx/nginx.conf
      - type: bind
        source: ./sablier.js
        target: /etc/nginx/conf.d/sablier.js
    ports: ['127.0.0.1:8080:8080']

And the sablier config file:

provider:
  name: docker
server:
  port: 10000
  base-path: /

Nginx configuration:

    server {
        listen 8080;
        # The internal location to reach sablier API
        set $sablierUrl /sablier;
        # Shared variable for default session duration
        set $sablierSessionDuration 30s;

        # internal location for sablier middleware
        # here, the sablier API is a container named "sablier" inside the same network as nginx
        location /sablier/ {
            internal;
            proxy_method GET;
            proxy_pass http://sablier:10000/;
        }

        # A named location that can be used by the sablier middleware to redirect
        location @whoami {
            # Use variable in order to refresh DNS cache
            set $whoami_server whoami;
            proxy_pass http://$whoami_server:80;
            proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        }

        # The actual location to match your API
        # Will answer by a waiting page or redirect to your app if
        # it is already up
        location /aa {
            # set $sablierDynamicShowDetails true;
            # set $sablierDynamicRefreshFrequency 5s;
            set $sablierNginxInternalRedirect @whoami;
            set $sablierGroup my-group;
            # set $sablierNames my-group;
            # set $sablierDynamicName "Dynamic Whoami";
            # set $sablierDynamicTheme ghost;
            set $sablierBlockingTimeout 10s;
            js_content sablier.call;
        }
    }
acouvreur commented 3 months ago

Thanks @Derkades!

Actually, I have found the issue: https://github.com/acouvreur/sablier/blob/8bfb37a756e9098ca9fd81135c65ca714aa8d87d/app/sessions/sessions_manager.go#L228

This checks every five seconds the readiness of the app.

I can try to change some core behavior to be more event driven. For docker I know that I can watch events, so I might just do that at some point.

I'll track this ticket for performance improvements. Note that, as always, performance improvement for Sablier always comes with you properly defining an healthcheck, otherwise Sablier cannot know the difference between started and ready to receive requests.

Derkades commented 3 months ago

Hi, thanks for the update! Is the session request an expensive operation? It seems like it can be done much more often, say every 100 milliseconds. Before doing a more complex rewrite to an event-based system.

Thanks for the tip about defining a healthcheck, I'll keep that in mind when I start using it in production.

acouvreur commented 3 months ago

I'm planning on considerably enhancing communication with providers.

I'll use events instead of polling, meaning that we'll have real-time responses instead. I'll try to add that in the 1.8.0 release. So you should see nice improvements.

acouvreur commented 2 weeks ago

Polling was not added in 1.8.0 because providers send different events.

Therefore I cannot completely switch to polling. However I'm planning on doing the following: