Re: Pluggable Storage - Andres's take

Hi,

(resending with compressed attachements, perhaps that'll go through)

On 2018-12-10 18:13:40 -0800, Andres Freund wrote:
> On 2018-11-26 17:55:57 -0800, Andres Freund wrote:
> > FWIW, now that oids are removed, and the tuple table slot abstraction
> > got in, I'm working on rebasing the pluggable storage patchset ontop of
> > that.
> 
> I've pushed a version to that to the git tree, including a rebased
> version of zheap:
> https://github.com/anarazel/postgres-pluggable-storage
> https://github.com/anarazel/postgres-pluggable-zheap

I've pushed the newest, substantially revised, version to the same
repository. Note, that while the newest pluggable-zheap version is newer
than my last email, it's not based on the latest version, and the
pluggable-zheap development is now happening in the main zheap
repository.


> My next steps are:
> - make relation creation properly pluggable
> - remove the typedefs from tableam.h, instead move them into the
>   TableAmRoutine struct.
> - Move rs_{nblocks, startblock, numblocks} out of TableScanDescData
> - Move HeapScanDesc and IndexFetchHeapData out of relscan.h
> - remove ExecSlotCompare(), it's entirely unrelated to these changes imo
>   (and in the wrong place)

These are done.


> - split pluggable storage patchset, to commit earlier:
>   - EvalPlanQual slotification
>   - trigger slotification
>   - split of IndexBuildHeapScan out of index.c

The patchset is now pretty granularly split into individual pieces.
There's two commits that might be worthwhile to split up further:

1) The commit introducing table_beginscan et al, currently also
   introduces indexscans through tableam.
2) The commit introducing table_(insert|delete|update) also includes
   table_lock_tuple(), which in turn changes a bunch of EPQ related
   code. It's probably worthwhile to break that out.

I tried to make each individual commit make some sense, and pass all
tests on its own. That requires some changes that are then obsolted
in a later commit, but it's not as much as I feared.


> - rename HeapUpdateFailureData et al to not reference Heap

I've not done that, I decided it's best to do that after all the work
has gone in.


> - See if the slot in SysScanDescData can be avoided, it's not exactly
>   free of overhead.

After reconsidering, I don't think it's worth doing so.


There's pretty substantial changes in this series, besides the things
mentioned above:

- I re-introduced parallel scan into pluggable storage, but added a set
  of helper functions to avoid having to duplicate the current block
  based logic from heap. That way it can be shared between most/all
  block based AMs
- latestRemovedXid handling is moved into the table-AM, that's required
  for correct replay on Hot-Standby, where we do not know the AM of the
  current
- the whole truncation and relation creation code has been overhauled
- the order of functions in tableam.h, heapam_handler.c etc has been
  made more sensible
- a number of callbacks have been obsoleted (relation_sync,
  relation_create_init_fork, scansetlimits)
- A bunch of prerequisite work has been merged
- (heap|relation)_(open|openrv|close) have been split into their own
  files
- To avoid having to care about the bulk-insert flags code that uses a
  bulk-insert now unconditionally calls table_finish_bulk_insert(). The
  AM then internally can decide what it needs to do in case of
  e.g. HEAP_INSERT_SKIP_WAL.  Zheap currently for example doesn't
  implement that (because UNDO handling is complicated), and this way it
  can just ignore the option, without needing call-site code for that.
- A *lot* of cleanups

Todo:
- merge psql / pg_dump support by Dmitry
- consider removing scan_update_snapshot
- consider removing table_gimmegimmeslot()
- add substantial docs for every callback
- consider revising the current table_lock_tuple() API, I'm not quite
  convinced that's right
- reconsider heap_fetch() API changes, causes unnecessary pain
- polish the split out trigger and EPQ changes, so they can be merged
  soon-ish


I plan to merge the first few commits pretty soon (as largely announced
in related threads).


While I saw an initial attempt at writing smgl docs for the table AM
API, I'm not convinced that's the best approach.  I think it might make
more sense to have high-level docs in sgml, but then do all the
per-callback docs in tableam.h.

Greetings,

Andres Freund