Re: Parallel Seq Scan

On Fri, Oct 16, 2015 at 9:51 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Mon, Oct 5, 2015 at 8:20 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
> > [ new patch for heapam.c changes ]
>
> I went over this in a fair amount of detail and reworked it somewhat.
> The result is attached as parallel-heapscan-revised.patch. I think
> the way I did this is a bit cleaner than what you had, although it's
> basically the same thing. There are fewer changes to initscan, and we
> don't need one function to initialize the starting block that must be
> called in each worker and then another one to get the next block, and
> generally the changes are a bit more localized. I also went over the
> comments and, I think, improved them. I tweaked the logic for
> reporting the starting scan position as the last position report; I
> think the way you had it the last report would be for one block
> earlier. I'm pretty happy with this version and hope to commit it
> soon.
>

static BlockNumber
+heap_parallelscan_nextpage(HeapScanDesc scan)
+{
+ BlockNumber page = InvalidBlockNumber;
+ BlockNumber sync_startpage = InvalidBlockNumber;
+ BlockNumber report_page = InvalidBlockNumber;

..
..
+ * Report scan location. Normally, we report the current page number.
+ * When we reach the end of the scan, though, we report the starting page,
+ * not the ending page, just so the starting positions for later scans
+ * doesn't slew backwards. We only report the position at the end of the
+ * scan once, though: subsequent callers will have report nothing, since
+ * they will have page == InvalidBlockNumber.
+ */
+ if (scan->rs_syncscan)
+ {
+ if (report_page == InvalidBlockNumber)
+ report_page = page;
+ if (report_page != InvalidBlockNumber)
+ ss_report_location(scan->rs_rd, report_page);
+ }
..
}

I think due to above changes it will report sync location on each page
scan, don't we want to report it once at end of scan?

Other than that, patch looks okay to me.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com