RE: Rework LogicalOutputPluginWriterUpdateProgress

Dear Wang,

Thank you for updating the patch! I have briefly tested your patch
and it worked well in following case.

* WalSndUpdateProgressAndKeepalive is called when many inserts have come
but the publisher does not publish the insertion. PSA the script for this.
* WalSndUpdateProgressAndKeepalive is called when the commit record is not
related with the specified database
* WalSndUpdateProgressAndKeepalive is called when many inserts for unlogged
tables are done.

> > ---
> > 01. missing comments
> > You might miss the comment from Peter[1]. Or could you pin the related one?
>
> Since I think the functions WalSndPrepareWrite and WalSndWriteData have
> similar
> parameters and the HEAD has no related comments, I'm not sure whether we
> should
> add them in this patch, or in a separate patch to comment atop these callback
> functions or where they are called.

Make sense, OK.

> > ---
> > 02. LogicalDecodingProcessRecord()
> >
> > Don't we have to call UpdateDecodingProgressAndKeepalive() when there is no
> > decoding function? Assuming that the timeout parameter does not have enough
> > time
> > period and there are so many sequential operations in the transaction. At that
> > time
> > there may be a possibility that timeout is occurred while calling
> > ReorderBufferProcessXid()
> > several times. It may be a bad example, but I meant to say that we may have to
> > consider the case that decoding function has not implemented yet.
>
> I think it's ok in this function. If the decoding function has not been
> implemented for a record, I think we quickly return to the loop in the function
> WalSndLoop, where it will try to send the keepalive message.

I confirmed that and yes, we will go back to WalSndLoop().

> BTW, in the previous discussion [1], we decided to ignore some paths, because
> the gain from modifying them may not be so great.

I missed the discussion, thanks. Based on that codes seems right.

Followings are my comments.

---
```
+/*
+ * Update progress tracking and send keep alive (if required).
+ */
+static void
+UpdateProgressAndKeepalive(LogicalDecodingContext *ctx, bool finished_xact)
```

Can we add atop the UpdateProgressAndKeepalive()? Currently the developers who
create output plugins must call OutputPluginUpdateProgress(), but from now the
function is not only renamed but does not have nessesary to call from plugin
(of cource we do not restrict to call it). I think it must be clarified for them.

---
ReorderBufferUpdateProgressTxnCB must be removed from typedefs.list.

---
Do we have to write a document for the breakage somewhere? I think we do not have
to add appendix-obsolete-* file because we did not have any links for that, but
we can add a warning in "Functions for Producing Output" subsection if needed.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED