Re: [COMMITTERS] pgsql: Reset master xmin when hot_standby_feedback disabled.

Reset master xmin when hot_standby_feedback disabled.
If walsender has xmin of standby then ensure we
reset the value to 0 when we change from hot_standby_feedback=on
to hot_standby_feedback=off.

Branch
------
REL9_2_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/2dde11a632d3fe309b5af5480d01a0a3028f7f64

Modified Files
--------------
doc/src/sgml/protocol.sgml | 5 +++-
src/backend/replication/walreceiver.c | 45 +++++++++++++++++++++++++--------
src/backend/replication/walsender.c | 5 +++-
3 files changed, 42 insertions(+), 13 deletions(-)

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> Reset master xmin when hot_standby_feedback disabled.
> If walsender has xmin of standby then ensure we
> reset the value to 0 when we change from hot_standby_feedback=on
> to hot_standby_feedback=off.

> Branch
> ------
> REL9_2_STABLE

While I'm not necessarily objecting to the content of this patch,
I do have a problem with the process. Where was the discussion of
why this change should be back-patched? (If I'm identifying it
correctly, this is a back-patch of commit bd56e7412, a year and a
half later. That should have been noted in the commit message, too,
rather than leaving people to reconstruct why you'd only committed
into two old branches.)

regards, tom lane

On 15 July 2014 19:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> Reset master xmin when hot_standby_feedback disabled.
>> If walsender has xmin of standby then ensure we
>> reset the value to 0 when we change from hot_standby_feedback=on
>> to hot_standby_feedback=off.
>
>> Branch
>> ------
>> REL9_2_STABLE
>
>
> While I'm not necessarily objecting to the content of this patch,
> I do have a problem with the process. Where was the discussion of
> why this change should be back-patched? (If I'm identifying it
> correctly, this is a back-patch of commit bd56e7412, a year and a
> half later. That should have been noted in the commit message, too,
> rather than leaving people to reconstruct why you'd only committed
> into two old branches.)

Sorry if my actions confused.

I kept the commit message deliberately identical to help people, not to confuse.

There was recent discussion of it on-list and a public request to
backpatch, which I agreed with and acknowledged.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 15 July 2014 19:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Where was the discussion of
> why this change should be back-patched?

Your question has broader implications. Currently commit messages do
not reference the conversations that led to them and a great many use
entirely different descriptions on-list and in the commit message, nor
are entries added to conversations to reflect commits.

I share your pain in not being able to follow or remember what led to
many of them. In fact, I'm pretty sure more than a few commits have no
public discussion at all, though mostly there is good discussion.

I'm happy to follow any agreed process we lay out.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> On 15 July 2014 19:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> While I'm not necessarily objecting to the content of this patch,
>> I do have a problem with the process. Where was the discussion of
>> why this change should be back-patched?

> There was recent discussion of it on-list and a public request to
> backpatch, which I agreed with and acknowledged.

I searched the archives looking for that discussion and couldn't find it;
can you provide a link?

> I kept the commit message deliberately identical to help people, not to confuse.

That's appropriate when you're committing functionally identical patches
into multiple branches at about the same time. In a situation like this,
though, I'd argue that the later commits ought to explicitly reference
the older one ("this is a back-patch of commit NNNNNNN"). As it stands,
it's very hard for anyone looking at the commit logs to make the
connection.

regards, tom lane

On 15 July 2014 22:01, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
>> On 15 July 2014 19:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> While I'm not necessarily objecting to the content of this patch,
>>> I do have a problem with the process. Where was the discussion of
>>> why this change should be back-patched?
>
>> There was recent discussion of it on-list and a public request to
>> backpatch, which I agreed with and acknowledged.
>
> I searched the archives looking for that discussion and couldn't find it;
> can you provide a link?

http://www.postgresql.org/message-id/E1U2JOd-0005W4-OM@gemulon.postgresql.org

>> I kept the commit message deliberately identical to help people, not to confuse.
>
> That's appropriate when you're committing functionally identical patches
> into multiple branches at about the same time. In a situation like this,
> though, I'd argue that the later commits ought to explicitly reference
> the older one ("this is a back-patch of commit NNNNNNN"). As it stands,
> it's very hard for anyone looking at the commit logs to make the
> connection.

Sounds reasonable, I will endeavour to follow that in future.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> On 15 July 2014 22:01, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I searched the archives looking for that discussion and couldn't find it;
>> can you provide a link?

> http://www.postgresql.org/message-id/E1U2JOd-0005W4-OM@gemulon.postgresql.org

Ah. I hadn't searched backwards quite far enough :-(. Sorry for the
noise.

regards, tom lane

Simon Riggs wrote:
> On 15 July 2014 19:15, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > Where was the discussion of
> > why this change should be back-patched?
>
> Your question has broader implications. Currently commit messages do
> not reference the conversations that led to them and a great many use
> entirely different descriptions on-list and in the commit message, nor
> are entries added to conversations to reflect commits.

I try to mention message-ids in commit messages. Are these useful?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Jul 15, 2014 at 9:58 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> I try to mention message-ids in commit messages. Are these useful?

In my view, yes, certainly.

--
Peter Geoghegan