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