neondatabase / neon

Neon: Serverless Postgres. We separated storage and compute to offer autoscaling, code-like database branching, and scale to zero.
https://neon.tech
Apache License 2.0
14.78k stars 430 forks source link

Reduce layer map locking on the read path #6833

Closed VladLazar closed 6 months ago

VladLazar commented 8 months ago

Currently, the read path (Timeline::get and Timeline::get_vectored when it merges) hold the layer manager read lock while accumulating the reconstruct data (includes on-demand downloads and disk IO). The non-vectored read path grabs the lock here.

Locking for this long is not required. Instead, the read path could hold on to a storage_layer::Layer (introduced in https://github.com/neondatabase/neon/pull/4938) and only hold the lock while querying the layer map. The fix for the vectored read path will be similar, but slightly more involved. The search fringe must be updated to hold storage_layer::Layer instead of layer descriptions and the in-memory handles will need some thought as well.

koivunej commented 7 months ago

Just realized while looking at some logs (slack thread), that compaction now needs to wait for on-demand downloads. This is a regression introduced with the struct Layer work. Before that, we released the read lock for layer downloading and switching.

Since no one has noticed this in quite some time, I don't think this is critical, but we should get it eventually fixed, so a slight boost to this issue.

What needs to be done:

VladLazar commented 6 months ago

Update:

VladLazar commented 6 months ago

Both PRs merged. Releasing on the week of 2024-04-08