Re: Pluggable Storage - Andres's take

From: Andres Freund <andres(at)anarazel(dot)de>
To: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Asim R P <apraveen(at)pivotal(dot)io>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Ashwin Agrawal <aagrawal(at)pivotal(dot)io>, Dmitry Dolgov <9erthalion6(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
Subject: Re: Pluggable Storage - Andres's take
Date: 2019-03-24 03:16:30
Message-ID: 20190324031630.nt7numguo5ojq6uv@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi,

(sorry, I somehow miskeyed, and sent a partial version of this email
before it was ready)

On 2019-03-21 11:15:57 -0700, Andres Freund wrote:
> Pending work:
> - Wondering if table_insert/delete/update should rather be
> table_tuple_insert etc. Would be a bit more consistent with the
> callback names, but a bigger departure from existing code.

I've left this as is.

> - I'm not yet happy with TableTupleDeleted computation in heapam.c, I
> want to revise that further

I changed that. Found a bunch of untested paths, I've pushed tests for
those already.

> - formatting

Done that.

> - commit message

Done that.

> - a few comments need a bit of polishing (ExecCheckTIDVisible, heapam_tuple_lock)

Done that.

> - Rename TableTupleMayBeModified to TableTupleOk, but also probably a s/TableTuple/TableMod/

It's now TM_*.

/*
* Result codes for table_{update,delete,lock}_tuple, and for visibility
* routines inside table AMs.
*/
typedef enum TM_Result
{
/*
* Signals that the action succeeded (i.e. update/delete performed, lock
* was acquired)
*/
TM_Ok,

/* The affected tuple wasn't visible to the relevant snapshot */
TM_Invisible,

/* The affected tuple was already modified by the calling backend */
TM_SelfModified,

/*
* The affected tuple was updated by another transaction. This includes
* the case where tuple was moved to another partition.
*/
TM_Updated,

/* The affected tuple was deleted by another transaction */
TM_Deleted,

/*
* The affected tuple is currently being modified by another session. This
* will only be returned if (update/delete/lock)_tuple are instructed not
* to wait.
*/
TM_BeingModified,

/* lock couldn't be acquired, action skipped. Only used by lock_tuple */
TM_WouldBlock
} TM_Result;

> - I'll probably move TUPLE_LOCK_FLAG_LOCK_* into tableam.h

Done.

> - two more passes through the patch

One of them completed. Which is good, because there was a subtle bug in
heapam_tuple_lock (*tid was adjusted to be the followup tuple after the
heap_fetch(), before going to heap_lock_tuple - but that's wrong, it
should only be adjusted when heap_fetch() ing the next version.).

I'm pretty happy with that last version (of the first patch). I'm
planning to do one more pass, and then push.

There are no meaningful changes to later patches in the series besides
followup changes required from changes in the first patch.

Greetings,

Andres Freund

Attachment Content-Type Size
v21-0001-tableam-Add-tuple_-insert-delete-update-lock-and.patch.gz application/x-patch-gzip 34.3 KB
v21-0002-tableam-Add-fetch_row_version.patch.gz application/x-patch-gzip 3.4 KB
v21-0003-tableam-Add-use-tableam_fetch_follow_check.patch.gz application/x-patch-gzip 1.4 KB
v21-0004-tableam-Add-table_get_latest_tid.patch.gz application/x-patch-gzip 1.7 KB
v21-0005-tableam-multi_insert-and-slotify-COPY.patch.gz application/x-patch-gzip 6.5 KB
v21-0006-tableam-finish_bulk_insert.patch.gz application/x-patch-gzip 1.8 KB
v21-0007-tableam-slotify-CREATE-TABLE-AS-and-CREATE-MATER.patch.gz application/x-patch-gzip 1.5 KB
v21-0008-tableam-index-builds.patch.gz application/x-patch-gzip 16.1 KB
v21-0009-tableam-relation-creation-VACUUM-FULL-CLUSTER-SE.patch.gz application/x-patch-gzip 15.4 KB
v21-0010-tableam-VACUUM-and-ANALYZE.patch.gz application/x-patch-gzip 5.4 KB
v21-0011-tableam-planner-size-estimation.patch.gz application/x-patch-gzip 3.9 KB
v21-0012-tableam-Sample-Scan-Support.patch.gz application/x-patch-gzip 7.2 KB
v21-0013-tableam-bitmap-heap-scan.patch.gz application/x-patch-gzip 5.0 KB
v21-0014-tableam-Only-allow-heap-in-a-number-of-contrib-m.patch.gz application/x-patch-gzip 1.3 KB
v21-0015-WIP-Move-xid-horizon-computation-for-page-level-.patch.gz application/x-patch-gzip 6.2 KB
v21-0016-tableam-Add-function-to-determine-newest-xid-amo.patch.gz application/x-patch-gzip 1.2 KB
v21-0017-tableam-Fetch-tuples-for-triggers-EPQ-using-a-pr.patch.gz application/x-patch-gzip 2.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2019-03-24 03:18:39 Re: Pluggable Storage - Andres's take
Previous Message Tomas Vondra 2019-03-24 03:06:08 Re: CPU costs of random_zipfian in pgbench