Re: refactoring basebackup.c (zstd workers)

On Mon, Mar 14, 2022 at 01:02:20PM -0400, Robert Haas wrote:
> On Mon, Mar 14, 2022 at 12:35 PM Justin Pryzby <pryzby(at)telsasoft(dot)com> wrote:
> > I suggest to use a syntax that's more general than that, maybe something like
> >
> > :[level=]N,parallel=N,flag,flag,...
> >
> > For example, someone may want to use zstd "long" mode or (when it's released)
> > rsyncable mode, or specify fine-grained compression parameters (strategy,
> > windowLog, hashLog, etc).
>
> That's an interesting idea. I wonder what the replication protocol
> ought to look like in that case. Should we have a COMPRESSION_DETAIL
> argument that is just a string, and let the server parse it out? Or
> separate protocol-level options? It does feel reasonable to have both
> COMPRESSION_LEVEL and COMPRESSION_WORKERS as first-class options, but
> I don't know that we want COMPRESSION_HASHLOG true as part of our
> first-class grammar.

I was only referring to the user-facing grammar.

Internally, I was thinking they'd all be handled as first-class options, with
separate struct fields and separate replication protocol options. If an option
isn't known, it'd be rejected on the client side, rather than causing an error
on the server.

Maybe there'd be an option parser for this in common/ (I think that might
require having new data structure there too, maybe one for each compression
method, or maybe a union{} to handles them all). Most of the ~100 lines to
support wal_compression='zstd:N' are to parse out the N.

--
Justin