On Fri, Mar 03, 2023 at 10:32:53AM -0800, Jacob Champion wrote: > On Sat, Feb 25, 2023 at 5:22 PM Justin Pryzby <pry...@telsasoft.com> wrote: > > This resolves cfbot warnings: windows and cppcheck. > > And refactors zstd routines. > > And updates docs. > > And includes some fixes for earlier patches that these patches conflicts > > with/depends on. > > This'll need a rebase (cfbot took a while to catch up).
Soon. > The patchset includes basebackup modifications, which are part of a > different CF entry; was that intended? Yes, it's intentional - if zstd:long mode were to be merged first, then this patch should include long mode from the start. Or, if pgdump+zstd were merged first, then long mode could be added to both places. > I tried this on a local, 3.5GB, mostly-text table (from the UK Price Thanks for looking. If your zstd library is compiled with thread support, could you also try with :workers=N ? I believe this is working correctly, but I'm going to ask for help verifying that... It'd be especially useful to test under windows, where pgdump/restore use threads instead of forking... If you have a windows environment but not set up for development, I think it's possible to get cirrusci to compile a patch for you and then retrieve the binaries provided as an "artifact" (credit/blame for this idea should be directed to Thomas Munro). > With this particular dataset, I don't see much improvement with > zstd:long. Yeah. I this could be because either 1) you already got very good comprssion without looking at more data; and/or 2) the neighboring data is already very similar, maybe equally or more similar, than the further data, from which there's nothing to gain. > (At nearly double the CPU time, I get a <1% improvement in > compression size.) I assume it's heavily data dependent, but from the > notes on --long [2] it seems like they expect you to play around with > the window size to further tailor it to your data. Does it make sense > to provide the long option without the windowLog parameter? I don't want to start exposing lots of fine-granined parameters at this point. In the immediate case, it looks like it may require more than just adding another parameter: Note: If windowLog is set to larger than 27, --long=windowLog or --memory=windowSize needs to be passed to the decompressor. -- Justin