public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 17/25] another way to install uri_regress/testclient 5+ messages / 4 participants [nested] [flat]
* [PATCH 17/25] another way to install uri_regress/testclient @ 2022-05-27 03:24 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Justin Pryzby @ 2022-05-27 03:24 UTC (permalink / raw) This reverts commit a17fd67d2f2861ae0ce00d1aeefdf2facc47cd5e. TODO: partially revert 6b04abdfc5e0653542ac5d586e639185a8c61a39 See also: a17fd67d2f2861ae0ce00d1aeefdf2facc47cd5e https://www.postgresql.org/message-id/20220416144454.GX26620%40telsasoft.com https://www.postgresql.org/message-id/20220223211606.ikcwoolsz2nohsw6%40alap3.anarazel.de TODO: doc/src/sgml/extend.sgml ci-os-only: linux, windows --- src/interfaces/libpq/test/Makefile | 8 +++---- src/tools/msvc/Mkvcbuild.pm | 36 +++++++++++++----------------- 2 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/interfaces/libpq/test/Makefile b/src/interfaces/libpq/test/Makefile index 75ac08f943d..91c8d87d528 100644 --- a/src/interfaces/libpq/test/Makefile +++ b/src/interfaces/libpq/test/Makefile @@ -14,11 +14,11 @@ endif override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) LDFLAGS_INTERNAL += $(libpq_pgport) -PROGS = libpq_testclient libpq_uri_regress +PROGRAMS = libpq_testclient libpq_uri_regress -all: $(PROGS) +all: $(PROGRAMS) -$(PROGS): $(WIN32RES) +$(PROGRAMS): $(WIN32RES) clean distclean maintainer-clean: - rm -f $(PROGS) *.o + rm -f $(PROGRAMS) *.o diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm index c3cef625ad3..f327e631cdd 100644 --- a/src/tools/msvc/Mkvcbuild.pm +++ b/src/tools/msvc/Mkvcbuild.pm @@ -36,11 +36,14 @@ my @unlink_on_exit; # Set of variables for modules in contrib/ and src/test/modules/ my @contrib_uselibpq = (); -my @contrib_uselibpgport = (); +my @contrib_uselibpgport = ('libpq_uri_regress', 'libpq_testclient'); my @contrib_uselibpgcommon = (); my $contrib_extralibs = { 'libpq_pipeline' => ['ws2_32.lib'] }; my $contrib_extraincludes = {}; -my $contrib_extrasource = {}; +my $contrib_extrasource = { + 'libpq_uri_regress' => ['src/interfaces/libpq/test/libpq_uri_regress.c'], + 'libpq_testclient' => ['src/interfaces/libpq/test/libpq_testclient.c'], +}; my @contrib_excludes = ( 'bool_plperl', 'commit_ts', 'hstore_plperl', 'hstore_plpython', @@ -289,24 +292,6 @@ sub mkvcbuild $libpqwalreceiver->AddIncludeDir('src/interfaces/libpq'); $libpqwalreceiver->AddReference($postgres, $libpq); - my $libpq_testclient = - $solution->AddProject('libpq_testclient', 'exe', 'misc', - 'src/interfaces/libpq/test'); - $libpq_testclient->AddFile( - 'src/interfaces/libpq/test/libpq_testclient.c'); - $libpq_testclient->AddIncludeDir('src/interfaces/libpq'); - $libpq_testclient->AddReference($libpgport, $libpq); - $libpq_testclient->AddLibrary('ws2_32.lib'); - - my $libpq_uri_regress = - $solution->AddProject('libpq_uri_regress', 'exe', 'misc', - 'src/interfaces/libpq/test'); - $libpq_uri_regress->AddFile( - 'src/interfaces/libpq/test/libpq_uri_regress.c'); - $libpq_uri_regress->AddIncludeDir('src/interfaces/libpq'); - $libpq_uri_regress->AddReference($libpgport, $libpq); - $libpq_uri_regress->AddLibrary('ws2_32.lib'); - my $pgoutput = $solution->AddProject('pgoutput', 'dll', '', 'src/backend/replication/pgoutput'); $pgoutput->AddReference($postgres); @@ -477,7 +462,7 @@ sub mkvcbuild push @contrib_excludes, 'uuid-ossp'; } - foreach my $subdir ('contrib', 'src/test/modules') + foreach my $subdir ('contrib', 'src/test/modules', 'src/interfaces/libpq') { opendir($D, $subdir) || croak "Could not opendir on $subdir!\n"; while (my $d = readdir($D)) @@ -988,6 +973,15 @@ sub AddContrib AdjustContribProj($proj); push @projects, $proj; } + elsif ($mf =~ /^PROGRAMS\s*=\s*(.*)$/mg) + { + foreach my $proj (split /\s+/, $1) + { + my $p = $solution->AddProject($proj, 'exe', 'contrib', "$subdir/$n"); + AdjustContribProj($p); + push @projects, $p; + } + } else { croak "Could not determine contrib module type for $n\n"; -- 2.17.1 --IS0zKkzwUGydFO0o Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0018-Move-libpq_pipeline-test-into-src-interfaces-libpq.patch" ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: subscription/015_stream sometimes breaks @ 2023-08-24 07:50 Alvaro Herrera <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Alvaro Herrera @ 2023-08-24 07:50 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: Zhijie Hou (Fujitsu) <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers On 2023-Aug-24, Amit Kapila wrote: > On Wed, Aug 23, 2023 at 1:31 PM Alvaro Herrera <[email protected]> wrote: > > Hmm, I think if worker->in_use is false, we shouldn't consult the rest > > of the struct at all, so I propose to add the attached 0001 as a minimal > > fix. > > I think that way we may need to add the check for in_use before > accessing each of the LogicalRepWorker struct fields or form some rule > about which fields (or places) are okay to access without checking > in_use field. As far as I realize, we have that rule already. It's only a few relatively new places that have broken it. I understand that the in_use concept comes from the one of the same name in ReplicationSlot, except that it is not at all documented in worker_internal.h. So I propose we do both: apply Zhijie's patch and my 0001 now; and somebody gets to document the locking design for LogicalRepWorker. > > In fact, I'd go further and propose that if we do take that stance, then > > we don't need clear out the contents of this struct at all, so let's > > not. That's 0002. > > Personally, I think we should consider this change (0002 and 0002) separately. I agree. I'd maybe even retract them. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: subscription/015_stream sometimes breaks @ 2023-08-24 10:18 Amit Kapila <[email protected]> parent: Alvaro Herrera <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Amit Kapila @ 2023-08-24 10:18 UTC (permalink / raw) To: Alvaro Herrera <[email protected]>; +Cc: Zhijie Hou (Fujitsu) <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers On Thu, Aug 24, 2023 at 1:20 PM Alvaro Herrera <[email protected]> wrote: > > On 2023-Aug-24, Amit Kapila wrote: > > > On Wed, Aug 23, 2023 at 1:31 PM Alvaro Herrera <[email protected]> wrote: > > > > Hmm, I think if worker->in_use is false, we shouldn't consult the rest > > > of the struct at all, so I propose to add the attached 0001 as a minimal > > > fix. > > > > I think that way we may need to add the check for in_use before > > accessing each of the LogicalRepWorker struct fields or form some rule > > about which fields (or places) are okay to access without checking > > in_use field. > > As far as I realize, we have that rule already. It's only a few > relatively new places that have broken it. I understand that the in_use > concept comes from the one of the same name in ReplicationSlot, except > that it is not at all documented in worker_internal.h. > > So I propose we do both: apply Zhijie's patch and my 0001 now; and > somebody gets to document the locking design for LogicalRepWorker. > Agreed. -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: subscription/015_stream sometimes breaks @ 2023-08-25 03:39 Peter Smith <[email protected]> parent: Amit Kapila <[email protected]> 0 siblings, 1 reply; 5+ messages in thread From: Peter Smith @ 2023-08-25 03:39 UTC (permalink / raw) To: Amit Kapila <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Zhijie Hou (Fujitsu) <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers On Thu, Aug 24, 2023 at 8:18 PM Amit Kapila <[email protected]> wrote: > > On Thu, Aug 24, 2023 at 1:20 PM Alvaro Herrera <[email protected]> wrote: > > > > On 2023-Aug-24, Amit Kapila wrote: > > > > > On Wed, Aug 23, 2023 at 1:31 PM Alvaro Herrera <[email protected]> wrote: > > > > > > Hmm, I think if worker->in_use is false, we shouldn't consult the rest > > > > of the struct at all, so I propose to add the attached 0001 as a minimal > > > > fix. > > > > > > I think that way we may need to add the check for in_use before > > > accessing each of the LogicalRepWorker struct fields or form some rule > > > about which fields (or places) are okay to access without checking > > > in_use field. > > > > As far as I realize, we have that rule already. It's only a few > > relatively new places that have broken it. I understand that the in_use > > concept comes from the one of the same name in ReplicationSlot, except > > that it is not at all documented in worker_internal.h. > > > > So I propose we do both: apply Zhijie's patch and my 0001 now; and > > somebody gets to document the locking design for LogicalRepWorker. > > > > Agreed. Both of these patches (Hou-san's expedient resetting of the worker type, Alvaro's 0001 putting the 'in_use' check within the isXXXWorker type macros) appear to be blending the concept of "type" with whether the worker is "alive" or not, which I am not sure is a good thing. IMO the type is the type forever, so I felt type should get assigned only once when the worker is "born". For example, a dead horse is still a horse. ------ Kind Regards, Peter Smith. Fujitsu Australia ^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: subscription/015_stream sometimes breaks @ 2023-08-25 03:55 Amit Kapila <[email protected]> parent: Peter Smith <[email protected]> 0 siblings, 0 replies; 5+ messages in thread From: Amit Kapila @ 2023-08-25 03:55 UTC (permalink / raw) To: Peter Smith <[email protected]>; +Cc: Alvaro Herrera <[email protected]>; Zhijie Hou (Fujitsu) <[email protected]>; Thomas Munro <[email protected]>; pgsql-hackers On Fri, Aug 25, 2023 at 9:09 AM Peter Smith <[email protected]> wrote: > > On Thu, Aug 24, 2023 at 8:18 PM Amit Kapila <[email protected]> wrote: > > > > On Thu, Aug 24, 2023 at 1:20 PM Alvaro Herrera <[email protected]> wrote: > > > > > > On 2023-Aug-24, Amit Kapila wrote: > > > > > > > On Wed, Aug 23, 2023 at 1:31 PM Alvaro Herrera <[email protected]> wrote: > > > > > > > > Hmm, I think if worker->in_use is false, we shouldn't consult the rest > > > > > of the struct at all, so I propose to add the attached 0001 as a minimal > > > > > fix. > > > > > > > > I think that way we may need to add the check for in_use before > > > > accessing each of the LogicalRepWorker struct fields or form some rule > > > > about which fields (or places) are okay to access without checking > > > > in_use field. > > > > > > As far as I realize, we have that rule already. It's only a few > > > relatively new places that have broken it. I understand that the in_use > > > concept comes from the one of the same name in ReplicationSlot, except > > > that it is not at all documented in worker_internal.h. > > > > > > So I propose we do both: apply Zhijie's patch and my 0001 now; and > > > somebody gets to document the locking design for LogicalRepWorker. > > > > > > > Agreed. > > Both of these patches (Hou-san's expedient resetting of the worker > type, Alvaro's 0001 putting the 'in_use' check within the isXXXWorker > type macros) appear to be blending the concept of "type" with whether > the worker is "alive" or not, which I am not sure is a good thing. IMO > the type is the type forever, so I felt type should get assigned only > once when the worker is "born". For example, a dead horse is still a > horse. > I think it is important to have a alive check before accessing the worker type as we are doing for some of the other fields. For example, see the usage of in_use flag in the function logicalrep_worker_find(). The usage of parallel apply workers doesn't consider the use of in_use flag where as other worker types would first check in_use flag. -- With Regards, Amit Kapila. ^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2023-08-25 03:55 UTC | newest] Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-05-27 03:24 [PATCH 17/25] another way to install uri_regress/testclient Justin Pryzby <[email protected]> 2023-08-24 07:50 Re: subscription/015_stream sometimes breaks Alvaro Herrera <[email protected]> 2023-08-24 10:18 ` Re: subscription/015_stream sometimes breaks Amit Kapila <[email protected]> 2023-08-25 03:39 ` Re: subscription/015_stream sometimes breaks Peter Smith <[email protected]> 2023-08-25 03:55 ` Re: subscription/015_stream sometimes breaks Amit Kapila <[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