On Wed, Feb 26, 2014 at 3:02 AM, Stephen Frost <sfrost@snowman.net> wrote:
>> The custom-scan node is intended to perform on regular relations, not
>> only foreign tables. It means a special feature (like GPU acceleration)
>> can perform transparently for most of existing applications. Usually,
>> it defines regular tables for their work on installation, not foreign
>> tables. It is the biggest concern for me.
>
> The line between a foreign table and a local one is becoming blurred
> already, but still, if this is the goal then I really think the
> background worker is where you should be focused, not on this Custom
> Scan API. Consider that, once we've got proper background workers,
> we're going to need new nodes which operate in parallel (or some other
> rejiggering of the nodes- I don't pretend to know exactly what Robert is
> thinking here, and I've apparently forgotten it if he's posted it
> somewhere) and those interfaces may drive changes which would impact the
> Custom Scan API- or worse, make us deprecate or regret having added it
> because now we'll need to break backwards compatibility to add in the
> parallel node capability to satisfy the more general non-GPU case.
This critique seems pretty odd to me. I haven't had the time to look
at this patch set, but I don't see why anyone would want to use the
background worker facility for GPU acceleration, which is what
KaiGai's trying to accomplish here. Surely you want, if possible, to
copy the data directly from the user backend into the GPU's working
memory. What would the use of a background worker be? We definitely
need background workers to make use of additional *CPUs*, but I don't
see what good they are in leveraging *GPUs*.
I seriously doubt there's any real conflict with parallelism here.
Parallelism may indeed add more ways to scan a relation
(ParallelSeqScan, ParallelIndexScan?) but that doesn't mean that we
shouldn't have a Custom Scan node too. Indeed, my principle concern
about this patch set isn't that it's too specialized, as you seem to
be worrying about, but that it's aiming to satisfy *too many* use
cases. I think FDW join pushdown is a fairly different problem from
adding a custom scan type, and I doubt one patch should try to solve
both problems.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company