Horton

Re: Pluggable Storage - Andres's take

Fabrízio de Royes Mello <fabriziomello@gmail.com>

View thread View on postgr.es
From: Fabrízio de Royes Mello <fabriziomello@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Andres Freund <andres@anarazel.de>, Justin Pryzby <pryzby@telsasoft.com>, Haribabu Kommi <kommi.haribabu@gmail.com>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, Alvaro Herrera <alvherre@2ndquadrant.com>, David Rowley <david.rowley@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Asim R P <apraveen@pivotal.io>, Pgsql Hackers <pgsql-hackers@postgresql.org>, Ashwin Agrawal <aagrawal@pivotal.io>, Dmitry Dolgov <9erthalion6@gmail.com>, Alexander Korotkov <a.korotkov@postgrespro.ru>
Date: 2019-04-08T15:34:35Z
Lists: pgsql-hackers 
On Mon, Apr 8, 2019 at 9:34 AM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>
> I wrote a little toy implementation that just returns constant data to
> play with this a little. Looks good overall.
>
> There were a bunch of typos in the comments in tableam.h, see attached.
> Some of the comments could use more copy-editing and clarification, I
> think, but I stuck to fixing just typos and such for now.
>
> index_update_stats() calls RelationGetNumberOfBlocks(<table>). If the AM
> doesn't use normal data files, that won't work. I bumped into that with
> my toy implementation, which wouldn't need to create any data files, if
> it wasn't for this.
>
> The comments for relation_set_new_relfilenode() callback say that the AM
> can set *freezeXid and *minmulti to invalid. But when I did that, VACUUM
> hits this assertion:
>
> TRAP: FailedAssertion("!(((classForm->relfrozenxid) >= ((TransactionId)
> 3)))", File: "vacuum.c", Line: 1323)
>
> There's a little bug in index-only scan executor node, where it mixes up
> the slots to hold a tuple from the index, and from the table. That
> doesn't cause any ill effects if the AM uses TTSOpsHeapTuple, but with
> my toy AM, which uses a virtual slot, it caused warnings like this from
> index-only scans:
>
> WARNING:  problem in alloc set ExecutorState: detected write past chunk
> end in block 0x56419b0f88e8, chunk 0x56419b0f8f90
>
> Attached is a patch with the toy implementation I used to test this.
> I'm not suggesting we should commit that - although feel free to do that
> if you think it's useful - but it shows how I bumped into these issues.
> The second patch fixes the index-only-scan slot confusion (untested,
> except with my toy AM).
>

Awesome... it's built and ran tests cleanly, but I got assertion running
VACUUM:

fabrizio=# vacuum toytab ;
TRAP: FailedAssertion("!(((classForm->relfrozenxid) >= ((TransactionId)
3)))", File: "vacuum.c", Line: 1323)
psql: server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: 2019-04-08
12:29:16.204 -03 [20844] LOG:  server process (PID 24457) was terminated by
signal 6: Aborted
2019-04-08 12:29:16.204 -03 [20844] DETAIL:  Failed process was running:
vacuum toytab ;
2019-04-08 12:29:16.204 -03 [20844] LOG:  terminating any other active
server processes
2019-04-08 12:29:16.205 -03 [24458] WARNING:  terminating connection
because of crash of another server process

And backtrace is:

(gdb) bt
#0  0x00007f813779f428 in __GI_raise (sig=sig@entry=6) at
../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007f81377a102a in __GI_abort () at abort.c:89
#2  0x0000000000ec0de9 in ExceptionalCondition (conditionName=0x10e3bb8
"!(((classForm->relfrozenxid) >= ((TransactionId) 3)))",
errorType=0x10e33f3 "FailedAssertion", fileName=0x10e345a "vacuum.c",
lineNumber=1323) at assert.c:54
#3  0x0000000000893646 in vac_update_datfrozenxid () at vacuum.c:1323
#4  0x000000000089127a in vacuum (relations=0x26c4390,
params=0x7ffeb1a3fb30, bstrategy=0x26c4218, isTopLevel=true) at vacuum.c:452
#5  0x00000000008906ae in ExecVacuum (pstate=0x26145b8, vacstmt=0x25f46f0,
isTopLevel=true) at vacuum.c:196
#6  0x0000000000c3a883 in standard_ProcessUtility (pstmt=0x25f4a50,
queryString=0x25f3be8 "vacuum toytab ;", context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x25f4b48, completionTag=0x7ffeb1a3ffb0 "")
at utility.c:670
#7  0x0000000000c3977a in ProcessUtility (pstmt=0x25f4a50,
queryString=0x25f3be8 "vacuum toytab ;", context=PROCESS_UTILITY_TOPLEVEL,
params=0x0, queryEnv=0x0, dest=0x25f4b48, completionTag=0x7ffeb1a3ffb0 "")
at utility.c:360
#8  0x0000000000c3793e in PortalRunUtility (portal=0x265ba28,
pstmt=0x25f4a50, isTopLevel=true, setHoldSnapshot=false, dest=0x25f4b48,
completionTag=0x7ffeb1a3ffb0 "") at pquery.c:1175
#9  0x0000000000c37d7f in PortalRunMulti (portal=0x265ba28,
isTopLevel=true, setHoldSnapshot=false, dest=0x25f4b48, altdest=0x25f4b48,
completionTag=0x7ffeb1a3ffb0 "") at pquery.c:1321
#10 0x0000000000c36899 in PortalRun (portal=0x265ba28,
count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x25f4b48,
altdest=0x25f4b48, completionTag=0x7ffeb1a3ffb0 "") at pquery.c:796
#11 0x0000000000c2a40e in exec_simple_query (query_string=0x25f3be8 "vacuum
toytab ;") at postgres.c:1215
#12 0x0000000000c332a3 in PostgresMain (argc=1, argv=0x261fe68,
dbname=0x261fca8 "fabrizio", username=0x261fc80 "fabrizio") at
postgres.c:4249
#13 0x0000000000b051fc in BackendRun (port=0x2616d20) at postmaster.c:4429
#14 0x0000000000b042c3 in BackendStartup (port=0x2616d20) at
postmaster.c:4120
#15 0x0000000000afc70a in ServerLoop () at postmaster.c:1703
#16 0x0000000000afb94e in PostmasterMain (argc=3, argv=0x25ed850) at
postmaster.c:1376
#17 0x0000000000977de8 in main (argc=3, argv=0x25ed850) at main.c:228


Isn't better raise an exception as you did in other functions??

static void
toyam_relation_vacuum(Relation onerel,
                      struct VacuumParams *params,
                      BufferAccessStrategy bstrategy)
{
    ereport(ERROR,
            (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
             errmsg("function %s not implemented yet", __func__)));
}

Regards,

--
   Fabrízio de Royes Mello         Timbira - http://www.timbira.com.br/
   PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento

Commits

  1. Add default_table_access_method to postgresql.conf.sample.

  2. tableam: Avoid relying on relation size to determine validity of tids.

  3. tableam: Don't assume that every AM uses md.c style storage.

  4. Allow pg_class xid & multixid horizons to not be set.

  5. Fix slot type issue for fuzzy distance index scan over out-of-core table AM.

  6. tableam: comment and formatting fixes.

  7. table: docs: fix typos and grammar.

  8. tableam: basic documentation.

  9. Only allow heap in a number of contrib modules.

  10. tableam: Add table_finish_bulk_insert().

  11. tableam: sample scan.

  12. tableam: bitmap table scan.

  13. tableam: Move heap specific logic from estimate_rel_size below tableam.

  14. tableam: VACUUM and ANALYZE support.

  15. tableam: relation creation, VACUUM FULL/CLUSTER, SET TABLESPACE.

  16. tableam: Support for an index build's initial table scan(s).

  17. tableam: Add table_get_latest_tid, to wrap heap_get_latest_tid.

  18. tableam: Add helper for indexes to check if a corresponding table tuples exist.

  19. tableam: Add and use table_fetch_row_version().

  20. tableam: Use in CREATE TABLE AS and CREATE MATERIALIZED VIEW.

  21. Ensure sufficient alignment for ParallelTableScanDescData in BTShared.

  22. Don't reuse slots between root and partition in ON CONFLICT ... UPDATE.

  23. Use a virtual rather than a heap slot in two places where that suffices.

  24. Store tuples for EvalPlanQual in slots, rather than as HeapTuples.

  25. Use slots in trigger infrastructure, except for the actual invocation.

  26. Store table oid and tuple's tid in tuple slots directly.

  27. Allow to use HeapTupleData embedded in [Buffer]HeapTupleTableSlot.

  28. Add ExecStorePinnedBufferHeapTuple.

  29. Add ArchiveOpts to pass options to ArchiveEntry

  30. Rename RelationData.rd_amroutine to rd_indam.

  31. Rephrase references to "time qualification".

  32. Move remaining code from tqual.[ch] to heapam.h / heapam_visibility.c.

  33. Move generic snapshot related code from tqual.h to snapmgr.h.

  34. Remove superfluous tqual.h includes.

  35. Don't duplicate parallel seqscan shmem sizing logic in nbtree.

  36. Move vacuumlazy.c into access/heap.

  37. Support parallel bitmap heap scans.