On Sat, Mar 28, 2020 at 04:14:04PM +0530, Amit Kapila wrote: > > I see some basic problems with the patch. The way it tries to compute > WAL usage for parallel stuff doesn't seem right to me. Can you share > or point me to any test done where we have computed WAL for parallel > operations like Parallel Vacuum or Parallel Create Index?
Ah, that's indeed a good point and AFAICT WAL records from parallel utility workers won't be accounted for. That being said, I think that an argument could be made that proper infrastructure should have been added in the original parallel utility patches, as pg_stat_statement is already broken wrt. buffer usage in parallel utility, unless I'm missing something. > Basically, > I don't know changes done in ExecInitParallelPlan and friends allow us > to compute WAL for parallel operations. Those will primarily cover > parallel queries that won't write WAL. How you have tested those > changes? I didn't tested those, and I'm not even sure how to properly and reliably test that. Do you have any advice on how to achieve that? However the patch is mimicking the buffer instrumentation that already exists, and the approach also looks correct to me. Do you have a reason to believe that the approach that works for buffer usage wouldn't work for WAL records? (I of course agree that this should be tested anyway)
