Re: Support worker_spi to execute the function dynamically.

On Fri, Jul 28, 2023 at 1:26 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Fri, Jul 28, 2023 at 10:47:39AM +0530, Bharath Rupireddy wrote:
> > +# check their existence.  Use IDs that do not overlap with the schemas created
> > +# by the previous workers.
> >
> > While using different IDs in tests is a simple fix, -1 for it. I'd
> > prefer if worker_spi uses different schema prefixes for static and
> > dynamic bg workers to avoid conflicts. We can either look at
> > MyBgworkerEntry->bgw_type in worker_spi_main and have schema name as
> > '{static, dyamic}_worker_schema_%d', id or pass schema name in
> > bgw_extra.
>
> For the sake of a test module, I am not really convinced that there is
> any need to go down to such complexity with the names of the schemas
> created.

I don't think something like [1] is complex. It makes worker_spi
foolproof. Rather, the other approach proposed, that is to provide
non-conflicting worker IDs to worker_spi_launch in the TAP test file,
looks complicated to me. And it's easy for someone to come, add a test
case with conflicting IDs input to worker_spi_launch and end up in the
same state that we're in now.

[1]
diff --git a/src/test/modules/worker_spi/t/001_worker_spi.pl
b/src/test/modules/worker_spi/t/001_worker_spi.pl
index c293871313..700530afc7 100644
--- a/src/test/modules/worker_spi/t/001_worker_spi.pl
+++ b/src/test/modules/worker_spi/t/001_worker_spi.pl
@@ -27,16 +27,16 @@ is($result, 't', "dynamic bgworker launched");
 $node->poll_query_until(
        'postgres',
        qq[SELECT count(*) > 0 FROM information_schema.tables
-           WHERE table_schema = 'schema4' AND table_name = 'counted';]);
+           WHERE table_schema = 'dynamic_worker_schema4' AND
table_name = 'counted';]);
 $node->safe_psql('postgres',
-       "INSERT INTO schema4.counted VALUES ('total', 0), ('delta', 1);");
+       "INSERT INTO dynamic_worker_schema4.counted VALUES ('total',
0), ('delta', 1);");
 # Issue a SIGHUP on the node to force the worker to loop once, accelerating
 # this test.
 $node->reload;
 # Wait until the worker has processed the tuple that has just been inserted.
 $node->poll_query_until('postgres',
-       qq[SELECT count(*) FROM schema4.counted WHERE type = 'delta';], '0');
-$result = $node->safe_psql('postgres', 'SELECT * FROM schema4.counted;');
+       qq[SELECT count(*) FROM dynamic_worker_schema4.counted WHERE
type = 'delta';], '0');
+$result = $node->safe_psql('postgres', 'SELECT * FROM
dynamic_worker_schema4.counted;');
 is($result, qq(total|1), 'dynamic bgworker correctly consumed tuple data');

 note "testing bgworkers loaded with shared_preload_libraries";
diff --git a/src/test/modules/worker_spi/worker_spi.c
b/src/test/modules/worker_spi/worker_spi.c
index 903dcddef9..02b4204aa2 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -135,10 +135,19 @@ worker_spi_main(Datum main_arg)
        int                     index = DatumGetInt32(main_arg);
        worktable  *table;
        StringInfoData buf;
-       char            name[20];
+       char            name[NAMEDATALEN];

        table = palloc(sizeof(worktable));
-       sprintf(name, "schema%d", index);
+
+       /*
+        * Use different schema names for static and dynamic bg workers to avoid
+        * name conflicts.
+        */
+       if (strcmp(MyBgworkerEntry->bgw_type, "worker_spi") == 0)
+               sprintf(name, "worker_schema%d", index);
+       else if (strcmp(MyBgworkerEntry->bgw_type, "worker_spi dynamic") == 0)
+               sprintf(name, "dynamic_worker_schema%d", index);
+
        table->schema = pstrdup(name);
        table->name = pstrdup("counted");

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com