Re: RFC: replace pg_stat_activity.waiting with something more descriptive

On Tue, Aug 4, 2015 at 5:45 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Tue, Jul 28, 2015 at 3:28 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
wrote:
> > * The patch requires that the LWLOCK_INDIVIDUAL_NAMES array is kept in
sync
> > with the list of individual locks in lwlock.h. Sooner or later someone
will
> > add an LWLock and forget to update the names-array. That needs to be
made
> > less error-prone, so that the names are maintained in the same place as
the
> > #defines. Perhaps something like rmgrlist.h.
>
> This is a good idea, but it's not easy to do in the style of
> rmgrlist.h, because I don't believe there's any way to define a macro
> that expands to a preprocessor directive. Attached is a patch that
> instead generates the list of macros from a text file, and also
> generates an array inside lwlock.c with the lock names that gets used
> by the Trace_lwlocks stuff where applicable.
>
> Any objections to this solution to the problem? If not, I'd like to
> go ahead and push this much. I can't test the Windows changes
> locally, though, so it would be helpful if someone could check that
> out.
>

Okay, I have check it on Windows and found below issues which I
have fixed in patch attached with this mail.

1. Got below error while running mkvcbuild.pl

Use of uninitialized value in numeric lt (<) at Solution.pm line 103.
Could not open src/backend/storage/lmgr/lwlocknames.h at Mkvcbuild.pm line
674.

+ if (IsNewer(
+ 'src/backend/storage/lmgr/lwlocknames.txt',
'src/include/storage/lwlocknames.h'))

I think the usage of old and new filenames in IsNewer is reversed here.

2.
+ print "Generating lwlocknames.c and fmgroids.h...\n";

Here it should be lwlocknames.h instead of fmgroids.h

3.
+ if (IsNewer(
+ 'src/include/storage/lwlocknames.h',
+
'src/backend/storage/lmgr/fmgroids.h'))

Here, it seems lwlocknames.h needs to compared instead of fmgroids.h

4. Got below error while running mkvcbuild.pl
Generating lwlocknames.c and fmgroids.h...
/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit
*/
/* there is deliberately not an #ifndef LWLOCKNAMES_H here */
rename: lwlocknames.h.tmp3180: Permission denied at generate-lwlocknames.pl
line
59, <$lwlocknames> line 47.

In generate-lwlocknames.pl, below line is causing problem.

rename($htmp, 'lwlocknames.h') || die "rename: $htmp: $!";

The reason is that closing of tmp files is missing.

Some other general observations

5.
On running perl mkvcbuild.pl in Windows, below message is generated.

Generating lwlocknames.c and lwlocknames.h...
/* autogenerated from src/backend/storage/lmgr/lwlocknames.txt, do not edit
*/
/* there is deliberately not an #ifndef LWLOCKNAMES_H here */

Following message gets displayed, which looks slightly odd, displaying
it in C comments makes it look out of place and other similar .pl files
don't generate such message. Having said that, I think it displays useful
information, so it might be okay to retain it.

6.
+
+maintainer-clean: clean
+ rm -f lwlocknames.h

Here, don't we need to clean lwlocknames.c?

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