Hi Kuroda.
Thanks for the feedback.
> 01.
> I feel the patch looks large and it is bit difficult to be reviewed. Can you
> separate it into several parts, e.g., producer and consumer parts?
>
Ok I will try to split it.
> 02.
> The file name seems to be "xlogpipeline.{h|c}" per others.
>
Noted, thanks.
> 03.
> Can you add tests? it's also helpful to understand the patch.
>
Right now I am trying to fix the existing tests under
`src/test/recovery/`. But sure we should add some pipeline
specific tests.
> 04.
> Some Assert() are commented out but they won't be accepted. Can you added a
> function like AmWalPipelineProducer() and call there?
>
Noted. Sure I will add AmWalPipelineProducer().
> 05.
> Producer is implemented as the bgworker process, but we have another option to
> be as a background process. Which one is better and why?
>
Yes I myself don't have any good answer for this question. For me the
bgworker api looked simple and good enough to start the pipeline
implementation.
> 06.
> Can we enable the pipeline with the streaming replication? I've tried but it
> did not work well - the producer exit and recovery finished without any
> promote
> signals. My expectation was that producer waits till new record was arrived.
>
> ```
> ...
> LOG: starting backup recovery with redo LSN 0/02000028, checkpoint LSN
> 0/02000080, on timeline ID 1
> LOG: entering standby mode
> LOG: [walpipeline] started.
> LOG: redo starts at 0/02000028
> LOG: completed backup recovery with redo LSN 0/02000028 and end LSN
> 0/02000128
> LOG: consistent recovery state reached at 0/02000128
> LOG: database system is ready to accept read-only connections
> LOG: [walpipeline] consumer: received shutdown message from the producer
> LOG: [walpipeline] producer: exiting: sent=4 received=4
> LOG: WAL pipeline stopped
> LOG: redo done at 0/02000128 system usage: CPU: user: 0.00 s, system: 0.02
> s, elapsed: 0.03 s
> ...
> ```
Yes am working to fix the streaming replication, so far the
testing/benchmarking were done with archive recovery.
>
> 07.
> As mentioned by Jakub, it is better to profile per process (startup and
> producer).
>
Noted.
> 08.
> I suggest to benchmark on larger environment. Do let me know if difficult - I
> can
> try on my env instead.
>
Yes, that would be very helpful. I myself tried very hard to collect
some good benchmarks but have some hardware limitations. If you can
help me with this , that would be great. You can use the scripts[1]
that I used. Let me know if you need any help understanding or using
the scripts.
[1]: https://github.com/imranzaheer612/pg-recovery-testing
Thanks for your valuable feedback. That was very helpful.
Regards,
Imran Zaheer
