public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 1/3] bootstrap: convert Typ to a List*
3+ messages / 3 participants
[nested] [flat]

* [PATCH 1/3] bootstrap: convert Typ to a List*
@ 2020-11-20 02:48  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Justin Pryzby @ 2020-11-20 02:48 UTC (permalink / raw)

---
 src/backend/bootstrap/bootstrap.c | 69 ++++++++++++++-----------------
 1 file changed, 31 insertions(+), 38 deletions(-)

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 6f615e6622..18eb62ca47 100644
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -159,7 +159,7 @@ struct typmap
 	FormData_pg_type am_typ;
 };
 
-static struct typmap **Typ = NULL;
+static List *Typ = NIL; /* List of struct typmap* */
 static struct typmap *Ap = NULL;
 
 static Datum values[MAXATTR];	/* current row's attribute values */
@@ -597,7 +597,7 @@ boot_openrel(char *relname)
 	 * pg_type must be filled before any OPEN command is executed, hence we
 	 * can now populate the Typ array if we haven't yet.
 	 */
-	if (Typ == NULL)
+	if (Typ == NIL)
 		populate_typ_array();
 
 	if (boot_reldesc != NULL)
@@ -688,7 +688,7 @@ DefineAttr(char *name, char *type, int attnum, int nullness)
 
 	typeoid = gettype(type);
 
-	if (Typ != NULL)
+	if (Typ != NIL)
 	{
 		attrtypes[attnum]->atttypid = Ap->am_oid;
 		attrtypes[attnum]->attlen = Ap->am_typ.typlen;
@@ -877,36 +877,25 @@ populate_typ_array(void)
 	Relation	rel;
 	TableScanDesc scan;
 	HeapTuple	tup;
-	int			nalloc;
-	int			i;
-
-	Assert(Typ == NULL);
 
-	nalloc = 512;
-	Typ = (struct typmap **)
-		MemoryContextAlloc(TopMemoryContext, nalloc * sizeof(struct typmap *));
+	Assert(Typ == NIL);
 
 	rel = table_open(TypeRelationId, NoLock);
 	scan = table_beginscan_catalog(rel, 0, NULL);
-	i = 0;
 	while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
 	{
 		Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup);
+		struct typmap *newtyp;
+		MemoryContext old;
 
-		/* make sure there will be room for a trailing NULL pointer */
-		if (i >= nalloc - 1)
-		{
-			nalloc *= 2;
-			Typ = (struct typmap **)
-				repalloc(Typ, nalloc * sizeof(struct typmap *));
-		}
-		Typ[i] = (struct typmap *)
-			MemoryContextAlloc(TopMemoryContext, sizeof(struct typmap));
-		Typ[i]->am_oid = typForm->oid;
-		memcpy(&(Typ[i]->am_typ), typForm, sizeof(Typ[i]->am_typ));
-		i++;
+		old = MemoryContextSwitchTo(TopMemoryContext);
+		newtyp = (struct typmap *) palloc(sizeof(struct typmap));
+		Typ = lappend(Typ, newtyp);
+		MemoryContextSwitchTo(old);
+
+		newtyp->am_oid = typForm->oid;
+		memcpy(&newtyp->am_typ, typForm, sizeof(newtyp->am_typ));
 	}
-	Typ[i] = NULL;				/* Fill trailing NULL pointer */
 	table_endscan(scan);
 	table_close(rel, NoLock);
 }
@@ -925,16 +914,17 @@ populate_typ_array(void)
 static Oid
 gettype(char *type)
 {
-	if (Typ != NULL)
+	if (Typ != NIL)
 	{
-		struct typmap **app;
+		ListCell *lc;
 
-		for (app = Typ; *app != NULL; app++)
+		foreach (lc, Typ)
 		{
-			if (strncmp(NameStr((*app)->am_typ.typname), type, NAMEDATALEN) == 0)
+			struct typmap *app = lfirst(lc);
+			if (strncmp(NameStr(app->am_typ.typname), type, NAMEDATALEN) == 0)
 			{
-				Ap = *app;
-				return (*app)->am_oid;
+				Ap = app;
+				return app->am_oid;
 			}
 		}
 	}
@@ -980,14 +970,17 @@ boot_get_type_io_data(Oid typid,
 	if (Typ != NULL)
 	{
 		/* We have the boot-time contents of pg_type, so use it */
-		struct typmap **app;
-		struct typmap *ap;
-
-		app = Typ;
-		while (*app && (*app)->am_oid != typid)
-			++app;
-		ap = *app;
-		if (ap == NULL)
+		struct typmap *ap = NULL;
+		ListCell *lc;
+
+		foreach (lc, Typ)
+		{
+			ap = lfirst(lc);
+			if (ap->am_oid == typid)
+				break;
+		}
+
+		if (!ap || ap->am_oid != typid)
 			elog(ERROR, "type OID %u not found in Typ list", typid);
 
 		*typlen = ap->am_typ.typlen;
-- 
2.26.2


--------------1F4A19FEB5D90C72707AFA7B
Content-Type: text/x-patch; charset=UTF-8;
 name="0002-Allow-composite-types-in-bootstrap-20210122.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename="0002-Allow-composite-types-in-bootstrap-20210122.patch"



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: IPC::Run accepts bug reports
@ 2024-10-04 11:00  Alexander Lakhin <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Alexander Lakhin @ 2024-10-04 11:00 UTC (permalink / raw)
  To: Noah Misch <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>

Hello Noah,

16.06.2024 02:48, Noah Misch wrote:
> I don't see in https://github.com/cpan-authors/IPC-Run/issues anything
> affecting PostgreSQL.  If you know of IPC::Run defects, please report them.
> If I knew of an IPC::Run defect affecting PostgreSQL, I likely would work on
> it before absurdity like https://github.com/cpan-authors/IPC-Run/issues/175
> NetBSD-10-specific behavior coping.

It looks like a recent indri failure [1] revealed one more IPC::Run
anomaly. The failure log contains:
# Running: pgbench -n -t 1 -Dfoo=bla -Dnull=null -Dtrue=true -Done=1 -Dzero=0.0 -Dbadtrue=trueXXX 
-Dmaxint=9223372036854775807 -Dminint=-9223372036854775808 -M prepared -f 
/Users/buildfarm/bf-data/HEAD/pgsql.build/src/bin/pgbench/tmp_check/t_001_pgbench_with_server_main_data/001_pgbench_error_sleep_undefined_variable
[22:38:14.887](0.014s) ok 362 - pgbench script error: sleep undefined variable status (got 2 vs expected 2)
[22:38:14.887](0.000s) ok 363 - pgbench script error: sleep undefined variable stdout /(?^:processed: 0/1)/
[22:38:14.887](0.000s) not ok 364 - pgbench script error: sleep undefined variable stderr /(?^:sleep: undefined variable)/
[22:38:14.887](0.000s)
[22:38:14.887](0.000s) #   Failed test 'pgbench script error: sleep undefined variable stderr /(?^:sleep: undefined 
variable)/'
#   at t/001_pgbench_with_server.pl line 1242.
[22:38:14.887](0.000s) #                   ''
#     doesn't match '(?^:sleep: undefined variable)'

So the pgbench process exited as expected, stdout contained expected
string, but stderr happened to be empty.

Maybe such behavior is specific to macOS, and even on indri it's the
only failure of that ilk out of 2000+ runs (and I couldn't reproduce
this in a Linux VM), but I find this place in IPC::Run suspicious:
sub _read {
...
     my $r = POSIX::read( $_[0], $s, 10_000 );
     croak "$!: read( $_[0] )" if not($r) and !$!{EINTR};

That is, EINTR kind of recognized as an expected error, but there is no
retry in this case. Thus, with the following modification, which simulates
read() failed with EINTR:
  sub _read {
      confess 'undef' unless defined $_[0];
      my $s = '';
-    my $r = POSIX::read( $_[0], $s, 10_000 );
+    my $r;
+if (int(rand(100)) == 0)
+{
+   $r = 0;  $! = Errno::EINTR;
+}
+else
+{
+    $r = POSIX::read( $_[0], $s, 10_000 );
+}
      croak "$!: read( $_[0] )" if not($r) and !$!{EINTR};

I can see failures like the one in question when running that test.

Perhaps, I could reproduce the issue with a program, that sends signals
to running (pgbench) processes (and thus interrupts read()), if it makes
sense.

What do you think?

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=indri&dt=2024-10-02%2002%3A34%3A16

Best regards,
Alexander






^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: IPC::Run accepts bug reports
@ 2024-10-04 18:57  Noah Misch <[email protected]>
  parent: Alexander Lakhin <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Noah Misch @ 2024-10-04 18:57 UTC (permalink / raw)
  To: Alexander Lakhin <[email protected]>; +Cc: pgsql-hackers; Robert Haas <[email protected]>

On Fri, Oct 04, 2024 at 02:00:00PM +0300, Alexander Lakhin wrote:
> sub _read {
> ...
>     my $r = POSIX::read( $_[0], $s, 10_000 );
>     croak "$!: read( $_[0] )" if not($r) and !$!{EINTR};
> 
> That is, EINTR kind of recognized as an expected error, but there is no
> retry in this case. Thus, with the following modification, which simulates
> read() failed with EINTR:
>  sub _read {
>      confess 'undef' unless defined $_[0];
>      my $s = '';
> -    my $r = POSIX::read( $_[0], $s, 10_000 );
> +    my $r;
> +if (int(rand(100)) == 0)
> +{
> +   $r = 0;  $! = Errno::EINTR;
> +}
> +else
> +{
> +    $r = POSIX::read( $_[0], $s, 10_000 );
> +}
>      croak "$!: read( $_[0] )" if not($r) and !$!{EINTR};
> 
> I can see failures like the one in question when running that test.

That makes sense.  Would you file this at
https://github.com/cpan-authors/IPC-Run/issues?  I suppose that code should
become roughly:

  do { $r = POSIX::read(...) } while (!defined($r) && $!{EINTR});
  croak ... unless defined($r);

> Perhaps, I could reproduce the issue with a program, that sends signals
> to running (pgbench) processes (and thus interrupts read()), if it makes
> sense.

That should work.  Best would be an IPC::Run test case, like the existing
t/eintr.t that makes select() report EINTR.  That said, IPC::Run could adopt
the retry without a test.

> [1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=indri&dt=2024-10-02%2002%3A34%3A16






^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2024-10-04 18:57 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 02:48 [PATCH 1/3] bootstrap: convert Typ to a List* Justin Pryzby <[email protected]>
2024-10-04 11:00 Re: IPC::Run accepts bug reports Alexander Lakhin <[email protected]>
2024-10-04 18:57 ` Re: IPC::Run accepts bug reports Noah Misch <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox