On Fri, May 5, 2017 at 8:13 PM, Petr Jelinek
<petr.jelinek@2ndquadrant.com> wrote:
> On 03/05/17 13:23, Erik Rijkers wrote:
>> On 2017-05-03 08:17, Petr Jelinek wrote:
>>> On 02/05/17 20:43, Robert Haas wrote:
>>>> On Thu, Apr 20, 2017 at 2:58 PM, Peter Eisentraut
>>
>>>>> code path that calls CommitTransactionCommand() should have one, no?
>>>>
>>>> Is there anything left to be committed here?
>>>>
>>>
>>> Afaics the fix was not committed. Peter wanted more comprehensive fix
>>> which didn't happen. I think something like attached should do the job.
>>
>> I'm running my pgbench-over-logical-replication test in chunk of 15
>> minutes, wth different pgbench -c (num clients) and -s (scale) values.
>>
>> With this patch (and nothing else) on top of master (8f8b9be51fd7 to be
>> precise):
>>
>>> fix-statistics-reporting-in-logical-replication-work.patch
>>
>> logical replication is still often failing (as expected, I suppose; it
>> seems because of "inital snapshot too large") but indeed I do not see
>
> Yes that's different thing that we've been discussing a bit in snapbuild
> woes thread.
>
>> the 'TRAP: FailedAssertion in pgstat.c' anymore.
>>
>> (If there is any other configuration of patches worth testing please let
>> me know)
>>
>
> Thanks, so the patch works.
>
I think that we should commit the local transaction that did initial
data copy, and then report stat as well. Currently table sync worker
doesn't commit the local transaction in LogicalRepSyncTableStart
(maybe until apply commit record?) if its status is changed to
SUBREL_STATE_CATCHUP. That's why the table sync worker issues
assertion failure.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center