Re: [HACKERS] Logical replication origin tracking fix

[Peter Eisentraut]

On 3/24/17 10:48, Petr Jelinek wrote:
> On 10/03/17 05:59, Petr Jelinek wrote:
>> while discussing with Craig issues around restarting logical replication
>> stream related to the patch he posted [1], I realized that we track
>> wrong origin LSN in the logical replication apply.
>>
>> We currently track commit_lsn which is *start* of commit record, what we
>> need to track is end_lsn which is *end* of commit record otherwise we
>> might request transaction that was already replayed if the subscription
>> instance has crashed right after commit.
>>
>> Attached patch fixes that.
>>
> 
> Rebase after table copy patch got committed.

committed

-- 
Peter Eisentraut  http://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] Logical replication origin tracking fix

[Petr Jelinek]

On 10/03/17 05:59, Petr Jelinek wrote:
> Hi,
> 
> while discussing with Craig issues around restarting logical replication
> stream related to the patch he posted [1], I realized that we track
> wrong origin LSN in the logical replication apply.
> 
> We currently track commit_lsn which is *start* of commit record, what we
> need to track is end_lsn which is *end* of commit record otherwise we
> might request transaction that was already replayed if the subscription
> instance has crashed right after commit.
> 
> Attached patch fixes that.
> 

Rebase after table copy patch got committed.

-- 
  Petr Jelinek  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services
>From 2a312a4ecce09aab6c72f5f039c49b2300b3f3a4 Mon Sep 17 00:00:00 2001
From: Petr Jelinek 
Date: Thu, 9 Mar 2017 12:54:43 +0100
Subject: [PATCH] Fix remote position tracking in logical replication

We need to set the origin remote position to end_lsn and not commit_lsn
as commit_lsn is start of commit record and we use the origin remote
position as start position when restarting replication stream. If we'd
use commit_lsn we could request data that we already received from
remote server after crash of downstream server.
---
 src/backend/replication/logical/worker.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index bbf3506..c2ccab7 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -421,9 +421,6 @@ apply_handle_begin(StringInfo s)
 
 	logicalrep_read_begin(s, _data);
 
-	replorigin_session_origin_timestamp = begin_data.committime;
-	replorigin_session_origin_lsn = begin_data.final_lsn;
-
 	remote_final_lsn = begin_data.final_lsn;
 
 	in_remote_transaction = true;
@@ -443,14 +440,18 @@ apply_handle_commit(StringInfo s)
 
 	logicalrep_read_commit(s, _data);
 
-	Assert(commit_data.commit_lsn == replorigin_session_origin_lsn);
-	Assert(commit_data.committime == replorigin_session_origin_timestamp);
-
 	Assert(commit_data.commit_lsn == remote_final_lsn);
 
 	/* The synchronization worker runs in single transaction. */
 	if (IsTransactionState() && !am_tablesync_worker())
 	{
+		/*
+		 * Update origin state so we can restart streaming from correct
+		 * position in case of crash.
+		 */
+		replorigin_session_origin_lsn = commit_data.end_lsn;
+		replorigin_session_origin_timestamp = commit_data.committime;
+
 		CommitTransactionCommand();
 
 		store_flush_position(commit_data.end_lsn);
-- 
2.7.4


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

[HACKERS] Logical replication origin tracking fix

[Petr Jelinek]

Hi,

while discussing with Craig issues around restarting logical replication
stream related to the patch he posted [1], I realized that we track
wrong origin LSN in the logical replication apply.

We currently track commit_lsn which is *start* of commit record, what we
need to track is end_lsn which is *end* of commit record otherwise we
might request transaction that was already replayed if the subscription
instance has crashed right after commit.

Attached patch fixes that.

[1]
https://www.postgresql.org/message-id/CAMsr+YGFvikx-U_mHQ0mAzTarqvCpwzvsPKv=7mfp9scdrm...@mail.gmail.com

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


0001-Fix-remote-position-tracking-in-logical-replication.patch
Description: binary/octet-stream

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