Re: Pluggable Storage - Andres's take

From: Fabrízio de Royes Mello <fabriziomello(at)gmail(dot)com>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Asim R P <apraveen(at)pivotal(dot)io>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>, Ashwin Agrawal <aagrawal(at)pivotal(dot)io>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Subject: Re: Pluggable Storage - Andres's take
Date: 2019-04-08 15:34:35
Message-ID: CAFcNs+pw+Ztq23RVJ-p65ejLrqOXh5-JFx7ucHDVRT_A7hwThA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Apr 8, 2019 at 9:34 AM Heikki Linnakangas <hlinnaka(at)iki(dot)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(at)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2019-04-08 15:43:24 pgbench - add option to show actual builtin script code
Previous Message Robert Haas 2019-04-08 15:06:57 Re: [PATCH] Implement uuid_version()