On Thu, Mar 28, 2019 at 11:43 AM Haribabu Kommi <kommi.harib...@gmail.com> wrote: > > On Wed, Mar 27, 2019 at 11:27 PM Amit Kapila <amit.kapil...@gmail.com> wrote: >> >> On Wed, Mar 27, 2019 at 6:53 AM Haribabu Kommi <kommi.harib...@gmail.com> >> wrote: >> > On Tue, Mar 26, 2019 at 9:08 PM Amit Kapila <amit.kapil...@gmail.com> >> > wrote: >> >> >> >> As part of this thread, maybe we can >> >> just fix the case of the parallel cooperating transaction. >> > >> > >> > With the current patch, all the parallel implementation transaction are >> > getting >> > skipped, in my tests parallel workers are the major factor in the >> > transaction >> > stats counter. Even before parallelism, the stats of the autovacuum and etc >> > are still present but their contribution is not greatly influencing the >> > stats. >> > >> > I agree with you in fixing the stats with parallel workers and improve >> > stats. >> > >> >> I was proposing to fix only the transaction started with >> StartParallelWorkerTransaction by using is_parallel_worker flag as >> discussed above. I understand that it won't completely fix the >> problem reported by you, but it will be a step in that direction. My >> main worry is that if we fix it the way you are proposing and we also >> invent a new way to deal with all other internal transactions, then >> the fix done by us now needs to be changed/reverted. Note, that this >> fix needs to be backpatched as well, so we should avoid doing any fix >> which needs to be changed or reverted. > > > I tried the approach as your suggested as by not counting the actual parallel > work > transactions by just releasing the resources without touching the counters, > the counts are not reduced much. > > HEAD - With 4 parallel workers running query generates 13 stats ( 4 * 3 + 1) > Patch - With 4 parallel workers running query generates 9 stats ( 4 * 2 + 1) > Old approach patch - With 4 parallel workers running query generates 1 stat > (1) > > Currently the parallel worker start transaction 3 times in the following > places. > 1. InitPostgres > 2. ParallelWorkerMain (2) > > with the attached patch, we reduce one count from ParallelWorkerMain. >
Right, that is expected from this fix. BTW, why you have changed the approach in this patch to not count anything by the parallel worker as compared to the earlier version where you were just ignoring the stats for transactions. As of now, either way is fine, but in future (after parallel inserts/updates), we want other stats to be updated. I think your previous idea was better, just you need to start using is_parallel_worker flag. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
