[PostgreSQL-Hackers] synchronous_commit and synchronous_replication Re: [COMMITTERS] pgsql: Efficient transaction-controlled synchronous replication.

Committed with some additional hacking. In particular, I believe that
your version made SYNCHRONOUS_COMMIT_LOCAL equivalent to
SYNCHRONOUS_COMMIT_OFF, which was wrong; and your replacement of
synchronous_replication by synchronous_commit in the docs was a bit
too formulaic; in particular, the section on setting up a basic sync
rep configuration said that all you needed to do was set
synchronous_commit=on, which clearly made no sense, since that was
neither necessary (since that's the default) nor sufficient (since you
have to set synchronous_standby_names).

Err, woops. Actually, I'm wrong about the first point: your coding
worked, but I had to adjust it when I reordered the enum. I think the
new ordering is more logical, but YMMV.

Yes. Thanks a lot!

Regards,

Hi,

Robert Haas <robertmhaas@gmail.com> writes:
Without discussion? I would think that this patch is stepping on the
other one toes and that maybe would need to make a decision about sync
rep behavior before to commit this change.

Maybe it's just me, but I'm struggling to understand current community
processes and decisions.

Regards,

Hmm.. I think that we reached the consensus about merging two GUCs
in previous discussion. You argue that synchronization level should be
controlled in separate two parameters?

Regards,

No, sorry about confusion. One GUC is better. What I'm wondering is
why commit it *now*, because I think we didn't yet decide on what the
supported behaviors supported in 9.1 should be.

Regards,

What do you mean by "supported behaviors"?

Well, I'm thinking about Simon's patch that some decided on procedural
grounds only that it was too late to apply in 9.1, and some others were
saying that given the cost benefit ratio of such a small patch that the
design had already been agreed on, it is legible for 9.1.

I think that we just expressed opinions on the async|recv|fsync|apply
patch, and that we've yet to reach a consensus and make a decision.

Regards,

Err, I thought we did. We had a protracted discussion of Simon's
patch: 9 people expressed an opinion; 6 were opposed.

With respect to this patch, the basic design was discussed previously
and Simon, Fujii Masao, Greg Stark and myself all were basically in
favor of something along these lines, and to the best of my
recollection no one spoke against it.
Well, I've already spent a fair amount of time trying to explain my
understanding of it, and for my trouble I got accused of being
long-winded. Which is probably true, but makes me think I should
probably keep this response short. I'm not unwilling to talk about
it, though, and perhaps someone else would like to chime in.

For what it's worth it seems to me this patch makrmes it at least
conceptually easier to add new modes like Simon plans, not harder. It's
worth making sure we pick names that still make sense when the new
functionality goes in of course.

The other question is whether it's "fair" that one kind of patch goes in and
not the other. Personally I feel changes to GUCs are the kind of thing we
most often want to do in alpha. Patches that change functionality require a
higher barrier and need to be fixing user complaints or bugs. My perception
was that Simon's patch was ggreenberg latter.
synchronous_commit.

I rather liked the brief comment in a recent post of yours where you
said that at this point we should only be accepting patches which
stabilize what has already been committed, rather than new features
which might require further stabilization. I don't know whether the
patch under discussion satisfies that test, but that should be the
main consideration at this point in the release cycle, in my view.

Of course, with anything this complex there will be gray areas where
people could have honest disagreement.

-Kevin

Quite. While we're on the subject, why did that int->money patch get
committed so quickly? I had assumed that was 9.2 material, because it
didn't seem to be addressing any new-in-9.1 issue. I'm not going to ask
for it to be backed out, but I am wondering what the decision process
was.

regards, tom lane

Well, I posted a note about this on Thursday:

http://archives.postgresql.org/pgsql-hackers/2011-03/msg01930.php

I didn't feel strongly that it needed to be done, but there seemed to
be some support for doing it:

http://archives.postgresql.org/pgsql-hackers/2011-03/msg01940.php
http://archives.postgresql.org/pgsql-hackers/2011-03/msg01943.php

But I'm wondering whether that was really the right decision. It
might have been better just to drop it, and I'll certainly back it out
if people feel that's more appropriate.

I am also wondering about the open issue of supporting comments to
SQL/MED objects. I thought that was pretty straightforward, but given
that it took me three commits to get servers and foreign data wrappers
squared away and then it turned out that we're still missing support
for user mappings, I've been vividly reminded of the danger of
seemingly harmless commits. Now I'm thinking that I should have just
replied to the initial report with "good point, but it's not a new
regression, so we'll fix it in 9.2". But given that part of the work
has already been done, I'm not sure whether I should (a) finish it, so
we don't have to revisit this in 9.2, (b) leave it well enough alone,
and we'll finish it in 9.2, or (c) back out what's already been done
and plan to fix the whole thing in 9.2.

Everything else on the open items list appears to be a bona fide bug,
though the generate_series thing is not a new regression.

On further review, I think (a) is not even an option worth discussing.
The permissions-checking logic for user mappings is quite different
from what we do in the general case, and it seems likely to me that
cleaning this up is going to require far more time and thought than we
ought to be putting into what is really a relatively minor wart. In
retrospect, it seems clear that this wasn't worth messing with in the
first place at this late date in the release cycle.

If there are any other items on the open items list that seem like
things we should not be worrying about right now, please point them
out. I'm likely guilty of tunnel vision, as I have been heavily
focused on trying to make the list go to zero, and of course
committing stuff is only one of two possible ways to get them off the
list - the other is to decide that that they shouldn't have been added
in the first place.

I agree that we should leave user mappings alone at the moment. I don't
see a need to back out the work that's been done for the other object
types, unless you think there may be flaws in that.

regards, tom lane