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 01:26:03
Message-ID: 8736ndm5as.fsf@alap4.lan
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi,

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.).

Greetings,

Andres Freund

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2019-03-24 03:06:08 Re: CPU costs of random_zipfian in pgbench
Previous Message Chapman Flack 2019-03-24 01:21:57 Re: The two "XML Fixes" patches still in need of review