Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Alvaro Herrera]

Andres Freund wrote:
> On 2016-11-12 11:30:42 -0500, Tom Lane wrote:
> > Andres Freund  writes:
> > > Committed after simplifying the Makefile.
> > 
> > While I have no particular objection to adding these tests, the
> > commit log's claim that this will improve buildfarm testing is
> > quite wrong.  The buildfarm runs "make installcheck" not
> > "make check" in contrib.
> 
> Gah.  It was more aimed at the coverage stuff, but that might work the
> same. Alvaro?

Already replied on IM, but for the record, coverage.postgresql.org
currently runs this:

make -j4 >> $LOG 2>&1
make check-world >> $LOG 2>&1
make -C src/test/ssl check >> $LOG 2>&1

-- 
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Andres Freund]

On 2016-11-14 12:14:10 -0800, Andres Freund wrote:
> On 2016-11-12 11:42:12 -0500, Tom Lane wrote:
> > Andres Freund  writes:
> > > On 2016-11-12 11:30:42 -0500, Tom Lane wrote:
> > >> which is a rather blatant waste of cycles. I would suggest an explicit
> > >> do-nothing installcheck rule rather than the hack you came up with here.
> >
> > > I had that at first, but that generates a warning about overwriting the
> > > makefile target - which afaics cannot be fixed.
> >
> > Hm.  What about inventing an additional macro NO_INSTALLCHECK that
> > prevents pgxs.mk from generating an installcheck rule?  It's not
> > like we don't have similar issues elsewhere, eg contrib/sepgsql.
>
> Well, that one seems a bit different.  Seems easy enough to do. Do we
> want to make that macro that prevents installcheck from being defined,
> or one that forces it to be empty? The former has the disadvantage that
> one has to be careful to define a target, to avoid breaking recursion
> from the upper levels.

Oh, that disadvantage doesn't actually exist, because installcheck is a
.PHONY target...

I've for now not added a variant that prevents plain 'make check' from
doing anything. I guess it could be useful when a contrib module wants
to run tests via something else than pg_regress?

Patch attached.

Andres
>From 906051a23831c5d337556e56d19962f2e857 Mon Sep 17 00:00:00 2001
From: Andres Freund 
Date: Mon, 14 Nov 2016 12:29:30 -0800
Subject: [PATCH] Provide NO_INSTALLCHECK for pgxs.

This allows us to avoid running the regression tests in contrib modules
like pg_stat_statement in a less ugly manner.

Discussion: <22432.1478968...@sss.pgh.pa.us>
---
 contrib/pg_stat_statements/Makefile | 7 +++
 doc/src/sgml/extend.sgml| 9 +
 src/makefiles/pgxs.mk   | 4 
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/contrib/pg_stat_statements/Makefile b/contrib/pg_stat_statements/Makefile
index f1a45eb..298951a 100644
--- a/contrib/pg_stat_statements/Makefile
+++ b/contrib/pg_stat_statements/Makefile
@@ -13,6 +13,9 @@ LDFLAGS_SL += $(filter -lm, $(LIBS))
 
 REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
 REGRESS = pg_stat_statements
+# Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
+# which typical installcheck users do not have (e.g. buildfarm clients).
+NO_INSTALLCHECK = 1
 
 ifdef USE_PGXS
 PG_CONFIG = pg_config
@@ -24,7 +27,3 @@ top_builddir = ../..
 include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif
-
-# Disabled because these tests require "shared_preload_libraries=pg_stat_statements",
-# which typical installcheck users do not have (e.g. buildfarm clients).
-installcheck: REGRESS=
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index e19c657..f9d91a3 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -1194,6 +1194,15 @@ include $(PGXS)
  
 
  
+  NO_INSTALLCHECK
+  
+   
+don't define an installcheck target, useful e.g. if tests require special configuration, or don't use pg_regress
+   
+  
+ 
+
+ 
   EXTRA_CLEAN
   

diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk
index 2b4d684..c27004e 100644
--- a/src/makefiles/pgxs.mk
+++ b/src/makefiles/pgxs.mk
@@ -40,6 +40,8 @@
 # which need to be built first
 #   REGRESS -- list of regression test cases (without suffix)
 #   REGRESS_OPTS -- additional switches to pass to pg_regress
+#   NO_INSTALLCHECK -- don't define an installcheck target, useful e.g. if
+# tests require special configuration, or don't use pg_regress
 #   EXTRA_CLEAN -- extra files to remove in 'make clean'
 #   PG_CPPFLAGS -- will be added to CPPFLAGS
 #   PG_LIBS -- will be added to PROGRAM link line
@@ -268,8 +270,10 @@ ifndef PGXS
 endif
 
 # against installed postmaster
+ifndef NO_INSTALLCHECK
 installcheck: submake $(REGRESS_PREP)
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)
+endif
 
 ifdef PGXS
 check:
-- 
2.10.2


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Andres Freund]

On 2016-11-12 11:42:12 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2016-11-12 11:30:42 -0500, Tom Lane wrote:
> >> which is a rather blatant waste of cycles. I would suggest an explicit
> >> do-nothing installcheck rule rather than the hack you came up with here.
> 
> > I had that at first, but that generates a warning about overwriting the
> > makefile target - which afaics cannot be fixed.
> 
> Hm.  What about inventing an additional macro NO_INSTALLCHECK that
> prevents pgxs.mk from generating an installcheck rule?  It's not
> like we don't have similar issues elsewhere, eg contrib/sepgsql.

Well, that one seems a bit different.  Seems easy enough to do. Do we
want to make that macro that prevents installcheck from being defined,
or one that forces it to be empty? The former has the disadvantage that
one has to be careful to define a target, to avoid breaking recursion
from the upper levels.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Tom Lane]

Andres Freund  writes:
> On 2016-11-12 11:42:12 -0500, Tom Lane wrote:
>> Hm.  What about inventing an additional macro NO_INSTALLCHECK that
>> prevents pgxs.mk from generating an installcheck rule?

> That'd work. I'd also be ok with living with the warning.  I have to say
> I find it hard to be concerned about the cycles here. It's not like
> anybody complained about make check unconditionally creating a test
> installation, even if there's tests in a contrib module...

[ shrug... ]  The reason why the buildfarm doesn't do "make check" here is
precisely the extra cycles it would take.  Those add up over hundreds of
runs, particularly on slow machines.  So I'm not excited about running
completely useless operations.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Andres Freund]

On 2016-11-12 11:42:12 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > On 2016-11-12 11:30:42 -0500, Tom Lane wrote:
> >> which is a rather blatant waste of cycles. I would suggest an explicit
> >> do-nothing installcheck rule rather than the hack you came up with here.
> 
> > I had that at first, but that generates a warning about overwriting the
> > makefile target - which afaics cannot be fixed.
> 
> Hm.  What about inventing an additional macro NO_INSTALLCHECK that
> prevents pgxs.mk from generating an installcheck rule?

That'd work. I'd also be ok with living with the warning.  I have to say
I find it hard to be concerned about the cycles here. It's not like
anybody complained about make check unconditionally creating a test
installation, even if there's tests in a contrib module...

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Tom Lane]

Andres Freund  writes:
> On 2016-11-12 11:30:42 -0500, Tom Lane wrote:
>> which is a rather blatant waste of cycles. I would suggest an explicit
>> do-nothing installcheck rule rather than the hack you came up with here.

> I had that at first, but that generates a warning about overwriting the
> makefile target - which afaics cannot be fixed.

Hm.  What about inventing an additional macro NO_INSTALLCHECK that
prevents pgxs.mk from generating an installcheck rule?  It's not
like we don't have similar issues elsewhere, eg contrib/sepgsql.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Andres Freund]

On 2016-11-12 11:30:42 -0500, Tom Lane wrote:
> Andres Freund  writes:
> > Committed after simplifying the Makefile.
> 
> While I have no particular objection to adding these tests, the
> commit log's claim that this will improve buildfarm testing is
> quite wrong.  The buildfarm runs "make installcheck" not
> "make check" in contrib.

Gah.  It was more aimed at the coverage stuff, but that might work the
same. Alvaro?

> which is a rather blatant waste of cycles. I would suggest an explicit
> do-nothing installcheck rule rather than the hack you came up with here.

I had that at first, but that generates a warning about overwriting the
makefile target - which afaics cannot be fixed.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Tom Lane]

Andres Freund  writes:
> Committed after simplifying the Makefile.

While I have no particular objection to adding these tests, the
commit log's claim that this will improve buildfarm testing is
quite wrong.  The buildfarm runs "make installcheck" not
"make check" in contrib.  What I'm actually seeing in the
buildfarm reports is

make -C pg_stat_statements installcheck
make -C ../../src/test/regress pg_regress
make -C ../../../src/port all
make -C ../backend submake-errcodes
make[4]: Nothing to be done for `submake-errcodes'.
make -C ../../../src/common all
make -C ../backend submake-errcodes
make[4]: Nothing to be done for `submake-errcodes'.
../../src/test/regress/pg_regress --inputdir=. 
--bindir='/Users/buildfarm/bf-data/HEAD/inst/bin'   --port=5678 --temp-config 
../../contrib/pg_stat_statements/pg_stat_statements.conf 
--dbname=contrib_regression_pg_stat_statements 
(using postmaster on Unix socket, port 5678)
== dropping database "contrib_regression_pg_stat_statements" 
==
NOTICE:  database "contrib_regression_pg_stat_statements" does not exist, 
skipping
DROP DATABASE
== creating database "contrib_regression_pg_stat_statements" 
==
CREATE DATABASE
ALTER DATABASE
== running regression test queries==

=
 All 0 tests passed. 
=

which is a rather blatant waste of cycles.  I would suggest an explicit
do-nothing installcheck rule rather than the hack you came up with here.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Andres Freund]

Hi,

On 2016-11-08 10:25:11 +0530, Amit Kapila wrote:
>  ifdef USE_PGXS
>  PG_CONFIG = pg_config
>  PGXS := $(shell $(PG_CONFIG) --pgxs)
> @@ -21,3 +25,29 @@ top_builddir = ../..
>  include $(top_builddir)/src/Makefile.global
>  include $(top_srcdir)/contrib/contrib-global.mk
>  endif
> +
> +# Disabled because these tests require 
> "shared_preload_libraries=pg_stat_statements",
> +# which typical installcheck users do not have (e.g. buildfarm clients).

> +installcheck:;
> +
> +check: regresscheck
> +
> +submake-regress:
> + $(MAKE) -C $(top_builddir)/src/test/regress all
> +
> +submake-pg_stat_statements:
> + $(MAKE) -C $(top_builddir)/contrib/pg_stat_statements

Why is this a submake? That seems to make little sense?   But stepping
back one step further: I don't think we need all the remade rules in the
first place.

REGRESS_OPTS = --temp-config 
$(top_srcdir)/contrib/pg_stat_statements/pg_stat_statements.conf
and then
installcheck: REGRESS=
to prevent installcheck from doing anything ought to be enough these days.

Committed after simplifying the Makefile.

We can't simplify test_decoding's makefile to that extent, because it
uses isolationtester, which we don't provide for contrib yet...

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Amit Kapila]

On Tue, Nov 8, 2016 at 4:23 PM, Ashutosh Sharma  wrote:
> Hi,
>
>> I had also thought about it while preparing patch, but I couldn't find
>> any clear use case.  I think it could be useful during development of
>> a module, but not sure if it is worth to add a target.  I think there
>> is no harm in adding such a target, but lets take an opinion from
>> committer or others as well before adding this target.  What do you
>> say?
>
> Ok. We can do that.
>
> I have verified the updated v2 patch. The patch looks good to me. I am
> marking it  as ready for committer.

Thanks for the review.



-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Ashutosh Sharma]

Hi,

> I had also thought about it while preparing patch, but I couldn't find
> any clear use case.  I think it could be useful during development of
> a module, but not sure if it is worth to add a target.  I think there
> is no harm in adding such a target, but lets take an opinion from
> committer or others as well before adding this target.  What do you
> say?

Ok. We can do that.

I have verified the updated v2 patch. The patch looks good to me. I am
marking it  as ready for committer. Thanks.

With Regards,
Ashutosh Sharma
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Amit Kapila]

On Wed, Nov 2, 2016 at 12:48 PM, Ashutosh Sharma  wrote:
> Hi,
>
> I have started with the review for this patch and would like to share
> some of my initial review comments that requires author's attention.
>

Thanks.

> 1) I am getting some trailing whitespace errors when trying to apply
> this patch using git apply command. Following are the error messages i
> am getting.
>
> [edb@localhost postgresql]$ git apply test_pg_stat_statements-v1.patch
> test_pg_stat_statements-v1.patch:28: trailing whitespace.
> $(MAKE) -C $(top_builddir)/src/test/regress all
> test_pg_stat_statements-v1.patch:41: space before tab in indent.
>  $(REGRESSCHECKS)
> warning: 2 lines add whitespace errors.
>

Fixed.

> 2) The test-case you have added is failing that is because queryid is
> going to be different every time you execute the test-case.
>

It shouldn't be different each time you execute test as this is a hash
code, but I agree that it is not stable and it can vary across
platforms, so removed it from test.

>
> 3) Ok. As you mentioned upthread, I do agree with the fact that we
> can't add pg_stat_statements tests for installcheck as this module
> requires shared_preload_libraries to be set. But, if there is an
> already existing installation with pg_stat_statements module loaded we
> should allow the test to run on this installation if requested
> explicitly by the user. I think we can add some target like
> 'installcheck-force' in the MakeFile that would help the end users to
> run the test on his own installation.
>

I had also thought about it while preparing patch, but I couldn't find
any clear use case.  I think it could be useful during development of
a module, but not sure if it is worth to add a target.  I think there
is no harm in adding such a target, but lets take an opinion from
committer or others as well before adding this target.  What do you
say?


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


test_pg_stat_statements-v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Ashutosh Sharma]

Hi,

I have started with the review for this patch and would like to share
some of my initial review comments that requires author's attention.

1) I am getting some trailing whitespace errors when trying to apply
this patch using git apply command. Following are the error messages i
am getting.

[edb@localhost postgresql]$ git apply test_pg_stat_statements-v1.patch
test_pg_stat_statements-v1.patch:28: trailing whitespace.
$(MAKE) -C $(top_builddir)/src/test/regress all
test_pg_stat_statements-v1.patch:41: space before tab in indent.
 $(REGRESSCHECKS)
warning: 2 lines add whitespace errors.

2) The test-case you have added is failing that is because queryid is
going to be different every time you execute the test-case. I think it
would be better to remove the queryid column from "SELECT queryid,
query, calls, rows from pg_stat_statements ORDER BY rows". Below is
the output i got upon test-case execution.

make regresscheck

== running regression test queries==
test pg_stat_statements   ... FAILED
== shutting down postmaster   ==

3) Ok. As you mentioned upthread, I do agree with the fact that we
can't add pg_stat_statements tests for installcheck as this module
requires shared_preload_libraries to be set. But, if there is an
already existing installation with pg_stat_statements module loaded we
should allow the test to run on this installation if requested
explicitly by the user. I think we can add some target like
'installcheck-force' in the MakeFile that would help the end users to
run the test on his own installation. Thoughts?


Also, I am changed the status in the commit-fest from "Needs review"
to "waiting on Author".

On Mon, Sep 19, 2016 at 6:26 PM, Amit Kapila  wrote:
> On Sat, Sep 10, 2016 at 5:02 PM, Amit Kapila  wrote:
>> On Tue, Aug 30, 2016 at 8:01 AM, Andres Freund  wrote:
>>> On 2016-08-30 07:57:19 +0530, Amit Kapila wrote:
 I will write such a test case either in this week or early next week.
>>>
>>> Great.
>>>
>>
>> Okay, attached patch adds some simple tests for pg_stat_statements.
>> One thing to note is that we can't add pg_stat_statements tests for
>> installcheck as this module requires shared_preload_libraries to be
>> set.  Hope this suffices the need.
>>
>
> Registered this patch for next CF.
>
> --
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Amit Kapila]

On Sat, Sep 10, 2016 at 5:02 PM, Amit Kapila  wrote:
> On Tue, Aug 30, 2016 at 8:01 AM, Andres Freund  wrote:
>> On 2016-08-30 07:57:19 +0530, Amit Kapila wrote:
>>> I will write such a test case either in this week or early next week.
>>
>> Great.
>>
>
> Okay, attached patch adds some simple tests for pg_stat_statements.
> One thing to note is that we can't add pg_stat_statements tests for
> installcheck as this module requires shared_preload_libraries to be
> set.  Hope this suffices the need.
>

Registered this patch for next CF.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Amit Kapila]

On Tue, Aug 30, 2016 at 8:01 AM, Andres Freund  wrote:
> On 2016-08-30 07:57:19 +0530, Amit Kapila wrote:
>> I will write such a test case either in this week or early next week.
>
> Great.
>

Okay, attached patch adds some simple tests for pg_stat_statements.
One thing to note is that we can't add pg_stat_statements tests for
installcheck as this module requires shared_preload_libraries to be
set.  Hope this suffices the need.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


test_pg_stat_statements-v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Andres Freund]

On 2016-08-30 07:57:19 +0530, Amit Kapila wrote:
> I will write such a test case either in this week or early next week.

Great.

> I hope this is not super urgent, let me know if you think otherwise.

It's not urgent, no.

Thanks!

Andres


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Amit Kapila]

On Mon, Aug 29, 2016 at 10:09 PM, Robert Haas  wrote:
> On Sat, Aug 27, 2016 at 3:30 AM, Andres Freund  wrote:
>> Hi,
>>
>> On 2016-02-04 21:43:14 +, Robert Haas wrote:
>>> Change the way that LWLocks for extensions are allocated.
>>>
>>> The previous RequestAddinLWLocks() method had several disadvantages.
>>> First, the locks would be in the main tranche; we've recently decided
>>> that it's useful for LWLocks used for separate purposes to have
>>> separate tranche IDs.  Second, there wasn't any correlation between
>>> what code called RequestAddinLWLocks() and what code called
>>> LWLockAssign(); when multiple modules are in use, it could become
>>> quite difficult to troubleshoot problems where LWLockAssign() ran out
>>> of locks.  To fix, create a concept of named LWLock tranches which
>>> can be used either by extension or by core code.
>>>
>>> Amit Kapila and Robert Haas
>>
>> I noticed that this code has no test coverage:
>>
>> http://coverage.postgresql.org/src/backend/storage/lmgr/lwlock.c.gcov.html
>>
>> It'd be good to add some, although I'm not entirely sure what the best
>> way is. Maybe add a simple pg_stat_statements test?
>
> That would also have the advantage of improving the test coverage for
> pg_stat_statements, which is at the moment quite a bit thinner than
> the coverage for lwlock.c.
>

I will write such a test case either in this week or early next week.
I hope this is not super urgent, let me know if you think otherwise.

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Alvaro Herrera]

Robert Haas wrote:

> That would also have the advantage of improving the test coverage for
> pg_stat_statements, which is at the moment quite a bit thinner than
> the coverage for lwlock.c.

Hmm, the coverage-html report is not currently covering contrib ... I
suppose that's an easily fixable oversight.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [COMMITTERS] pgsql: Change the way that LWLocks for extensions are allocated.

[Robert Haas]

On Sat, Aug 27, 2016 at 3:30 AM, Andres Freund  wrote:
> Hi,
>
> On 2016-02-04 21:43:14 +, Robert Haas wrote:
>> Change the way that LWLocks for extensions are allocated.
>>
>> The previous RequestAddinLWLocks() method had several disadvantages.
>> First, the locks would be in the main tranche; we've recently decided
>> that it's useful for LWLocks used for separate purposes to have
>> separate tranche IDs.  Second, there wasn't any correlation between
>> what code called RequestAddinLWLocks() and what code called
>> LWLockAssign(); when multiple modules are in use, it could become
>> quite difficult to troubleshoot problems where LWLockAssign() ran out
>> of locks.  To fix, create a concept of named LWLock tranches which
>> can be used either by extension or by core code.
>>
>> Amit Kapila and Robert Haas
>
> I noticed that this code has no test coverage:
>
> http://coverage.postgresql.org/src/backend/storage/lmgr/lwlock.c.gcov.html
>
> It'd be good to add some, although I'm not entirely sure what the best
> way is. Maybe add a simple pg_stat_statements test?

That would also have the advantage of improving the test coverage for
pg_stat_statements, which is at the moment quite a bit thinner than
the coverage for lwlock.c.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers