Re: Tips on committing
On Fri, Jun 29, 2018 at 06:02:07PM -0400, Bruce Momjian wrote: > On Thu, Jun 28, 2018 at 09:46:17AM -0700, Peter Geoghegan wrote: >> * Don't assume that you haven't broken the doc build if you make even >> a trivial doc change. Removing a GUC can break instances in the >> release notes where they're referenced. Even grep can miss this, since >> references to the GUC will have dashes rather than underscores, plus >> possibly other variations. > > Yes, I even check the docs after whitespace changes. Could it be possible to mention as well to run pgindent before a commit? After the indent run done for 11 beta 1, and it was just a matter of days before some garbage diffs have accumulated in the C code, which makes local runs more annoying when the same files are touched. -- Michael signature.asc Description: PGP signature
pgsql: Add wait event for fsync of WAL segments
Add wait event for fsync of WAL segments This has been visibly a forgotten spot in the first implementation of wait events for I/O added by 249cf07, and what has been missing is a fsync call for WAL segments which is a wrapper reacting on the value of GUC wal_sync_method. Reported-by: Konstantin Knizhnik Author: Konstantin Knizhnik Reviewed-by: Craig Ringer, Michael Paquier Discussion: https://postgr.es/m/4a243897-0ad8-f471-aa40-242591f24...@postgrespro.ru Branch -- master Details --- https://git.postgresql.org/pg/commitdiff/c55de5e5123ce58ee19a47c08425949599285041 Modified Files -- doc/src/sgml/monitoring.sgml | 4 src/backend/access/transam/xlog.c | 2 ++ src/backend/postmaster/pgstat.c | 3 +++ src/include/pgstat.h | 1 + 4 files changed, 10 insertions(+)
Re: Tips on committing
Greetings, * Peter Geoghegan (p...@bowt.ie) wrote: > FWIW, I developed a document on committing for my own reference, with > some help from Andres. A lot of it is about commit message style, the > use of fields, and so on. But I've also developed a check list for > committing, knowing that there are a few classic mistakes that > committers will make from time to time despite knowing better. These > are worth checking against mechanically, IMV. Here are some actual > examples from this document that I refer to: Good list. > * Do a dry run before really pushing by using --dry-run. In addition to this, I'd recommend using 'git show' on the results of the --dry-run, so that you see what you're really about to push. Thanks! Stephen signature.asc Description: PGP signature
Re: Tips on committing
On 2018-Jul-02, Stephen Frost wrote:
> > * Do a dry run before really pushing by using --dry-run.
>
> In addition to this, I'd recommend using 'git show' on the results of
> the --dry-run, so that you see what you're really about to push.
Since commit 653530c8b196 I use this little script I borrowed from Magnus, then
page through all of it before pushing.
git push --dry-run 2>&1 | grep -v '^To' | while read line; do
if [ "$line" == "Everything up-to-date" ]; then
echo $line
else
topush=$(echo $line | awk '{print $1}')
git log --format=oneline $topush | cat
git show --format=fuller --color $topush | cat
fi
done | less -R
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgsql: Add wait event for fsync of WAL segments
On 2018-Jul-02, Michael Paquier wrote: > Add wait event for fsync of WAL segments > > This has been visibly a forgotten spot in the first implementation of > wait events for I/O added by 249cf07, and what has been missing is a > fsync call for WAL segments which is a wrapper reacting on the value of > GUC wal_sync_method. I wonder if we should backpatch this one all the way to pg10. I don't see no reason not to. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: pgsql: Clarify use of temporary tables within partition trees
On Fri, Jun 29, 2018 at 5:13 AM, David Rowley wrote: > On 20 June 2018 at 13:53, Michael Paquier wrote: >> Clarify use of temporary tables within partition trees > > Thanks for committing this fix. > > I think slightly more should have been done. There's still some dead > code in expand_partitioned_rtentry that I think should be removed. > > The code won't cost much performance wise, but it may mislead someone > into thinking they can add some other condition there to skip > partitions. > > The attached removes it. I'd rather keep an elog(ERROR) than completely remove the check. Also, for the record, I think the subject line of Michael's commit message was pretty unclear about what it was actually doing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: pgsql: Clarify use of temporary tables within partition trees
On Mon, Jul 02, 2018 at 02:07:37PM -0400, Robert Haas wrote: > I'd rather keep an elog(ERROR) than completely remove the check. +1. > Also, for the record, I think the subject line of Michael's commit > message was pretty unclear about what it was actually doing. How would you formulate it? Perhaps the error message did not emphasize enough on the fast that it actually blocked a behavior, say "Block mix of temporary and permanent relations in partition trees" or such? -- Michael signature.asc Description: PGP signature
Re: pgsql: Add wait event for fsync of WAL segments
On Mon, Jul 02, 2018 at 12:23:35PM -0400, Alvaro Herrera wrote: > I wonder if we should backpatch this one all the way to pg10. I don't > see no reason not to. ABI breakage (if that's the correct wording?). Simply cherry-picking the patch from master to back-branches would cause extensions and plugins already compiled with those versions to be confused by the ordering of the enum WaitEventIO. Well, one simple solution is to simply put the new event purposefully at the bottom of the list. If that sounds right, I could do that on back-branches but I'd rather let HEAD on it current state with the event set correctly ordered. -- Michael signature.asc Description: PGP signature
Re: pgsql: Clarify use of temporary tables within partition trees
On 3 July 2018 at 10:16, Michael Paquier wrote: > On Mon, Jul 02, 2018 at 02:07:37PM -0400, Robert Haas wrote: >> I'd rather keep an elog(ERROR) than completely remove the check. > > +1. Attached >> Also, for the record, I think the subject line of Michael's commit >> message was pretty unclear about what it was actually doing. > > How would you formulate it? Perhaps the error message did not emphasize > enough on the fast that it actually blocked a behavior, say "Block mix > of temporary and permanent relations in partition trees" or such? For me, reading the subject line of the commit I'd have expected a doc change, or improved/new code comments. This is really more "Disallow mixed temp/permanent partitioned hierarchies". "Clarify" does not really involve a change of behaviour. It's an explanation of what the behaviour is. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services remove_dead_code_from_expand_partitioned_rtentry_v2.patch Description: Binary data
Re: pgsql: Clarify use of temporary tables within partition trees
On Mon, Jul 2, 2018 at 8:59 PM, David Rowley wrote: >> How would you formulate it? Perhaps the error message did not emphasize >> enough on the fast that it actually blocked a behavior, say "Block mix >> of temporary and permanent relations in partition trees" or such? Yes. > For me, reading the subject line of the commit I'd have expected a doc > change, or improved/new code comments. > > This is really more "Disallow mixed temp/permanent partitioned hierarchies". Yes. > "Clarify" does not really involve a change of behaviour. It's an > explanation of what the behaviour is. And yes. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
