Hi,
> This scheme is reused later for the XLogRecord xl_tot_len field over
> at [0], and FWIW is thus being reused. Sure, it's tailored to this WAL
> use case, but IMO we're getting good value from it. We don't use
> protobuf or JSON for WAL, we use our own serialization format. Having
> some specialized encoding/decoding in that format for certain fields
> is IMO quite acceptable.
>
> > Yes - I proposed that and wrote an implementation of reasonably efficient
> > varint encoding. Here's my prototype:
> > https://postgr.es/m/20221004234952.anrguppx5owewb6n%40awork3.anarazel.de
>
> As I mentioned on that thread, that prototype has a significant
> probability of doing nothing to improve WAL size, or even increasing
> the WAL size for installations which consume a lot of OIDs.
>
> > I think it's a bad tradeoff to write lots of custom varint encodings, just to
> > eek out a bit more space savings.
>
> This is only a single "custom" varint encoding though, if you can even
> call it that. It makes a field's size depend on flags set in another
> byte, which is not that much different from the existing use of
> XLR_BLOCK_ID_DATA_[LONG, SHORT].
>
> > The increase in code complexity IMO makes it a bad tradeoff.
>
> Pardon me for asking, but what would you consider to be a good
> tradeoff then? I think the code relating to the WAL storage format is
> about as simple as you can get it within the feature set it provides
> and the size of the resulting records. While I think there is still
> much to gain w.r.t. WAL record size, I don't think we can get much of
> those improvements without adding at least some amount of complexity,
> something I think to be true for most components in PostgreSQL.
>
> So, except for redesigning significant parts of the public WAL APIs,
> are we just going to ignore any potential improvements because they
> "increase code complexity"?
Here are my two cents.
I definitely see the value in having reusable varint encoding. This
would probably be a good option for extendable TOAST pointers,
compression dictionaries, and perhaps other patches.
In this particular case however Matthias has good arguments that this
is not the right tool for this particular task, IMO. We don't use
rbtrees for everything that needs a map from x to y. Hash tables have
other compromises. Sometimes one container is a better fit, sometimes
the other, sometimes none and we implement a fine tuned container for
the given case. Same here.
--
Best regards,
Aleksander Alekseev