On Thu, Jan 26, 2023 at 11:24:47AM +, gkokola...@pm.me wrote:
> I gave this a little bit of thought. I think that ReadHead should not
> emit a warning, or at least not this warning as it is slightly misleading.
> It implies that it will automatically turn off data restoration, which is
>
On Thu, Jan 26, 2023 at 02:49:27PM +0900, Michael Paquier wrote:
> On Wed, Jan 25, 2023 at 12:00:20PM -0600, Justin Pryzby wrote:
> > While looking at this, I realized that commit 5e73a6048 introduced a
> > regression:
> >
> > @@ -3740,19 +3762,24 @@ ReadHead(ArchiveHandle *AH)
> >
> > -
On Wed, Jan 25, 2023 at 12:00:20PM -0600, Justin Pryzby wrote:
> While looking at this, I realized that commit 5e73a6048 introduced a
> regression:
>
> @@ -3740,19 +3762,24 @@ ReadHead(ArchiveHandle *AH)
>
> - if (AH->compression != 0)
> - pg_log_warning("archive is
On Wed, Jan 25, 2023 at 07:57:18PM +, gkokola...@pm.me wrote:
> Nice catch!
Let me see..
--
Michael
signature.asc
Description: PGP signature
--- Original Message ---
On Wednesday, January 25th, 2023 at 7:00 PM, Justin Pryzby
wrote:
>
>
> On Wed, Jan 25, 2023 at 03:37:12PM +, gkokola...@pm.me wrote:
>
> While looking at this, I realized that commit 5e73a6048 introduced a
> regression:
>
> @@ -3740,19 +3762,24
--- Original Message ---
On Wednesday, January 25th, 2023 at 6:28 PM, Tomas Vondra
wrote:
>
>
>
> On 1/25/23 16:37, gkokola...@pm.me wrote:
>
> > --- Original Message ---
> > On Wednesday, January 25th, 2023 at 2:42 AM, Justin Pryzby
> > pry...@telsasoft.com wrote:
>
On Wed, Jan 25, 2023 at 03:37:12PM +, gkokola...@pm.me wrote:
> Of course, one can throw the error before entering the loop, yet I think
> that it does not help the readability of the code. IMHO it is easier to
> follow if the error is thrown once during that check.
> If anything, I can
On 1/25/23 16:37, gkokola...@pm.me wrote:
>
>
>
>
>
> --- Original Message ---
> On Wednesday, January 25th, 2023 at 2:42 AM, Justin Pryzby
> wrote:
>
>
>>
>>
>> On Tue, Jan 24, 2023 at 03:56:20PM +, gkokola...@pm.me wrote:
>>
>>> On Monday, January 23rd, 2023 at 7:00 PM,
On Tue, Jan 24, 2023 at 03:56:20PM +, gkokola...@pm.me wrote:
> On Monday, January 23rd, 2023 at 7:00 PM, Justin Pryzby
> wrote:
> > On Mon, Jan 23, 2023 at 05:31:55PM +, gkokola...@pm.me wrote:
> >
> > > Please find attached v23 which reintroduces the split.
> > >
> > > 0001 is
On Mon, Jan 23, 2023 at 05:31:55PM +, gkokola...@pm.me wrote:
> Please find attached v23 which reintroduces the split.
>
> 0001 is reworked to have a reduced footprint than before. Also in an attempt
> to facilitate the readability, 0002 splits the API's and the uncompressed
> implementation
--- Original Message ---
On Friday, January 20th, 2023 at 12:34 AM, Tomas Vondra
wrote:
>
>
> On 1/19/23 18:55, Tomas Vondra wrote:
>
> > Hi,
> >
> > On 1/19/23 17:42, gkokola...@pm.me wrote:
> >
> > > ...
> > >
> > > Agreed. It was initially submitted as one patch. Then it
On 1/19/23 18:55, Tomas Vondra wrote:
> Hi,
>
> On 1/19/23 17:42, gkokola...@pm.me wrote:
>>
>> ...
>>
>> Agreed. It was initially submitted as one patch. Then it was requested to be
>> split up in two parts, one to expand the use of the existing API and one to
>> replace with the new interface.
Hi,
On 1/19/23 17:42, gkokola...@pm.me wrote:
>
> --- Original Message ---
> On Thursday, January 19th, 2023 at 4:45 PM, Tomas Vondra
> wrote:
>>
>> On 1/18/23 20:05, gkokola...@pm.me wrote:
>>
>>> --- Original Message ---
>>> On Wednesday, January 18th, 2023 at 3:00 PM, Tomas
--- Original Message ---
On Thursday, January 19th, 2023 at 4:45 PM, Tomas Vondra
wrote:
>
>
> On 1/18/23 20:05, gkokola...@pm.me wrote:
>
> > --- Original Message ---
> > On Wednesday, January 18th, 2023 at 3:00 PM, Tomas Vondra
> > tomas.von...@enterprisedb.com
On 1/18/23 20:05, gkokola...@pm.me wrote:
>
>
>
>
>
> --- Original Message ---
> On Wednesday, January 18th, 2023 at 3:00 PM, Tomas Vondra
> wrote:
>
>
>>
>>
>> Hi,
>>
>> On 1/16/23 16:14, gkokola...@pm.me wrote:
>>
>>> Hi,
>>>
>>> I admit I am completely at lost as to what is
Hi,
On 1/16/23 16:14, gkokola...@pm.me wrote:
> Hi,
>
> I admit I am completely at lost as to what is expected from me anymore.
>
:-(
I understand it's frustrating not to know why a patch is not moving
forward. Particularly when is seems fairly straightforward ...
Let me briefly explain my
On Sun, Jan 15, 2023 at 07:56:25PM -0600, Justin Pryzby wrote:
> On Mon, Jan 16, 2023 at 10:28:50AM +0900, Michael Paquier wrote:
>> The functions changed by 0001 are cfopen[_write](),
>> AllocateCompressor() and ReadDataFromArchive(). Why is it a good idea
>> to change these interfaces which
On Mon, Jan 16, 2023 at 02:27:56AM +, gkokola...@pm.me wrote:
> Oh, I didn’t realize you took over Justin? Why? After almost a year of work?
>
> This is rather disheartening.
I believe you've misunderstood my intent here. I sent rebased versions
of your patches with fixup commits
Oh, I didn’t realize you took over Justin? Why? After almost a year of work?
This is rather disheartening.
On Mon, Jan 16, 2023 at 02:56, Justin Pryzby wrote:
> On Mon, Jan 16, 2023 at 10:28:50AM +0900, Michael Paquier wrote:
>> On Sat, Jan 14, 2023 at 03:43:09PM -0600, Justin Pryzby wrote:
>>
On Mon, Jan 16, 2023 at 10:28:50AM +0900, Michael Paquier wrote:
> On Sat, Jan 14, 2023 at 03:43:09PM -0600, Justin Pryzby wrote:
> > On Sun, Jan 08, 2023 at 01:45:25PM -0600, Justin Pryzby wrote:
> >> pg_compress_specification is being passed by value, but I think it
> >> should be passed as a
On Sat, Jan 14, 2023 at 03:43:09PM -0600, Justin Pryzby wrote:
> On Sun, Jan 08, 2023 at 01:45:25PM -0600, Justin Pryzby wrote:
>> pg_compress_specification is being passed by value, but I think it
>> should be passed as a pointer, as is done everywhere else.
>
> ISTM that was an issue with
On Sun, Jan 08, 2023 at 01:45:25PM -0600, Justin Pryzby wrote:
> On Thu, Dec 22, 2022 at 11:08:59AM -0600, Justin Pryzby wrote:
> > There's a couple of lz4 bits which shouldn't be present in 002: file
> > extension and comments.
> BTW I noticed that cfdopen() was accidentally committed to
On Thu, Dec 22, 2022 at 11:08:59AM -0600, Justin Pryzby wrote:
> There's a couple of lz4 bits which shouldn't be present in 002: file
> extension and comments.
There were "LZ4" comments and file extension stuff in the preparatory
commit. But now it seems like you *removed* them in the LZ4 commit
There's a couple of lz4 bits which shouldn't be present in 002: file
extension and comments.
On Tue, Dec 20, 2022 at 11:19:15AM +, gkokola...@pm.me wrote:
> --- Original Message ---
> On Monday, December 19th, 2022 at 6:27 PM, Justin Pryzby
> wrote:
> > On Mon, Dec 19, 2022 at 05:03:21PM +, gkokola...@pm.me wrote:
> >
> > > > > 001 still doesn't compile on freebsd, and
--- Original Message ---
On Monday, December 19th, 2022 at 6:27 PM, Justin Pryzby
wrote:
>
>
> On Mon, Dec 19, 2022 at 05:03:21PM +, gkokola...@pm.me wrote:
>
> > > > 001 still doesn't compile on freebsd, and 002 doesn't compile on
> > > > windows. Have you checked test
On Mon, Dec 19, 2022 at 01:06:00PM +0900, Michael Paquier wrote:
> On Sat, Dec 17, 2022 at 05:26:15PM -0600, Justin Pryzby wrote:
> > 001: still refers to "gzip", which is correct for -Fp and -Fd but not
> > for -Fc, for which it's more correct to say "zlib".
>
> Or should we begin by changing
On Mon, Dec 19, 2022 at 05:03:21PM +, gkokola...@pm.me wrote:
> > > 001 still doesn't compile on freebsd, and 002 doesn't compile on
> > > windows. Have you checked test results from cirrusci on your private
> > > github account ?
>
> There are still known gaps in 0002 and 0003, for example
On Sat, Dec 17, 2022 at 05:26:15PM -0600, Justin Pryzby wrote:
> 001: still refers to "gzip", which is correct for -Fp and -Fd but not
> for -Fc, for which it's more correct to say "zlib".
Or should we begin by changing all these existing "not built with zlib
support" error strings to the more
001: still refers to "gzip", which is correct for -Fp and -Fd but not
for -Fc, for which it's more correct to say "zlib". That affects the
name of the function, structures, comments, etc. I'm not sure if it's
an issue to re-use the basebackup compression routines here. Maybe we
should accept
On Mon, Dec 05, 2022 at 12:48:28PM +, gkokola...@pm.me wrote:
> I also took the liberty of applying the test pattern when it the dump
> is explicitly compressed.
Sticking with glob_patterns is fine by me.
> I was thinking a bit more about this. I think that we can use the list
> TOC option
On Sat, Dec 03, 2022 at 11:45:30AM +0900, Michael Paquier wrote:
> While this is correct in checking that the contents are compressed
> under --with-zlib, this also removes the coverage where we make sure
> that this command is able to complete under --without-zlib without
> compressing any of the
On Fri, Dec 02, 2022 at 04:15:10PM +, gkokola...@pm.me wrote:
> You are very correct. However one can glob after the fact. Please find
> 0001 of the attached v14 which attempts to implement it.
+ if ($pgdump_runs{$run}->{glob_pattern})
+ {
+ my $glob_pattern =
On Thu, Dec 01, 2022 at 02:58:35PM +, gkokola...@pm.me wrote:
> Nuking the warning from orbit and changing the behaviour around disabling
> the requested compression when the libraries are not present, should not
> mean that we need to change the behaviour of default values for different
>
--- Original Message ---
On Thursday, December 1st, 2022 at 3:05 AM, Michael Paquier
wrote:
>
>
> On Wed, Nov 30, 2022 at 05:11:44PM +, gkokola...@pm.me wrote:
>
> > Fair enough. The atteched v11 does that. 0001 introduces compression
> > specification and is using it
On Wed, Nov 30, 2022 at 05:11:44PM +, gkokola...@pm.me wrote:
> Fair enough. The atteched v11 does that. 0001 introduces compression
> specification and is using it throughout. 0002 paves the way to the
> new interface by homogenizing the use of cfp. 0003 introduces the new
> API and stores
On Tue, Nov 29, 2022 at 12:10:46PM +, gkokola...@pm.me wrote:
> Thank you. Please advice if is preferable to split 0002 in two parts.
> I think not but I will happily do so if you think otherwise.
This one makes me curious. What kind of split are you talking about?
If it makes the code
On Tue, Nov 29, 2022 at 03:19:17PM +0900, Michael Paquier wrote:
> Attached is an alternative that does not sacrifice the pluggability of
> the existing routines while allowing 0003~ to still use them (I don't
> really want to move around the checks on the supported build options
> now in
On Mon, Nov 28, 2022 at 04:32:43PM +, gkokola...@pm.me wrote:
> The focus of this version of this series is 0001 and 0002.
>
> Admittedly 0001 could be presented in a separate thread though given its size
> and
> proximity to the topic, I present it here.
I don't mind. This was a hole in
On Tue, Nov 22, 2022 at 10:00:47AM +, gkokola...@pm.me wrote:
> For the record I am currently working on it simply unsure if I should submit
> WIP patches and add noise to the list or wait until it is in a state that I
> feel that the comments have been addressed.
>
> A new version that I
On Tue, Nov 22, 2022 at 10:00:47AM +, gkokola...@pm.me wrote:
> A new version that I feel that is in a decent enough state for review should
> be ready within this week. I am happy to drop the patch if you think I should
> not work on it though.
If you can post a new version of the patch,
--- Original Message ---
On Monday, November 21st, 2022 at 12:13 AM, Michael Paquier
wrote:
>
>
> On Sun, Nov 20, 2022 at 11:26:11AM -0600, Justin Pryzby wrote:
>
> > I think this patch record should be closed for now. You can re-open the
> > existing patch record once a patch
On Sun, Nov 20, 2022 at 11:26:11AM -0600, Justin Pryzby wrote:
> I think this patch record should be closed for now. You can re-open the
> existing patch record once a patch is ready to be reviewed.
Indeed. As of things are, this is just a dead entry in the CF which
would be confusing. I have
On Fri, Aug 05, 2022 at 02:23:45PM +, Georgios Kokolatos wrote:
> Thank you for your work during commitfest.
>
> The patch is still in development. Given vacation status, expect the next
> patches to be ready for the November commitfest.
> For now it has moved to the September one. Further
On Wed, Nov 2, 2022 at 14:28, Justin Pryzby wrote:
> Checking if you'll be able to submit new patches soon ?
Thank you for checking up. Expect new versions within this commitfest cycle.
Checking if you'll be able to submit new patches soon ?
This entry has been waiting on author input for a while (our current
threshold is roughly two weeks), so I've marked it Returned with
Feedback.
Once you think the patchset is ready for review again, you (or any
interested party) can resurrect the patch entry by visiting
On Tue, Jul 05, 2022 at 01:22:47PM +, gkokola...@pm.me wrote:
> I have updated for "some" of the comments. This is not an unwillingness to
> incorporate those specific comments. Simply this patchset had started to
> divert
> heavily already based on comments from Mr. Paquier who had already
This is a review of 0001.
On Tue, Jul 05, 2022 at 01:22:47PM +, gkokola...@pm.me wrote:
> Simply this patchset had started to divert
> heavily already based on comments from Mr. Paquier who had already requested
> for
> the APIs to be refactored to use function pointers. This is happening in
--- Original Message ---
On Sunday, June 26th, 2022 at 5:55 PM, Justin Pryzby
wrote:
>
>
> Hi,
>
> Will you be able to send a rebased patch for the next CF ?
Thank you for taking an interest in the PR. The plan is indeed to sent
a new version.
> If you update for the review
Hi,
Will you be able to send a rebased patch for the next CF ?
If you update for the review comments I sent in March, I'll plan to do another
round of review.
On Sat, Mar 26, 2022 at 11:21:56AM -0500, Justin Pryzby wrote:
> LZ4F_HEADER_SIZE_MAX isn't defined in old LZ4.
>
> I ran into that on
--- Original Message ---
On Tuesday, April 5th, 2022 at 12:55 PM, Michael Paquier
wrote:
> On Tue, Apr 05, 2022 at 07:13:35AM +, gkokola...@pm.me wrote:
> No need to carry it forward anymore, I think ;)
Thank you for committing!
Cheers,
//Georgios
> --
> Michael
On Tue, Apr 05, 2022 at 07:13:35AM +, gkokola...@pm.me wrote:
> Thank you. I agree with the attached and I will carry it forward to the
> rest of the patchset.
No need to carry it forward anymore, I think ;)
--
Michael
signature.asc
Description: PGP signature
--- Original Message ---
On Tuesday, April 5th, 2022 at 3:34 AM, Michael Paquier
wrote:
> On Fri, Apr 01, 2022 at 03:06:40PM +, gkokola...@pm.me wrote:
> Splitting the program and its arguments makes sense.
Great.
> At the end I am finishing with the attached. I also saw an
On Fri, Apr 01, 2022 at 03:06:40PM +, gkokola...@pm.me wrote:
> I understand the itch. Indeed when LZ4 is added as compression method, this
> block changes slightly. I went with the minimum amount changed. Please find
> in 0001 of the attached this variable renamed as $gzip_program_exist. I
>
On Thursday, March 31st, 2022 at 4:34 AM, Michael Paquier
wrote:
> On Wed, Mar 30, 2022 at 03:32:55PM +, gkokola...@pm.me wrote:
> > On Wednesday, March 30th, 2022 at 7:54 AM, Michael Paquier
> > mich...@paquier.xyz wrote:
>
> Okay. 0002 looks fine as-is, and I don't mind the extra fatal()
On Wed, Mar 30, 2022 at 03:32:55PM +, gkokola...@pm.me wrote:
> On Wednesday, March 30th, 2022 at 7:54 AM, Michael Paquier
> wrote:
>> While moving on with 0002, I have noticed the following in
>>
>> _StartBlob():
>> if (AH->compression != 0)
>> sfx = ".gz";
>> else
>>
On Tue, Mar 29, 2022 at 09:14:03AM -0400, Robert Haas wrote:
> I don't know what you mean by this. I think the specification stuff
> could be reused in a lot of places. If you can ask for a base backup
> with zstd:level=3,long=1,fancystuff=yes or whatever we end up with,
> why not enable exactly
On Tue, Mar 29, 2022 at 09:46:27AM +, gkokola...@pm.me wrote:
> On Tuesday, March 29th, 2022 at 9:27 AM, Michael Paquier
> wrote:
>> On Sat, Mar 26, 2022 at 01:14:41AM -0500, Justin Pryzby wrote:
>> Wow. This stuff is old enough to vote (c3e18804), dead since its
>> introduction. There is
On Tue, Mar 29, 2022 at 1:03 AM Michael Paquier wrote:
> > Then, if people are willing to adopt the syntax that the
> > backup_compression.c/h stuff supports as a project standard (+1 from
> > me) we can go the other way and rename that stuff to be more generic,
> > taking backup out of the name.
On Sat, Mar 26, 2022 at 01:14:41AM -0500, Justin Pryzby wrote:
> See 0001 and the manpage.
>
> + 'pg_dump: compression is not supported by tar archive
> format');
>
> When I submitted a patch to support zstd, I spent awhile trying to make
> compression work with tar, but it's a
On Mon, Mar 28, 2022 at 08:36:15AM -0400, Robert Haas wrote:
> Well, I think the first thing we should do is get rid of enum
> WalCompressionMethod and use enum WalCompression instead. They've got
> the same elements and very similar names, but the WalCompressionMethod
> ones just have names like
On Sun, Mar 27, 2022 at 12:06 PM Justin Pryzby wrote:
> Maybe it should take an argument which specifies the default algorithm to use
> for input of a numeric "level". And reject such input if not specified, since
> wal_compression has never taken a "level", so it's not useful or desirable to
>
> On 27 Mar 2022, at 00:51, Justin Pryzby wrote:
>
> On Sun, Mar 27, 2022 at 12:37:27AM +0100, Daniel Gustafsson wrote:
>>> On 26 Mar 2022, at 17:21, Justin Pryzby wrote:
>>
>>> I suggested off-list to add an 0099 patch to change LZ4 to the default, to
>>> exercise it more on CI.
>>
>> No
On Sun, Mar 27, 2022 at 10:13:00AM -0400, Robert Haas wrote:
> On Sat, Mar 26, 2022 at 12:22 PM Justin Pryzby wrote:
> > 0002: I wonder if you're able to re-use any of the basebackup parsing stuff
> > from commit ffd53659c. You're passing both the compression method *and*
> > level.
> > I think
On Sat, Mar 26, 2022 at 12:22 PM Justin Pryzby wrote:
> 0002: I wonder if you're able to re-use any of the basebackup parsing stuff
> from commit ffd53659c. You're passing both the compression method *and*
> level.
> I think there should be a structure which includes both. In the future, that
On Sun, Mar 27, 2022 at 12:37:27AM +0100, Daniel Gustafsson wrote:
> > On 26 Mar 2022, at 17:21, Justin Pryzby wrote:
>
> > I suggested off-list to add an 0099 patch to change LZ4 to the default, to
> > exercise it more on CI.
>
> No need to change the defaults in autoconf for that. The CFBot
> On 26 Mar 2022, at 17:21, Justin Pryzby wrote:
> I suggested off-list to add an 0099 patch to change LZ4 to the default, to
> exercise it more on CI.
No need to change the defaults in autoconf for that. The CFBot uses the cirrus
file in the tree so changing what the job includes can be
On Sat, Mar 26, 2022 at 11:21:56AM -0500, Justin Pryzby wrote:
> You're passing both the compression method *and* level. I think there should
> be a structure which includes both. In the future, that can also handle
> additional options.
I'm not sure if there's anything worth saving, but I did
LZ4F_HEADER_SIZE_MAX isn't defined in old LZ4.
I ran into that on an ubuntu LTS, so I don't think it's so old that it
shouldn't be handled more gracefully. LZ4 should either have an explicit
version check, or else shouldn't depend on that feature (or should define a
safe fallback version if the
On Sat, Mar 26, 2022 at 02:57:50PM +0900, Michael Paquier wrote:
> I have a question about 0002, actually. What has led you to the
> conclusion that this code is dead and could be removed?
See 0001 and the manpage.
+ 'pg_dump: compression is not supported by tar archive format');
On Fri, Mar 25, 2022 at 11:43:17PM +, gkokola...@pm.me wrote:
> On Saturday, March 26th, 2022 at 12:13 AM, Rachel Heaton
> wrote:
>> Here is an updated patchset from Georgios, with minor assistance from myself.
>> The comments above should be addressed, but please let us know if
>
> A small
--- Original Message ---
On Saturday, March 26th, 2022 at 12:13 AM, Rachel Heaton
wrote:
> On Fri, Mar 25, 2022 at 6:22 AM Justin Pryzby pry...@telsasoft.com wrote:
>
> > On Fri, Mar 25, 2022 at 01:20:47AM -0400, Greg Stark wrote:
> >
> > > It seems development on this has stalled.
On Fri, Mar 25, 2022 at 01:20:47AM -0400, Greg Stark wrote:
> It seems development on this has stalled. If there's no further work
> happening I guess I'll mark the patch returned with feedback. Feel
> free to resubmit it to the next CF when there's progress.
Since it's a reasonably large patch
It seems development on this has stalled. If there's no further work
happening I guess I'll mark the patch returned with feedback. Feel
free to resubmit it to the next CF when there's progress.
The patch is failing on cfbot/freebsd.
http://cfbot.cputube.org/georgios-kokolatos.html
Also, I wondered if you'd looked at the "high compression" interfaces in
lz4hc.h ? Should pg_dump use that ?
On Fri, Feb 25, 2022 at 08:03:40AM -0600, Justin Pryzby wrote:
> Thanks for working on this. Your
On Fri, Feb 25, 2022 at 12:05:31PM +, Georgios wrote:
> The first commit does the heavy lifting required for additional compression
> methods.
> It expands testing coverage for the already supported gzip compression. Commit
> bf9aa490db introduced cfp in compress_io.{c,h} with the intent of
101 - 177 of 177 matches
Mail list logo
