The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
Thanks. All looks good, making it ready for committer.
Regards,
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
Hi,
The patch looks good to me, Just one suggestion
Hi Hackers,
I have been looking into adding parallel backup feature in pg_basebackup.
Currently pg_basebackup sends BASE_BACKUP command for taking full backup,
server scans the PGDATA and sends the files to pg_basebackup. In general,
server takes the following steps on BASE_BACKUP command:
- do
On Fri, Aug 23, 2019 at 3:18 PM Asim R P wrote:
> Hi Asif
>
> Interesting proposal. Bulk of the work in a backup is transferring files
> from source data directory to destination. Your patch is breaking this
> task down in multiple sets of files and transferring each set in parallel.
> This
Hi Robert,
Thanks for the feedback. Please see the comments below:
On Tue, Sep 24, 2019 at 10:53 PM Robert Haas wrote:
> On Wed, Aug 21, 2019 at 9:53 AM Asif Rehman
> wrote:
> > - BASE_BACKUP [PARALLEL] - returns a list of files in PGDATA
> > If the parallel option is there,
On Thu, Oct 3, 2019 at 6:40 PM Robert Haas wrote:
> On Fri, Sep 27, 2019 at 12:00 PM Asif Rehman
> wrote:
> >> > - SEND_FILES_CONTENTS (file1, file2,...) - returns the files in given
> list.
> >> > pg_basebackup will then send back a list of filenames
On Fri, Nov 1, 2019 at 8:53 PM Robert Haas wrote:
> On Wed, Oct 30, 2019 at 10:16 AM Asif Rehman
> wrote:
> > 'startptr' is used by sendFile() during checksum verification. Since
> > SendBackupFiles() is using sendFIle we have to set a valid WAL location.
>
> Ugh, g
On Mon, Oct 28, 2019 at 8:29 PM Robert Haas wrote:
> On Mon, Oct 28, 2019 at 10:03 AM Asif Rehman
> wrote:
> > I have updated the patch to include the changes suggested by Jeevan.
> This patch also implements the thread workers instead of
> > processes and fetches a
On Wed, Nov 27, 2019 at 1:38 PM Jeevan Chalke <
jeevan.cha...@enterprisedb.com> wrote:
>
>
> On Wed, Nov 13, 2019 at 7:04 PM Asif Rehman
> wrote:
>
>>
>> Sorry, I sent the wrong patches. Please see the correct version of the
>> patches (_v6).
>>
a list of files to process after
> somebody else has started the backup on another connection. Or maybe
> nobody wants to do any of those things, but it doesn't seem to cost us
> much of anything to split the commands, so I think we should.
>
+1
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
On Mon, Oct 7, 2019 at 6:35 PM Asif Rehman wrote:
>
>
> On Mon, Oct 7, 2019 at 6:05 PM Robert Haas wrote:
>
>> On Mon, Oct 7, 2019 at 8:48 AM Asif Rehman
>> wrote:
>> > Sure. Though the backup manifest patch calculates and includes the
>> checksum
On Thu, Oct 24, 2019 at 3:21 PM Ibrar Ahmed wrote:
>
>
> On Fri, Oct 18, 2019 at 4:12 PM Jeevan Chalke <
> jeevan.cha...@enterprisedb.com> wrote:
>
>>
>>
>> On Thu, Oct 17, 2019 at 10:51 AM Asif Rehman
>> wrote:
>>
>>>
>>>
, are still
expected in PGDATA.
I can make these changes part of parallel backup (which would be on top of
backup manifest patch) or
these changes can be done as part of manifest patch and then parallel can
use them.
Robert what do you suggest?
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
On Mon, Oct 7, 2019 at 6:05 PM Robert Haas wrote:
> On Mon, Oct 7, 2019 at 8:48 AM Asif Rehman wrote:
> > Sure. Though the backup manifest patch calculates and includes the
> checksum of backup files and is done
> > while the file is being transferred to the frontend-end. T
ow();
now
---
2020-03-05 12:13:29.775373+00
(1 row)
postgres=# select test.v2;
v2
--------
2020-03-05 12:13:37.192317 -- I was expecting this to be earlier than the
above timestamp.
(1 row)
postgres=# select test.v2;
v2
2020-03-05 12:13:37.192317
(1 row)
postgres=# let test.v2 = default;
LET
postgres=# select test.v2;
v2
2020-03-05 12:14:07.538615
(1 row)
To continue my testing of the patch I made few fixes for the above-mentioned
comments. The patch for those changes is attached if it could be of any use.
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
sv-fixes.patch
Description: Binary data
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
The patch applies cleanly and works as expected. Just a few
Hi,
I have created a commitfest entry.
https://commitfest.postgresql.org/27/2472/
On Mon, Feb 17, 2020 at 1:39 PM Asif Rehman wrote:
> Thanks Jeevan. Here is the documentation patch.
>
> On Mon, Feb 10, 2020 at 6:49 PM Jeevan Chalke <
> jeevan.cha...@enterprisedb.com> w
looked at your
> > patch closely.. Sorry.
>
> Here is finally a rebased patch for the (IMO) more important issue in
> pg_basebackup. I've added a commitfest entry for this now:
> https://commitfest.postgresql.org/25/2308/
>
>
>
Hi Michael,
The patch does not seem to app
Thanks Jeevan. Here is the documentation patch.
On Mon, Feb 10, 2020 at 6:49 PM Jeevan Chalke <
jeevan.cha...@enterprisedb.com> wrote:
> Hi Asif,
>
> On Thu, Jan 30, 2020 at 7:10 PM Asif Rehman
> wrote:
>
>>
>> Here are the the updated patches, taking care
On Thu, Dec 19, 2019 at 10:47 PM Robert Haas wrote:
> On Thu, Dec 12, 2019 at 10:20 AM Asif Rehman
> wrote:
> > I have updated the patches (v7 attached) and have taken care of all
> issues pointed by Jeevan, additionally
> > ran the pgindent on each patch. Furthermore, Co
edb@localhost bin]$
> [edb@localhost bin]$ ./pg_basebackup -v -D /home/edb/Desktop/backup/
> pg_basebackup: initiating base backup, waiting for checkpoint to complete
> pg_basebackup: checkpoint completed
> pg_basebackup: write-ahead log start point: 0/C528 on timeline 1
> pg_baseba
On Tue, Apr 7, 2020 at 10:03 PM Jeevan Chalke <
jeevan.cha...@enterprisedb.com> wrote:
>
>
> On Tue, Apr 7, 2020 at 10:14 PM Asif Rehman
> wrote:
>
>> Hi,
>>
>> Thanks, Kashif and Rajkumar. I have fixed the reported issues.
>>
>> I have adde
strtol_l_internal (nptr=0x0, endptr=0x0, base=10, group= optimized out>, loc=0x392158ee40) at ../stdlib/strtol_l.c:298
> #1 0x003921233b30 in atoi (nptr=) at atoi.c:28
> #2 0x0040841e in main (argc=5, argv=0x7ffeaa6fb968) at
> pg_basebackup.c:2526
>
> Thanks & R
load)
On Tue, Apr 21, 2020 at 9:27 AM Amit Kapila wrote:
> On Tue, Apr 14, 2020 at 8:07 PM Asif Rehman
> wrote:
>
>>
>> I forgot to make a check for no-manifest. Fixed. Attached is the updated
>> patch.
>>
>>
> Have we done any performance testing with th
On Tue, 21 Apr 2020 at 2:36 PM, Jeevan Ladhe
wrote:
> Hi Asif,
>
> On Tue, Apr 21, 2020 at 1:00 PM Asif Rehman
> wrote:
>
>> Hi,
>>
>> I did some tests a while back, and here are the results. The tests were
>> done to simulate
>> a live data
be an expected behavior. The START_BACKUP command has been
executed, and
pg_basebackup tries to start a WAL streaming process with a non-existent
slot, which results in
an error. So the backup is aborted while terminating all other processes.
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
On Thu, Apr 2, 2020 at 8:45 PM Robert Haas wrote:
> On Thu, Apr 2, 2020 at 11:17 AM Asif Rehman
> wrote:
> >> Why would you need to do that? As long as the process where
> >> STOP_BACKUP can do the check, that seems good enough.
> >
> > Yes, but th
PB_BACKUP_COMPLETE;
>>>> +}
>>>> +break;
>>>>
>>> Done.
>
>>>> Why free_filelist() is not called in PB_FETCH_WAL_FILES case?
>>>>
>>> Done.
The corrupted tablespace and crash, reported by Rajkumar, have been fixed.
A pointer
variable remained uninitialized which in turn caused the system to
misbehave.
Attached is the updated set of patches. AFAIK, to complete parallel backup
feature
set, there remain three sub-features:
1- parallel backup does not work with a standby server. In parallel backup,
the server
spawns multiple processes and there is no shared state being maintained. So
currently,
no way to tell multiple processes if the standby was promoted during the
backup since
the START_BACKUP was called.
2- throttling. Robert previously suggested that we implement throttling on
the client-side.
However, I found a previous discussion where it was advocated to be added
to the
backend instead[1].
So, it was better to have a consensus before moving the throttle function
to the client.
That’s why for the time being I have disabled it and have asked for
suggestions on it
to move forward.
It seems to me that we have to maintain a shared state in order to support
taking backup
from standby. Also, there is a new feature recently committed for backup
progress
reporting in the backend (pg_stat_progress_basebackup). This functionality
was recently
added via this commit ID: e65497df. For parallel backup to update these
stats, a shared
state will be required.
Since multiple pg_basebackup can be running at the same time, maintaining a
shared state
can become a little complex, unless we disallow taking multiple parallel
backups.
So proceeding on with this patch, I will be working on:
- throttling to be implemented on the client-side.
- adding a shared state to handle backup from the standby.
[1]
https://www.postgresql.org/message-id/flat/521B4B29.20009%402ndquadrant.com#189bf840c87de5908c0b4467d31b50af
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
<>
On Thu, Apr 2, 2020 at 4:47 PM Robert Haas wrote:
> On Fri, Mar 27, 2020 at 1:34 PM Asif Rehman
> wrote:
> > Yes, we are fetching a single file. However, SEND_FILES is still capable
> of fetching multiple files in one
> > go, that's why the name.
>
> I don't see
On Thu, Apr 23, 2020 at 11:43 AM Rajkumar Raghuwanshi <
rajkumar.raghuwan...@enterprisedb.com> wrote:
>
>
> On Wed, Apr 22, 2020 at 7:48 PM Asif Rehman
> wrote:
>
>>
>> Hi Dipesh,
>>
>> The rebased and updated patch is attached. Its rebased to (9f
nds like terrible behavior.
>
Removed the warning and generated an error if other then a regular file is
requested.
>
>
> + /*
> +* Check for checksum failures. If there are failures across
> multiple
> +* processes it may not report total checksum count, but it will
> error
> +* out,terminating the backup.
> +*/
>
> In other words, the patch breaks the feature. Not that the feature in
> question works particularly well as things stand, but this makes it
> worse.
>
Added an atomic uint64 total_checksum_failures to shared state to keep
the total count across workers, So it will have the same behavior as
current.
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
<>
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
The patch applies cleanly and works as expected.
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:tested, passed
The patch applies cleanly and looks fine to me. However
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
Patch looks good to me.
The new status of this patch is: Ready
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:tested, passed
Hi,
The patch looks good to me.
The new status of this
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
The patch looks good to me.
The new status of this patch is:
Hi,
The patch looks fine to me, however there is one hunk failing for the test
case, so it needs to be rebased.
The new status of this patch is: Waiting on Author
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:tested, passed
The patch applies cleanly and AFAICS there are no issues with
fixed patch
>
> Thank you for check
>
> Pavel
>
> cheers ./daniel
>>
>
Hi Pavel,
Since the idea originated from unescaping unicode string literals i.e.
select unescape('Odpov\u011Bdn\u00E1 osoba');
Shouldn't the built-in function support the above syntax as well?
--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
The patch applies cleanly and looks fine to me. However consider
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
Hi David,
I was reviewing this patch and the compilation failed with
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
This patch looks fine to me. master 48cb244fb9
The new status
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
Hi,
I have tested this patch. This patch still applies and it
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation:not tested
The patch does not apply on HEAD anymore. Looks like it needs to be rebased.
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
The patch applies cleanly and the functionality seems to work
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation:not tested
Wouldn't using opt_or_replace rule be a better option?
The new
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation:not tested
The patch applies with few "Hunk succeeded, offset -3 lines"
47 matches
Mail list logo
