Re: Initdb-time block size specification

Hi,

On 2023-06-30 12:35:09 -0500, David Christensen wrote:
> As discussed somewhat at PgCon, enclosed is v1 of a patch to provide
> variable block sizes; basically instead of BLCKSZ being a compile-time
> constant, a single set of binaries can support all of the block sizes
> Pg can support, using the value stored in pg_control as the basis.
> (Possible future plans would be to make this something even more
> dynamic, such as configured per tablespace, but this is out of scope;
> this just sets up the infrastructure for this.)

I am extremely doubtful this is a good idea. For one it causes a lot of churn,
but more importantly it turns currently cheap code paths into more expensive
ones.

Changes like

> -#define BufHdrGetBlock(bufHdr) ((Block) (BufferBlocks + ((Size) (bufHdr)->buf_id) * BLCKSZ))
> +#define BufHdrGetBlock(bufHdr) ((Block) (BufferBlocks + ((Size) (bufHdr)->buf_id) * CLUSTER_BLOCK_SIZE))

Note that CLUSTER_BLOCK_SIZE, despite looking like a macro that's constant, is
actually variable.

I am fairly certain this is going to be causing substantial performance
regressions. I think we should reject this even if we don't immediately find
them, because it's almost guaranteed to cause some.

Besides this, I've not really heard any convincing justification for needing
this in the first place.

Greetings,

Andres Freund