Re: ERROR: "ft1" is of the wrong type.
On Wed, Jun 30, 2021 at 5:56 AM Kyotaro Horiguchi wrote: > At Tue, 29 Jun 2021 20:13:14 +0000, ahsan hadi > wrote in > > 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 > > > > I have tested it with various object types and getting a meaningful > error. > > Thanks for looking this, Ahsan. > > However, Peter-E is proposing a change at a fundamental level, which > looks more promising (disregarding backpatch burden). > > > https://www.postgresql.org/message-id/01d4fd55-d4fe-5afc-446c-a7f99e043...@enterprisedb.com Sure I will also take a look at this patch. +1 for avoiding the backpatching burden. > > > regards. > > -- > Kyotaro Horiguchi > NTT Open Source Software Center > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: ERROR: "ft1" is of the wrong type.
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 I have tested it with various object types and getting a meaningful error.
Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
On Sat, May 8, 2021 at 8:17 AM Japin Li wrote: > > On Fri, 07 May 2021 at 19:50, ahsan hadi wrote: > > 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 > > > > I have also seen this error with logical replication using pglogical > extension, will this patch also address similar problem with pglogical? > > Does there is a test case to reproduce this problem (using pglogical)? > I encountered this, however I'm not find a case to reproduce it. > I have seen a user run into this with pglogical, the error is produced after logical decoding finds an inconsistent point. However we haven't been able to reproduce the user scenario locally... > -- > Regrads, > Japin Li. > ChengDu WenWu Information Technology Co.,Ltd. > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
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 I have also seen this error with logical replication using pglogical extension, will this patch also address similar problem with pglogical?
Re: pg_dump new feature: exporting functions only. Bad or good idea ?
The following review has been posted through the commitfest application: make installcheck-world: tested, failed Implements feature: tested, passed Spec compliant: tested, passed Documentation:not tested Few minor comments : - The latest patch has some hunk failures - Regression with master has many failures with/without the patch, it is difficult to tell if the patch is causing any failures. - This is probably intended behaviour that --functions-only switch is also dumping stored procedures? - If i create a procedure with LANGUAGE plpgsql SECURITY INVOKER It is not including "SECURITY INVOKER" in the dump. That's probably because INVOKER is default security rights.
Re: Add session statistics to pg_stat_database
Hi Laurenz, I have applied the latest patch on master, all the regression test cases are passing and the implemented functionality is also looking fine. The point that I raised about idle connection not included is also addressed. thanks, Ahsan On Wed, Oct 14, 2020 at 2:28 PM Laurenz Albe wrote: > Thanks for the --- as always --- valuable review! > > On Tue, 2020-10-13 at 17:55 -0500, Justin Pryzby wrote: > > On Tue, Oct 13, 2020 at 01:44:41PM +0200, Laurenz Albe wrote: > > > Attached is v3 with improvements. > > > > + > > + Time spent in database sessions in this database, in > milliseconds. > > + > > > > Should say "Total time spent *by* DB sessions..." ? > > That is indeed better. Fixed. > > > I think these counters are only accurate as of the last state change, > right? > > So a session which has been idle for 1hr, that 1hr is not included. I > think > > the documentation should explain that, or (ideally) the implementation > would be > > more precise. Maybe the timestamps should only be updated after a > session > > terminates (and the docs should say so). > > I agree, and I have added an explanation that the value doesn't include > the duration of the current state. > > Of course it would be nice to have totally accurate values, but I think > that the statistics are by nature inaccurate (datagrams can get lost), > and more frequent statistics updates increase the work load. > I don't think that is worth the effort. > > > + > > + connections bigint > > + > > + > > + Number of connections established to this database. > > > > *Total* number of connections established, otherwise it sounds like it > might > > mean "the number of sessions [currently] established". > > Fixed like that. > > > + Number of database sessions to this database that did not end > > + with a regular client disconnection. > > > > Does that mean "sessions which ended irregularly" ? Or does it also > include > > "sessions which have not ended" ? > > I have added an explanation for that. > > > + msg.m_aborted = (!disconnect || pgStatSessionDisconnected) ? 0 : > 1; > > > > I think this can be just: > > msg.m_aborted = (bool) (disconnect && !pgStatSessionDisconnected); > > I mulled over this and finally decided to leave it as it is. > > Since "m_aborted" gets added to the total counter, I'd prefer to > have it be an "int". > > Your proposed code works (the cast is actually not necessary, right?). > But I think that my version is more readable if you think of > "m_aborted" as a counter rather than a flag. > > > + if ((dbentry = pgstat_fetch_stat_dbentry(dbid)) == NULL) > > + result = 0; > > + else > > + result = ((double) dbentry->n_session_time) / 1000.0; > > > > I think these can say: > > > double result = 0; > > > if ((dbentry=..) != NULL) > > > result = (double) ..; > > > > That not only uses fewer LOC, but also the assignment to zero is (known > to be) > > done at compile time (BSS) rather than runtime. > > I didn't know about the performance difference. > Concise code (if readable) is good, so I changed the code like you propose. > > The code pattern is actually copied from neighboring functions, > which then should also be changed like this, but that is outside > the scope of this patch. > > Attached is v4 of the patch. > > Yours, > Laurenz Albe > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: Performing partition pruning using row value
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 I have performed testing of the patch with row comparison partition pruning scenarios, it is working well. I didn't code review hence not changing the status.
Re: Add session statistics to pg_stat_database
On Wed, Jul 8, 2020 at 4:17 PM Laurenz Albe wrote: > Here is a patch that adds the following to pg_stat_database: > - number of connections > Is it expected behaviour to not count idle connections? The connection is included after it is aborted but not while it was idle. > - number of sessions that were not disconnected regularly > - total time spent in database sessions > - total time spent executing queries > - total idle in transaction time > > This is useful to check if connection pooling is working. > It also helps to estimate the size of the connection pool > required to keep the database busy, which depends on the > percentage of the transaction time that is spent idling. > > Yours, > Laurenz Albe > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: Transactions involving multiple postgres foreign servers, take 2
On Fri, Jul 17, 2020 at 9:56 PM Fujii Masao
wrote:
>
>
> On 2020/07/16 14:47, Masahiko Sawada wrote:
> > On Tue, 14 Jul 2020 at 11:19, Fujii Masao
> wrote:
> >>
> >>
> >>
> >> On 2020/07/14 9:08, Masahiro Ikeda wrote:
> I've attached the latest version patches. I've incorporated the review
> comments I got so far and improved locking strategy.
> >>>
> >>> Thanks for updating the patch!
> >>
> >> +1
> >> I'm interested in these patches and now studying them. While checking
> >> the behaviors of the patched PostgreSQL, I got three comments.
> >
> > Thank you for testing this patch!
> >
> >>
> >> 1. We can access to the foreign table even during recovery in the HEAD.
> >> But in the patched version, when I did that, I got the following error.
> >> Is this intentional?
> >>
> >> ERROR: cannot assign TransactionIds during recovery
> >
> > No, it should be fixed. I'm going to fix this by not collecting
> > participants for atomic commit during recovery.
>
> Thanks for trying to fix the issues!
>
> I'd like to report one more issue. When I started new transaction
> in the local server, executed INSERT in the remote server via
> postgres_fdw and then quit psql, I got the following assertion failure.
>
> TRAP: FailedAssertion("fdwxact", File: "fdwxact.c", Line: 1570)
> 0 postgres0x00010d52f3c0
> ExceptionalCondition + 160
> 1 postgres0x00010cefbc49
> ForgetAllFdwXactParticipants + 313
> 2 postgres0x00010cefff14
> AtProcExit_FdwXact + 20
> 3 postgres0x00010d313fe3 shmem_exit + 179
> 4 postgres0x00010d313e7a
> proc_exit_prepare + 122
> 5 postgres0x00010d313da3 proc_exit + 19
> 6 postgres0x00010d35112f PostgresMain +
> 3711
> 7 postgres0x00010d27bb3a BackendRun + 570
> 8 postgres0x00010d27af6b BackendStartup
> + 475
> 9 postgres0x00010d279ed1 ServerLoop + 593
> 10 postgres0x00010d277940 PostmasterMain
> + 6016
> 11 postgres0x00010d1597b9 main + 761
> 12 libdyld.dylib 0x7fff7161e3d5 start + 1
> 13 ??? 0x0003 0x0 + 3
>
I have done a test with the latest set of patches shared by Swada and I am
not able to reproduce this issue. Started a prepared transaction on the
local server and then did a couple of inserts in a remote table using
postgres_fdw and the quit psql. I am not able to reproduce the assertion
failure.
>
> Regards,
>
>
> --
> Fujii Masao
> Advanced Computing Technology Center
> Research and Development Headquarters
> NTT DATA CORPORATION
>
>
>
--
Highgo Software (Canada/China/Pakistan)
URL : http://www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
EMAIL: mailto: ahsan.h...@highgo.ca
Re: Added tab completion for the missing options in copy statement
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 Tested the tab complete for copy command, it provides the tab completion after providing the "TO|FROM filename With|Where". Does this require any doc change? The new status of this patch is: Waiting on Author
Re: Improving psql slash usage help message
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: not tested Spec compliant: not tested Documentation:tested, passed This small documentation patch makes the document more accurate for psql terminal help. The new status of this patch is: Waiting on Author
Re: WIP/PoC for parallel backup
On Mon, May 4, 2020 at 6:22 PM Rushabh Lathia wrote: > > > On Thu, Apr 30, 2020 at 4:15 PM Amit Kapila > wrote: > >> On Wed, Apr 29, 2020 at 6:11 PM Suraj Kharage >> wrote: >> > >> > Hi, >> > >> > We at EnterpriseDB did some performance testing around this parallel >> backup to check how this is beneficial and below are the results. In this >> testing, we run the backup - >> > 1) Without Asif’s patch >> > 2) With Asif’s patch and combination of workers 1,2,4,8. >> > >> > We run those test on two setup >> > >> > 1) Client and Server both on the same machine (Local backups) >> > >> > 2) Client and server on a different machine (remote backups) >> > >> > >> > Machine details: >> > >> > 1: Server (on which local backups performed and used as server for >> remote backups) >> > >> > 2: Client (Used as a client for remote backups) >> > >> > >> ... >> > >> > >> > Client & Server on the same machine, the result shows around 50% >> improvement in parallel run with worker 4 and 8. We don’t see the huge >> performance improvement with more workers been added. >> > >> > >> > Whereas, when the client and server on a different machine, we don’t >> see any major benefit in performance. This testing result matches the >> testing results posted by David Zhang up thread. >> > >> > >> > >> > We ran the test for 100GB backup with parallel worker 4 to see the CPU >> usage and other information. What we noticed is that server is consuming >> the CPU almost 100% whole the time and pg_stat_activity shows that server >> is busy with ClientWrite most of the time. >> > >> > >> >> Was this for a setup where the client and server were on the same >> machine or where the client was on a different machine? If it was for >> the case where both are on the same machine, then ideally, we should >> see ClientRead events in a similar proportion? >> > > In the particular setup, the client and server were on different machines. > > >> During an offlist discussion with Robert, he pointed out that current >> basebackup's code doesn't account for the wait event for the reading >> of files which can change what pg_stat_activity shows? Can you please >> apply his latest patch to improve basebackup.c's code [1] which will >> take care of that waitevent before getting the data again? >> >> [1] - >> https://www.postgresql.org/message-id/CA%2BTgmobBw-3573vMosGj06r72ajHsYeKtksT_oTxH8XvTL7DxA%40mail.gmail.com >> > > > Sure, we can try out this and do a similar run to collect the > pg_stat_activity output. > Have you had the chance to try this out? > > >> -- >> With Regards, >> Amit Kapila. >> EnterpriseDB: http://www.enterprisedb.com >> >> >> > > -- > Rushabh Lathia > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: create partition table caused server crashed with self-referencing foreign key
On Wed, Apr 22, 2020 at 2:45 PM amul sul wrote: > > > On Wed, Apr 22, 2020 at 2:59 PM amul sul wrote: > >> >> >> On Wed, Apr 22, 2020 at 2:27 PM David Rowley >> wrote: >> >>> On Wed, 22 Apr 2020 at 20:11, amul sul wrote: >>> > >>> > On Wed, Apr 22, 2020 at 1:21 PM Rajkumar Raghuwanshi < >>> rajkumar.raghuwan...@enterprisedb.com> wrote: >>> >> #2 0x00acd16a in ExceptionalCondition >>> (conditionName=0xc32310 "numfks == attmap->maplen", errorType=0xc2ea23 >>> "FailedAssertion", fileName=0xc2f0bf "tablecmds.c", lineNumber=9046) at >>> assert.c:67 >>> > >>> > >>> > Looks like this assertion is incorrect, I guess it should have check >>> > numfks <= attmap->maplen instead. >>> >>> Even that seems like a very strange thing to Assert. Basically it's >>> saying, make sure the number of columns in the foreign key constraint >>> is less than or equal to the number of attributes in parentRel. >>> >>> It's true we do disallow duplicate column names in the foreign key >>> constraint (at least since 9da867537), but why do we want an Assert to >>> say that? I don't see anything about that code that would break if we >>> did happen to allow duplicate columns in the foreign key. I'd say the >>> Assert should just be removed completely. >>> >> >> Understood and agree with you. >> >> Attached patch removes this assertion and does a slight tweak to >> regression test >> to generate case where numfks != attmap->maplen, IMO, we should have this >> even if there is nothing that checks it. Thoughts? >> > > Kindly ignore the previously attached patch, correct patch attached here. > I did a quick test of the fix, the assertion failure is fixed and regression is not reporting any failures. > > Regards, > Amul > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: WIP/PoC for parallel backup
On Tue, Apr 21, 2020 at 4:50 PM Amit Kapila wrote: > On Tue, Apr 21, 2020 at 5:18 PM Amit Kapila > wrote: > > > > On Tue, Apr 21, 2020 at 1:00 PM Asif Rehman > wrote: > > > > > > I did some tests a while back, and here are the results. The tests > were done to simulate > > > a live database environment using pgbench. > > > > > > machine configuration used for this test: > > > Instance Type:t2.xlarge > > > Volume Type :io1 > > > Memory (MiB) :16384 > > > vCPU # :4 > > > Architecture:X86_64 > > > IOP :16000 > > > Database Size (GB) :102 > > > > > > The setup consist of 3 machines. > > > - one for database instances > > > - one for pg_basebackup client and > > > - one for pgbench with some parallel workers, simulating SELECT loads. > > > > > >basebackup | 4 workers | 8 Workers > | 16 workers > > > Backup Duration(Min): 69.25| 20.44 | 19.86 | > 20.15 > > > (pgbench running with 50 parallel client simulating SELECT load) > > > > > > Backup Duration(Min): 154.75 | 49.28 | 45.27 | > 20.35 > > > (pgbench running with 100 parallel client simulating SELECT load) > > > > > > > Thanks for sharing the results, these show nice speedup! However, I > > think we should try to find what exactly causes this speed up. If you > > see the recent discussion on another thread related to this topic, > > Andres, pointed out that he doesn't think that we can gain much by > > having multiple connections[1]. It might be due to some internal > > limitations (like small buffers) [2] due to which we are seeing these > > speedups. It might help if you can share the perf reports of the > > server-side and pg_basebackup side. > > > > Just to be clear, we need perf reports both with and without patch-set. > These tests were done a while back, I think it would be good to run the benchmark again with the latest patches of parallel backup and share the results and perf reports. > > -- > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com > > > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: WIP/PoC for parallel backup
On Wed, 15 Apr 2020 at 1:49 AM, Robert Haas wrote:
> On Tue, Apr 14, 2020 at 10:37 AM Asif Rehman
> wrote:
> > I forgot to make a check for no-manifest. Fixed. Attached is the updated
> patch.
>
> +typedef struct
> +{
> ...
> +} BackupFile;
> +
> +typedef struct
> +{
> ...
> +} BackupState;
>
> These structures need comments.
>
> +list_wal_files_opt_list:
> + SCONST SCONST
> {
> - $$ = makeDefElem("manifest_checksums",
> -
> (Node *)makeString($2), -1);
> + $$ = list_make2(
> + makeDefElem("start_wal_location",
> + (Node *)makeString($2),
> -1),
> + makeDefElem("end_wal_location",
> + (Node *)makeString($2),
> -1));
> +
> }
>
> This seems like an unnecessarily complicated parse representation. The
> DefElems seem to be completely unnecessary here.
>
> @@ -998,7 +1110,37 @@ SendBaseBackup(BaseBackupCmd *cmd)
> set_ps_display(activitymsg);
> }
>
> - perform_base_backup(&opt);
> + switch (cmd->cmdtag)
>
> So the design here is that SendBaseBackup() is now going to do a bunch
> of things that are NOT sending a base backup? With no updates to the
> comments of that function and no change to the process title it sets?
>
> - return (manifest->buffile != NULL);
> + return (manifest && manifest->buffile != NULL);
>
> Heck no. It appears that you didn't even bother reading the function
> header comment.
>
> + * Send a single resultset containing XLogRecPtr record (in text format)
> + * TimelineID and backup label.
> */
> static void
> -SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli)
> +SendXlogRecPtrResult(XLogRecPtr ptr, TimeLineID tli,
> +StringInfo label, char *backupid)
>
> This just casually breaks wire protocol compatibility, which seems
> completely unacceptable.
>
> + if (strlen(opt->tablespace) > 0)
> + sendTablespace(opt->tablespace, NULL, true, NULL, &files);
> + else
> + sendDir(".", 1, true, NIL, true, NULL, NULL, &files);
> +
> + SendFilesHeader(files);
>
> So I guess the idea here is that we buffer the entire list of files in
> memory, regardless of size, and then we send it out afterwards. That
> doesn't seem like a good idea. The list of files might be very large.
> We probably need some code refactoring here rather than just piling
> more and more different responsibilities onto sendTablespace() and
> sendDir().
>
> + if (state->parallel_mode)
> + SpinLockAcquire(&state->lock);
> +
> + state->throttling_counter += increment;
> +
> + if (state->parallel_mode)
> + SpinLockRelease(&state->lock);
>
> I don't like this much. It seems to me that we would do better to use
> atomics here all the time, instead of conditional spinlocks.
>
> +static void
> +send_file(basebackup_options *opt, char *file, bool missing_ok)
> ...
> + if (file == NULL)
> + return;
>
> That seems totally inappropriate.
>
> + sendFile(file, file + basepathlen, &statbuf,
> true, InvalidOid, NULL, NULL);
>
> Maybe I'm misunderstanding, but this looks like it's going to write a
> tar header, even though we're not writing a tarfile.
>
> + else
> + ereport(WARNING,
> + (errmsg("skipping special file
> or directory \"%s\"", file)));
>
> So, if the user asks for a directory or symlink, what's going to
> happen is that they're going to receive an empty file, and get a
> warning. That sounds like terrible behavior.
>
> + /*
> +* 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.
>
> I think this patch (0003) is in really bad shape. I'm having second
> thoughts about the design, but it's kind of hard to even have a
> discussion about the design when the patch is riddled with minor
> problems like inadequate comments, failure to update existing
> comments, and breaking a bunch of things. I understand that sometimes
> things get missed, but this is version 14 of a patch that's been
> kicking around since last August.
Fair enough. Some of this is also due to backup related features i.e backup
manifest, progress reporting that got committed to master towards the tail
end of PG-13. Rushing to get parallel backup feature compatible with these
features also caused some of the oversights.
>
> --
> Robert Haas
>
Re: 2pc leaks fds
I have tested with and without the commit from Andres using the pgbench script (below) provided in the initial email. pgbench -n -s 500 -c 4 -j 4 -T 10 -P1 -f pgbench-write-2pc.sql I am not getting the leak anymore, it seems to be holding up pretty well. On Wed, Apr 8, 2020 at 12:59 PM Antonin Houska wrote: > Andres Freund wrote: > > > On 2020-04-06 09:12:32 +0200, Antonin Houska wrote: > > > Andres Freund wrote: > > > It should have allowed users to have different ways to *locate the > segment* > > > file. The WALSegmentOpen callback could actually return file path > instead of > > > the file descriptor and let WALRead() perform the opening/closing, but > then > > > the WALRead function would need to be aware whether it is executing in > backend > > > or in frontend (so it can use the correct function to open/close the > file). > > > > > > I was aware of the problem that the correct function should be used to > open > > > the file and that's why this comment was added (although "mandatory" > would be > > > more suitable than "preferred"): > > > > > > * BasicOpenFile() is the preferred way to open the segment file in > backend > > > * code, whereas open(2) should be used in frontend. > > > */ > > > typedef int (*WALSegmentOpen) (XLogSegNo nextSegNo, WALSegmentContext > *segcxt, > > >TimeLineID > *tli_p); > > > > I don't think that BasicOpenFile() really solves anything here? If > > anything it *exascerbates* the problem, because it will trigger all of > > the "virtual file descriptors" for already opened Files to close() the > > underlying OS FDs. So not even a fully cached table can be seqscanned, > > because that tries to check the file size... > > Specifically for 2PC, isn't it better to close some OS-level FD of an > unrelated table scan and then succeed than to ERROR immediately? Anyway, > 0dc8ead46 hasn't changed this. > > I at least admit that the comment should not recommend particular function, > and that WALRead() should call the appropriate function to close the file, > rather than always calling close(). > > Can we just pass the existing FD to the callback as an additional argument, > and let the callback close it? > > -- > Antonin Houska > Web: https://www.cybertec-postgresql.com > > > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: Internal key management system
Hi Bruce/Joe, In the last meeting we discussed the need for improving the documentation for KMS so it is easier to understand the interface. Cary from highgo had a go at doing that, please see the previous email on this thread from Cary and let us know if it looks good...? -- Ahsan On Wed, Apr 8, 2020 at 3:46 AM Cary Huang wrote: > Hello > > Thanks a lot for the patch, I think in terms of functionality, the patch > provides very straightforward functionalities regarding key management. In > terms of documentation, I think the patch is still lacking some pieces of > information that kind of prevent people from fully understanding how KMS > works and how it can be used and why, (at least that is the impression I > got from the zoom meeting recordings :p). I spent some time today > revisiting the key-management documentation in the patch and rephrase and > restructure it based on my current understanding of latest KMS design. I > mentioned all 3 application level keys that we have agreed and emphasize on > explaining the SQL level encryption key because that is the key that can be > used right now. Block and WAL levels keys we can add here more information > once they are actually used in the TDE development. > > Please see below the KMS documentation that I have revised and I hope it > will be more clear and easier for people to understand KMS. Feel free to > make adjustments. Please note that we use the term "wrap" and "unwrap" a > lot in our past discussions. Originally we used the terms within a context > involving Key encryption keys (KEK). For example, "KMS wraps a master key > with KEK". Later, we used the same term in a context involving encrypting > user secret /password. For example, "KMS wraps a user secret with SQL key". > In my opinion, both make sense but it may be confusing to people having the > same term used differently. So in my revision below, the terms "wrap" and > "unwrap" refer to encrypting or decrypting user secret / password as they > are used in "pg_wrap() and pg_unwrap()". I use the terms "encapsulate" and > "restore" when KEK is used to encrypt or decrypt a key. > > > > Chapter 32: Encryption Key Management > -- > > PostgreSQL supports internal Encryption Key Management System, which is > designed to manage the life cycles of cryptographic keys within the > PostgreSQL system. This includes dealing with their generation, storage, > usage and rotation. > > Encryption Key Management is enabled when PostgreSQL is build > with --with-openssl and cluster passphrase command is specified > during initdb. The cluster passphrase provided > by --cluster-passphrase-command option during initdb and the one generated > by cluster_passphrase_command in the postgresql.conf must match, otherwise, > the database cluster will not start up. > > 32.1 Key Generations and Derivations > -- > > When cluster_passphrase_command option is specified to the initdb, the > process will derive the cluster passphrase into a Key Encryption Key (KEK) > and a HMAC Key using key derivation protocol before the actual generation > of application level cryptographic level keys. > > -Key Encryption Key (KEK) > KEK is primarily used to encapsulate or restore a given application level > cryptographic key > > -HMAC Key > HMAC key is used to compute the HASH of a given application level > cryptographic key for integrity check purposes > > These 2 keys are not stored physically within the PostgreSQL cluster as > they are designed to be derived from the correctly configured cluster > passphrase. > > Encryption Key Management System currently manages 3 application level > cryptographic keys that have different purposes and usages within the > PostgreSQL system and these are generated using pg_strong_random() after > KEK and HMAC key derivation during initdb process. > > The 3 keys are: > > -SQL Level Key > SQL Level Key is used to wrap and unwrap a user secret / passphrase via > pg_wrap() and pg_unwrap() SQL functions. These 2 functions are designed to > be used in conjunction with the cryptographic functions provided by > pgcrypto extension to perform column level encryption/decryption without > having to supply a clear text user secret or passphrase that is required by > many pgcrypto functions as input. Please refer to [Wrap and Unwrap User > Secret section] for usage examples. > > -Block Level Key > Block Level Key is primarily used to encrypt / decrypt buffers as part of > the Transparent Data Encryption (TDE) feature > > -WAL Level Key > WAL Level Key is primarily used to encrypt / decrypt WAL files as part of > the Transparent Data Encryption (TDE) feature > > The 3 application level keys above will be encapsulated and hashed using > KEK and HMAC key mentioned above before they are physically stored to > pg_cryptokeys directory within the cluster. > > 32.1. Key Initialization > - > > When a PostgreSQL cluster w
Re: WIP/PoC for parallel backup
On Mon, Mar 30, 2020 at 3:44 PM Rajkumar Raghuwanshi <
rajkumar.raghuwan...@enterprisedb.com> wrote:
> Thanks Asif,
>
> I have re-verified reported issue. expect standby backup, others are fixed.
>
Yes As Asif mentioned he is working on the standby issue and adding
bandwidth throttling functionality to parallel backup.
It would be good to get some feedback on Asif previous email from Robert on
the design considerations for stand-by server support and throttling. I
believe all the other points mentioned by Robert in this thread are
addressed by Asif so it would be good to hear about any other concerns that
are not addressed.
Thanks,
-- Ahsan
> Thanks & Regards,
> Rajkumar Raghuwanshi
>
>
> On Fri, Mar 27, 2020 at 11:04 PM Asif Rehman
> wrote:
>
>>
>>
>> On Wed, Mar 25, 2020 at 12:22 PM Rajkumar Raghuwanshi <
>> rajkumar.raghuwan...@enterprisedb.com> wrote:
>>
>>> Hi Asif,
>>>
>>> While testing further I observed parallel backup is not able to take
>>> backup of standby server.
>>>
>>> mkdir /tmp/archive_dir
>>> echo "archive_mode='on'">> data/postgresql.conf
>>> echo "archive_command='cp %p /tmp/archive_dir/%f'">> data/postgresql.conf
>>>
>>> ./pg_ctl -D data -l logs start
>>> ./pg_basebackup -p 5432 -Fp -R -D /tmp/slave
>>>
>>> echo "primary_conninfo='host=127.0.0.1 port=5432 user=edb'">>
>>> /tmp/slave/postgresql.conf
>>> echo "restore_command='cp /tmp/archive_dir/%f %p'">>
>>> /tmp/slave/postgresql.conf
>>> echo "promote_trigger_file='/tmp/failover.log'">>
>>> /tmp/slave/postgresql.conf
>>>
>>> ./pg_ctl -D /tmp/slave -l /tmp/slave_logs -o "-p 5433" start -c
>>>
>>> [edb@localhost bin]$ ./psql postgres -p 5432 -c "select
>>> pg_is_in_recovery();"
>>> pg_is_in_recovery
>>> ---
>>> f
>>> (1 row)
>>>
>>> [edb@localhost bin]$ ./psql postgres -p 5433 -c "select
>>> pg_is_in_recovery();"
>>> pg_is_in_recovery
>>> ---
>>> t
>>> (1 row)
>>>
>>>
>>>
>>>
>>> *[edb@localhost bin]$ ./pg_basebackup -p 5433 -D /tmp/bkp_s --jobs
>>> 6pg_basebackup: error: could not list backup files: ERROR: the standby was
>>> promoted during online backupHINT: This means that the backup being taken
>>> is corrupt and should not be used. Try taking another online
>>> backup.pg_basebackup: removing data directory "/tmp/bkp_s"*
>>>
>>> #same is working fine without parallel backup
>>> [edb@localhost bin]$ ./pg_basebackup -p 5433 -D /tmp/bkp_s --jobs 1
>>> [edb@localhost bin]$ ls /tmp/bkp_s/PG_VERSION
>>> /tmp/bkp_s/PG_VERSION
>>>
>>> Thanks & Regards,
>>> Rajkumar Raghuwanshi
>>>
>>>
>>> On Thu, Mar 19, 2020 at 4:11 PM Rajkumar Raghuwanshi <
>>> rajkumar.raghuwan...@enterprisedb.com> wrote:
>>>
Hi Asif,
In another scenarios, bkp data is corrupted for tablespace. again this
is not reproducible everytime,
but If I am running the same set of commands I am getting the same
error.
[edb@localhost bin]$ ./pg_ctl -D data -l logfile start
waiting for server to start done
server started
[edb@localhost bin]$
[edb@localhost bin]$ mkdir /tmp/tblsp
[edb@localhost bin]$ ./psql postgres -p 5432 -c "create tablespace
tblsp location '/tmp/tblsp';"
CREATE TABLESPACE
[edb@localhost bin]$ ./psql postgres -p 5432 -c "create database
testdb tablespace tblsp;"
CREATE DATABASE
[edb@localhost bin]$ ./psql testdb -p 5432 -c "create table testtbl (a
text);"
CREATE TABLE
[edb@localhost bin]$ ./psql testdb -p 5432 -c "insert into testtbl
values ('parallel_backup with tablespace');"
INSERT 0 1
[edb@localhost bin]$ ./pg_basebackup -p 5432 -D /tmp/bkp -T
/tmp/tblsp=/tmp/tblsp_bkp --jobs 2
[edb@localhost bin]$ ./pg_ctl -D /tmp/bkp -l /tmp/bkp_logs -o "-p
" start
waiting for server to start done
server started
[edb@localhost bin]$ ./psql postgres -p -c "select * from
pg_tablespace where spcname like 'tblsp%' or spcname = 'pg_default'";
oid | spcname | spcowner | spcacl | spcoptions
---++--++
1663 | pg_default | 10 ||
16384 | tblsp | 10 ||
(2 rows)
[edb@localhost bin]$ ./psql testdb -p -c "select * from testtbl";
psql: error: could not connect to server: FATAL:
"pg_tblspc/16384/PG_13_202003051/16385" is not a valid data directory
DETAIL: File "pg_tblspc/16384/PG_13_202003051/16385/PG_VERSION" is
missing.
[edb@localhost bin]$
[edb@localhost bin]$ ls
data/pg_tblspc/16384/PG_13_202003051/16385/PG_VERSION
data/pg_tblspc/16384/PG_13_202003051/16385/PG_VERSION
[edb@localhost bin]$ ls
/tmp/bkp/pg_tblspc/16384/PG_13_202003051/16385/PG_VERSION
ls: cannot access
/tmp/bkp/pg_tblspc/16384/PG_13_202003051/16385/PG_VERSION: No such file or
directory
Thanks & Regards,
Rajkumar Raghuwanshi
On Mon, Mar 16, 2020 at 6:19 PM Rajkumar Raghuwanshi <
>>>
Re: more ALTER .. DEPENDS ON EXTENSION fixes
On Sat, Feb 29, 2020 at 2:38 AM Alvaro Herrera wrote: > On 2020-Feb-28, ahsan hadi wrote: > > > > Tested the pg_dump patch for dumping "ALTER .. DEPENDS ON EXTENSION" in > case of indexes, functions, triggers etc. The "ALTER .. DEPENDS ON > EXTENSION" is included in the dump. However in some case not sure why > "ALTER INDEX.DEPENDS ON EXTENSION" is repeated several times in the > dump? > > Hi, thanks for testing. > > Are the repeated commands for the same index, same extension? Yes same index and same extension... > Did you > apply the same command multiple times before running pg_dump? > Yes but in some cases I applied the command once and it appeared multiple times in the dump.. > > There was an off-list complaint that if you repeat the ALTER .. DEPENDS > for the same object on the same extension, then the same dependency is > registered multiple times. (You can search pg_depend for "deptype = 'x'" > to see that). I suppose that would lead to the line being output > multiple times by pg_dump, also. Is that what you did? > I checked out pg_depend for "deptype='x'" the same dependency is registered multiple times... > > If so: Patch 0002 is supposed to fix that problem, by raising an error > if the dependency is already registered ... though it occurs to me now > that it would be more in line with custom to make the command a silent > no-op. In fact, doing that would cause old dumps (generated with > databases containing duplicated entries) to correctly restore a single > entry, without error. Therefore my inclination now is to change 0002 > that way and push and backpatch it ahead of 0001. > Makes sense, will also try our Patch 0002. > > I realize just now that I have failed to verify what happens with > partitioned indexes. > Yes I also missed this one.. > > -- > Álvaro Herrerahttps://www.2ndQuadrant.com/ > PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: more ALTER .. DEPENDS ON EXTENSION fixes
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 Tested the pg_dump patch for dumping "ALTER .. DEPENDS ON EXTENSION" in case of indexes, functions, triggers etc. The "ALTER .. DEPENDS ON EXTENSION" is included in the dump. However in some case not sure why "ALTER INDEX.DEPENDS ON EXTENSION" is repeated several times in the dump?
Re: DROP OWNED CASCADE vs Temp tables
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 I have tested the patch with REL_12_STABLE for the given error scenario by running the "CREATE TEMPORARY TABLE..." and "DROP OWNED BY..." commands concurrently using parallel background workers. I have also tested a few related scenarios and the patch does seem to fix the reported bug. Ran make installcheck-world, no difference with and without patch.
Re: pg_restore crash when there is a failure before all child process is created
The following review has been posted through the commitfest application: make installcheck-world: tested, passed Implements feature: not tested Spec compliant: tested, passed Documentation:not tested I have applied tested both patches separately and ran regression with both. No new test cases are failing with both patches. The issues is fixed by both patches however the fix from Tom (0001-fix-worker-status.patch) looks more elegant. I haven't done a detailed code review.
Re: pg_restore crash when there is a failure before all child process is created
I have applied tested both patches separately and ran regression with both. No new test cases are failing with both patches. The issues is fixed by both patches however the fix from Tom looks more elegant. I haven't done a detailed code review. On Fri, Jan 31, 2020 at 12:39 AM Tom Lane wrote: > vignesh C writes: > > On Wed, Jan 29, 2020 at 6:54 PM Ahsan Hadi wrote: > >> Can you share a test case or steps that you are using to reproduce this > issue? Are you reproducing this using a debugger? > > > I could reproduce with the following steps: > > Make cluster setup. > > Create few tables. > > Take a dump in directory format using pg_dump. > > Restore the dump generated above using pg_restore with very high > > number for --jobs options around 600. > > I agree this is quite broken. Another way to observe the crash is > to make the fork() call randomly fail, as per booby-trap-fork.patch > below (not intended for commit, obviously). > > I don't especially like the proposed patch, though, as it introduces > a great deal of confusion into what ParallelState.numWorkers means. > I think it's better to leave that as being the allocated array size, > and instead clean up all the fuzzy thinking about whether workers > are actually running or not. Like 0001-fix-worker-status.patch below. > > regards, tom lane > > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: Append with naive multiplexing of FDWs
Hi Hackers, Sharing the email below from Movead Li, I believe he wanted to share the benchmarking results as a response to this email thread but it started a new thread.. Here it is... " Hello I have tested the patch with a partition table with several foreign partitions living on seperate data nodes. The initial testing was done with a partition table having 3 foreign partitions, test was done with variety of scale facters. The seonnd test was with fixed data per data node but number of data nodes were increased incrementally to see the peformance impact as more nodes are added to the cluster. The test three is similar to the initial test but with much huge data and 4 nodes. The results are summary is given below and test script attached: *Test ENV* Parent node:2Core 8G Child Nodes:2Core 4G *Test one:* 1.1 The partition struct as below: [ ptf:(a int, b int, c varchar)] (Parent node) | | | [ptf1] [ptf2] [ptf3] (Node1) (Node2)(Node3) The table data is partitioned across nodes, the test is done using a simple select query and a count aggregate as shown below. The result is an average of executing each query multiple times to ensure reliable and consistent results. ①select * from ptf where b = 100; ②select count(*) from ptf; 1.2. Test Results For ① result: scalepernodemasterpatched performance 2G7s 2s 350% 5G173s 63s 275% 10G 462s 156s 296% 20G 968s 327s 296% 30G 1472s 494s 297% For ② result: scalepernodemasterpatched performance 2G1079s 291s 370% 5G2688s 741s 362% 10G 4473s 1493s 299% It takes too long time to test a aggregate so the test was done with a smaller data size. 1.3. summary With the table partitioned over 3 nodes, the average performance gain across variety of scale factors is almost 300% *Test Two* 2.1 The partition struct as below: [ ptf:(a int, b int, c varchar)] (Parent node) | | | [ptf1] ... [ptfN] (Node1) (...)(NodeN) ①select * from ptf ②select * from ptf where b = 100; This test is done with same size of data per node but table is partitioned across N number of nodes. Each varation (master or patches) is tested at-least 3 times to get reliable and consistent results. The purpose of the test is to see impact on performance as number of data nodes are increased. 2.2 The results For ① result（scalepernode=2G）: nodenumber masterpatched performance 2 432s180s 240% 3 636s 223s 285% 4 830s 283s 293% 5 1065s 361s 295% For ② result（scalepernode=10G）: nodenumber masterpatched performance 2 281s140s 201% 3 421s140s 300% 4 562s141s 398% 5 702s141s 497% 6 833s139s 599% 7 986s141s 699% 8 1125s 140s 803% *Test Three* This test is similar to the [test one] but with much huge data and 4 nodes. For ① result: scalepernodemasterpatched performance 100G6592s 1649s 399% For ② result: scalepernodemasterpatched performance 100G35383 12363 286% The result show it work well in much huge data. *Summary* The patch is pretty good, it works well when there were little data back to the parent node. The patch doesn’t provide parallel FDW scan, it ensures that child nodes can send data to parent in parallel but the parent can only sequennly process the data from data nodes. Providing there is no performance degrdation for non FDW append queries, I would recomend to consider this patch as an interim soluton while we are waiting for parallel FDW scan. " On Thu, Dec 12, 2019 at 5:41 PM Kyotaro Horiguchi wrote: > Hello. > > I think I can say that this patch doesn't slows non-AsyncAppend, > non-postgres_fdw scans. > > > At Mon, 9 Dec 2019 12:18:44 -0500, Bruce Momjian wrote > in > > Certainly any overhead on normal queries would be unacceptable. > > I took performance numbers on the current shape of the async execution > patch for the following scan cases. > > t0 : single local table (parallel disabled) > pll : local partitioning (local Append, pa
Re: Extension development
Hi Yonatan, You can follow this blog for creating your own extension in PostgreSQL.. https://www.highgo.ca/2019/10/01/a-guide-to-create-user-defined-extension-modules-to-postgres/ -- Ahsan On Tue, Nov 12, 2019 at 11:54 AM Yonatan Misgan wrote: > I am developed my own PostgreSQL extension for learning purpose and it is > working correctly but I want to know to which components of the database is > my own extension components communicate. For example I have c code, make > file sql script, and control file after compiling the make file to which > components of the database are each of my extension components to > communicate. Thanks for your response. > > > > Regards, > > > > *Yonathan Misgan * > > *Assistant Lecturer, @ Debre Tabor University* > > *Faculty of Technology* > > *Department of Computer Science* > > *Studying MSc in **Computer Science** (in Data and Web Engineering) * > > *@ Addis Ababa University* > > *E-mail: yona...@dtu.edu.et * > > *yonathanmisga...@gmail.com * > > *Tel: **(+251)-911180185 (mob)* > > > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: Proposal for syntax to support creation of partition tables when creating parent table
On Wed, Sep 25, 2019 at 8:53 PM Tom Lane wrote: > Muhammad Usama writes: > > I want to propose an extension to CREATE TABLE syntax to allow the > creation > > of partition tables along with its parent table using a single statement. > > TBH, I think this isn't a particularly good idea. It seems very > reminiscent of the variant of CREATE SCHEMA that lets you create > a bunch of contained objects along with the schema. That variant > is a mess to support and AFAIK it's practically unused in the > real world. (If it were used, we'd get requests to support more > than the small number of object types that the CREATE SCHEMA > grammar currently allows.) > IMO creating auto-partitions shouldn't be viewed as creating bunch of schema objects with CREATE SCHEMA command. Most of the other RDBMS solutions support the table partition syntax where parent partition table is specified with partitions and sub-partitions in same SQL statement. As I understand the proposal is not changing the syntax of creating partitions, it is providing the ease of creating parent partition table along with its partitions in same statement. I think it does make it easier when you are creating a big partition table with lots of partitions and sub-partitions. The would also benefit users migrating to postgres from Oracle or mysql etc where similar syntax is supported. And if not more I think it is a tick in the box with minimal code change. > > As Fabien noted, there's been some related discussion about this > area, but nobody was advocating a solution of this particular shape. > The thread that Usama mentioned in his email is creating auto-partitions just for HASH partitions, this is trying to do similar for all types of partitions. > regards, tom lane > > > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: enhance SPI to support EXECUTE commands
I don't see much use for this because the documentation says that "server's execute command cannot be used directly within pl/pgsql function (and it is not needed). Within pl/pgsql you can execute update/delete commands using pl/pgsql EXECUTE command and get results like row_count using "get diagnostic". Why would somebody do what you have shown in your example in pl/pgsql? Or do you have a more general use-case for this enhancement? On Thu, Sep 5, 2019 at 11:39 AM Quan Zongliang < zongliang.q...@postgresdata.com> wrote: > Dear hackers, > > I found that such a statement would get 0 in PL/pgSQL. > > PREPARE smt_del(int) AS DELETE FROM t1; > EXECUTE 'EXECUTE smt_del(100)'; > GET DIAGNOSTICS j = ROW_COUNT; > > In fact, this is a problem with SPI, it does not support getting result > of the EXECUTE command. I made a little enhancement. Support for the > number of rows processed when executing INSERT/UPDATE/DELETE statements > dynamically. > > Regards, > Quan Zongliang > -- Highgo Software (Canada/China/Pakistan) URL : http://www.highgo.ca ADDR: 10318 WHALLEY BLVD, Surrey, BC EMAIL: mailto: ahsan.h...@highgo.ca
Re: patch: psql - enforce constant width of last column
On Tue, Sep 17, 2019 at 8:16 PM Pavel Stehule wrote: > > > út 17. 9. 2019 v 17:06 odesílatel Ahsan Hadi > napsal: > >> Hi Pavel, >> >> I have been trying to reproduce the case of badly displaying last columns >> of a query result-set. I played around with the legal values for psql >> border variable but not able to find a case where last columns are badly >> displayed. Can you please share an example that I can use to reproduce this >> problem. I will try out your patch once I am able to reproduce the problem. >> > > you need to use pspg, and vertical cursor. > > https://github.com/okbob/pspg > vertical cursor should be active > okay thanks for the info. I don't think it was possible to figure this out by reading the initial post. I will check it out. does this patch have any value for psql without pspg? > \pset border 1 > \pset linestyle ascii > \pset pager always > > select * from generate_series(1,3); > > Regards > > Pavel > > >> Thanks, >> >> -- Ahsan >> >> >> On Mon, Sep 9, 2019 at 2:32 PM Pavel Stehule >> wrote: >> >>> Hi >>> >>> When I played with vertical cursor support I got badly displayed last >>> columns when border was not 2. Only when border is 2, then psql displays >>> last column with same width for each row. >>> >>> I think so we can force column width alignment for any border styles >>> today (for alignment and wrapping styles) or as minimum this behave can be >>> optional. >>> >>> I wrote a patch with pset option "final_spaces", but I don't see a >>> reason why we trim rows today. >>> >>> Regards >>> >>> Pavel >>> >>
Re: patch: psql - enforce constant width of last column
Hi Pavel, I have been trying to reproduce the case of badly displaying last columns of a query result-set. I played around with the legal values for psql border variable but not able to find a case where last columns are badly displayed. Can you please share an example that I can use to reproduce this problem. I will try out your patch once I am able to reproduce the problem. Thanks, -- Ahsan On Mon, Sep 9, 2019 at 2:32 PM Pavel Stehule wrote: > Hi > > When I played with vertical cursor support I got badly displayed last > columns when border was not 2. Only when border is 2, then psql displays > last column with same width for each row. > > I think so we can force column width alignment for any border styles today > (for alignment and wrapping styles) or as minimum this behave can be > optional. > > I wrote a patch with pset option "final_spaces", but I don't see a reason > why we trim rows today. > > Regards > > Pavel >
Re: Email to hackers for test coverage
On Wed, Aug 28, 2019 at 9:43 PM Peter Eisentraut <
peter.eisentr...@2ndquadrant.com> wrote:
> On 2019-08-22 11:46, movead...@highgo.ca wrote:
> > *1. src/include/utils/float.h:140*
> >
> > Analyze:
> > This is an error report line when converting a big float8 value
> > which a float4 can not storage to float4.
> >
> > Test case:
> > Add a test case as below in file float4.sql:
> > select float4(1234567890123456789012345678901234567890::float8);
>
> > +-- Add test case for float4() type fonversion function
>
> Check spelling
>
> > *2. src/include/utils/float.h:145*
> >
> > Analyze:
> > This is an error report line when converting a small float8 value
> > which a float4 can not storage to float4.
> >
> > Test case:
> > Add a test case as below in file float4.sql:
> > select float4(0.01::float8);
> >
> > *3.src/include/utils/sortsupport.h:264*
> >
> > Analyze:
> > It is reverse sorting for the data type that has abbreviated for
> > sort, for example macaddr, uuid, numeric, network and I choose
> > numeric to do it.
> >
> > Test cast:
> > Add a test case as below in file numeric.sql:
> > INSERT INTO num_input_test(n1) values('99.998');
> > INSERT INTO num_input_test(n1) values('99.997');
> > SELECT * FROM num_input_test ORDER BY n1 DESC;
>
> > INSERT INTO num_input_test(n1) VALUES ('nan');
> > +INSERT INTO num_input_test(n1) values('99.998');
> > +INSERT INTO num_input_test(n1) values('99.997');
>
> Make spaces and capitalization match surrounding code.
>
> > Result and patch
> >
> > By adding the test cases, the test coverage of float.h increased from
> > 97.7% to 100% and sortsupport.h increased from 76.7% to 80.0%.
>
> That's fine, but I suggest that if you really want to make an impact in
> test coverage, look to increase function coverage rather than line
> coverage. Or look for files that are not covered at all.
>
>
+1
> --
> Peter Eisentraut http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>
>
>
Re: WIP/PoC for parallel backup
On Fri, 23 Aug 2019 at 10:26 PM, Stephen Frost wrote: > Greetings, > > * Asif Rehman (asifr.reh...@gmail.com) wrote: > > On Fri, Aug 23, 2019 at 3:18 PM Asim R P wrote: > > > 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 seems correct, however, your patch is also creating a new process > to > > > handle each set. Is that necessary? I think we should try to achieve > this > > > using multiple asynchronous libpq connections from a single basebackup > > > process. That is to use PQconnectStartParams() interface instead of > > > PQconnectdbParams(), wich is currently used by basebackup. On the > server > > > side, it may still result in multiple backend processes per > connection, and > > > an attempt should be made to avoid that as well, but it seems > complicated. > > > > Thanks Asim for the feedback. This is a good suggestion. The main idea I > > wanted to discuss is the design where we can open multiple backend > > connections to get the data instead of a single connection. > > On the client side we can have multiple approaches, One is to use > > asynchronous APIs ( as suggested by you) and other could be to decide > > between multi-process and multi-thread. The main point was we can extract > > lot of performance benefit by using the multiple connections and I built > > this POC to float the idea of how the parallel backup can work, since the > > core logic of getting the files using multiple connections will remain > the > > same, wether we use asynchronous, multi-process or multi-threaded. > > > > I am going to address the division of files to be distributed evenly > among > > multiple workers based on file sizes, that would allow to get some > concrete > > numbers as well as it will also us to gauge some benefits between async > and > > multiprocess/thread approach on client side. > > I would expect you to quickly want to support compression on the server > side, before the data is sent across the network, and possibly > encryption, and so it'd likely make sense to just have independent > processes and connections through which to do that. It would be interesting to see the benefits of compression (before the data is transferred over the network) on top of parallelism. Since there is also some overhead associated with performing the compression. I agree with your suggestion of trying to add parallelism first and then try compression before the data is sent across the network. > > Thanks, > > Stephen >
Re: Email to hackers for test coverage
The subject of the email may be bit misleading however this is really good
exercise to analyse the current code of Postgres regression suite using
GCOV and then adding test cases incrementally to increase the test
coverage.
On Thu, 22 Aug 2019 at 2:46 PM, movead...@highgo.ca
wrote:
> Hello hackers,
>
> One of the area that didn't get much attention in the community
> recently is analysing and increasing the code coverage of
> PostgreSQL regession test suite. I have started working on the
> code coverage by running the GCOV code coverage analysis tool in
> order to analyse the current code coverage and come up with test
> cases to increase the code coverage. This is going to be a long
> exercise so my plan is do it incrementaly. I will be analysing
> some area of untested code and then coming up with test cases to
> test those lines of code in regression and then moving on next
> area of untested code and so on.
>
> So far I have come up with 3 test cases to increase the code
> coverage of PostgreSQL regression test suite.
>
> I have performed the regression run for this exercise on this commit:
> (Commit version 75c1921cd6c868c5995b88113b4463a4830b9a27):
>
> The regression is executed with make check-world command and the
> results are gathered using 'make coverage-html' command.
>
> Below are the lines of untested code that i have analysed and the
> test cases added to regression to test these as part of regression.
>
> *1. src/include/utils/float.h:140*
>
> Analyze:
> This is an error report line when converting a big float8 value
> which a float4 can not storage to float4.
>
> Test case:
> Add a test case as below in file float4.sql:
> select float4(1234567890123456789012345678901234567890::float8);
>
> *2. src/include/utils/float.h:145*
>
> Analyze:
> This is an error report line when converting a small float8 value
> which a float4 can not storage to float4.
>
> Test case:
> Add a test case as below in file float4.sql:
> select float4(0.01::float8);
>
> *3.src/include/utils/sortsupport.h:264*
>
> Analyze:
> It is reverse sorting for the data type that has abbreviated for
> sort, for example macaddr, uuid, numeric, network and I choose
> numeric to do it.
>
> Test cast:
> Add a test case as below in file numeric.sql:
> INSERT INTO num_input_test(n1) values('99.998');
> INSERT INTO num_input_test(n1) values('99.997');
> SELECT * FROM num_input_test ORDER BY n1 DESC;
>
> Result and patch
>
> By adding the test cases, the test coverage of float.h increased from
> 97.7% to 100% and sortsupport.h increased from 76.7% to 80.0%.
>
> The increase in code coverage can be seen in the before and after
> pictures of GCOV test coverage analysis summary.
>
> The attached patch contain the test cases added in regression for
> increasing the coverage.
>
>
> --
> Movead Li
>
Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS)
I have shared a calendar invite for TDE/KMS weekly meeting with the members who expressed interest of joining the meeting in this chain. Hopefully I haven't missed anyone. I am not aware of everyone's timezone but I have tried to setup a time that's not very inconvenient. It won't be ideal for everyone as we are dealing with multiple timezone but do let me know It is too bizarre for you and I will try to find another slot. I will share a zoom link for the meeting on the invite in due course. -- Ahsan On Mon, Aug 19, 2019 at 9:26 AM Ahsan Hadi wrote: > > > On Mon, 19 Aug 2019 at 6:23 AM, Smith, Peter > wrote: > >> > From: Ibrar Ahmed Sent: Sunday, 18 August 2019 >> 2:43 AM >> > +1 for voice call, bruce we usually have a weekly TDE call. I will >> include you in >> >> If you don't mind, please also add me to that TDE call list. >> > > Sure will do. > > >> Thanks/Regards, >> --- >> Peter Smith >> Fujitsu Australia >> >
Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS)
On Mon, 19 Aug 2019 at 6:23 AM, Smith, Peter wrote: > > From: Ibrar Ahmed Sent: Sunday, 18 August 2019 > 2:43 AM > > +1 for voice call, bruce we usually have a weekly TDE call. I will > include you in > > If you don't mind, please also add me to that TDE call list. > Sure will do. > Thanks/Regards, > --- > Peter Smith > Fujitsu Australia >
Re: [Proposal] Table-level Transparent Data Encryption (TDE) and Key Management Service (KMS)
The current calendar entry for TDE weekly call will not work for EST timezone. I will change the invite so we can accommodate people from multiple time zones. Stay tuned. On Sun, 18 Aug 2019 at 2:29 AM, Sehrope Sarkuni wrote: > On Sat, Aug 17, 2019 at 12:43 PM Ibrar Ahmed > wrote: > >> +1 for voice call, bruce we usually have a weekly TDE call. >> > > Please add me to the call as well. Thanks! > > Regards, > -- Sehrope Sarkuni > Founder & CEO | JackDB, Inc. | https://www.jackdb.com/ > >
Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack
Hi Michael, Can you rebase the reindex-priv-93.patch, it is getting some hunk failures. patching file doc/src/sgml/ref/reindex.sgml Hunk #1 succeeded at 227 (offset 13 lines). patching file src/backend/commands/indexcmds.c Hunk #1 FAILED at 1873. 1 out of 1 hunk FAILED -- saving rejects to file src/backend/commands/indexcmds.c.rej Thanks.
