Re: Introduce contributor badges

From: Christoph Berg <myon(at)debian(dot)org>
To: Dave Page <dpage(at)pgadmin(dot)org>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>, pgsql-www(at)lists(dot)postgresql(dot)org, Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, PostgreSQL Contributors <contributors(at)lists(dot)postgresql(dot)org>
Subject: Re: Introduce contributor badges
Date: 2026-10-16 08:59:52
Message-ID: atHniPzZPas6faP6@msg.df7cb.de
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-www

Re: Dave Page
> I haven't had time to review it properly yet, but I just had Claude Opus
> review it, and it seems to think there are a few issues that need fixing. I
> told it to pay particular attention to performance:

Thanks!

> ## Varnish cache invalidation

Fwiw some of these gaps were intentional because we deemed them either
minor, or out-of-scope-ish because they need fixes in the "items"
management infra in pgweb and aren't specific to badges. We should
have documented that, sorry. Anyway, this is probably the time to fix
all of them, thanks for pointing them out.

> 4. **The image URL never changes when the image does.**
> `/community/badge/<slug>/image/` has no file extension and no version in
> it. The docs say Varnish applies a longer policy to images "because they
> are considered never to change". Varnish itself is purged, but if the
> Varnish configuration (VCL) also sends long browser cache headers for
> images, browsers will keep showing the old image.

I think purging Varnish is enough here. Badge image changes aren't
critical, if a user sees the image somewhat longer, that doesn't cause
trouble.

> ## Database and app server
>
> 5. **The `has_image` annotation reads every image out of TOAST**,
> PostgreSQL's out-of-line storage for large values
> (`BadgeManager.with_has_image`). The comment says the value is "never
> transferred", which is true for the network, but `imagedata > ''` compares
> the full bytea, so PostgreSQL has to fetch and reassemble the whole value.
> A test of 200 rows of about 96 KB each on the local database showed the
> comparison reading about 3,000 buffers in 31 ms, whilst `octet_length(img)
> > 0` read 2 buffers in 0.04 ms, because it only reads the TOAST header.
> Switching to `Length('imagedata') > 0` is a one-line fix. It runs on every
> cache miss of the people page (up to 200 badges) and on every account
> profile page load, which is never cached.

Ack.

> 6. **Oversized images are decoded in full (a "decompression bomb").**
> `ImageBinaryField.to_python` decodes the entire image with Pillow before
> rescaling. A PNG under 1 MB can still be about 170 megapixels before Pillow
> refuses it, which is several hundred MB of RAM plus a bicubic resize on the
> app server. `ModelForm` full-clean also re-parses the stored image on every
> badge edit, even when the image hasn't changed. Only managers of an
> approved organisation can upload, so the risk is moderate, but checking
> `img.size` before decoding would be cheap.

What max input size would we accept? 2000x2000 (4 megapixels) should
be pretty much enough for everyone?

Christoph

In response to

Responses

Browse pgsql-www by date

  From Date Subject
Next Message Christoph Berg 2026-10-16 09:09:24 Re: Introduce contributor badges
Previous Message Álvaro Herrera 2026-10-16 08:10:19 Re: Wiki editor request