On Mon, Mar 15, 2021 at 6:14 PM Ajin Cherian <itsa...@gmail.com> wrote:
>
> On Mon, Mar 15, 2021 at 2:04 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
>>
>> I think something on these lines should be much
>> easier than the spool-file implementation unless we see any problem
>> with this idea.
>>
>
> Here's a new patch-set that implements this new solution proposed by Amit.

Thanks for the updated patch.
Few comments:
1) These are no longer needed as it has been removed with the new changes.
@@ -1959,6 +1962,8 @@ ProtocolVersion
 PrsStorage
 PruneState
 PruneStepResult
+PsfFile
+PsfHashEntry

2) "Binary mode and streaming and two_phase" should be "Binary mode,
streaming and two_phase" in the below code:
@@ -6097,13 +6097,15 @@ describeSubscriptions(const char *pattern, bool verbose)

        if (verbose)
        {
-               /* Binary mode and streaming are only supported in v14
and higher */
+               /* Binary mode and streaming and two_phase are only
supported in v14 and higher */
                if (pset.sversion >= 140000)
                        appendPQExpBuffer(&buf,

3) We have some reference to psf spoolfile, this should be removed.
Also check if the assert should be <= or ==.
+       /*
+        * Normally, prepare_lsn == remote_final_lsn, but if this
prepare message
+        * was dispatched via the psf spoolfile replay then the remote_final_lsn
+        * is set to commit lsn instead. Hence the <= instead of == check below.
+        */
+       Assert(prepare_data.prepare_lsn <= remote_final_lsn);

4) Similarly in below code:
+       /*
+        * It is possible that we haven't received prepare because it occurred
+        * before walsender reached a consistent point in which case we need to
+        * skip rollback prepared.
+        *
+        * And we also skip the FinishPreparedTransaction if we're using the
+        * Prepare Spoolfile (using_psf) because in that case there is
no matching
+        * PrepareTransactionBlock done yet.
+        */
+       if (LookupGXact(rollback_data.gid, rollback_data.prepare_end_lsn,
+                                       rollback_data.preparetime))
+       {

5) Should this be present:
+#if 1
+       /* This is just debugging, for confirmation the update worked. */
+       {
+               Subscription *new_s;
+
+               StartTransactionCommand();
+               new_s = GetSubscription(MySubscription->oid, false);
+               CommitTransactionCommand();
+       }
+#endif

Regards,
Vignesh


Reply via email to