Re: Pluggable Storage - Andres's take

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Pluggable Storage - Andres's take
Date: 2018-10-15 19:06:25
Message-ID: CAPpHfdt2v3fa-fBvMOr6g=C7Gc_GmOX+zzMKqqcx1OOva4bjeQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hi!

On Wed, Oct 3, 2018 at 8:16 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> I've pushed an updated version, with a fair amount of pending changes,
> and I hope all your pending (and not redundant, by our concurrent
> development), patches merged.

I'd like to also share some patches. I've used current state of
pluggable-zheap for the base of my patches.

* 0001-remove-extra-snapshot-functions.patch – removes
snapshot_satisfiesUpdate() and snapshot_satisfiesVacuum() functions
from tableam API. snapshot_satisfiesUpdate() was unused completely.
snapshot_satisfiesVacuum() was used only in heap_copy_for_cluster().
So, I've replaced it with direct heapam_satisfies_vacuum().

* 0002-add-costing-function-to-API.patch – adds function for costing
sequential and table sample scan to tableam API. zheap costing
function are now copies of heap costing function. This should be
adjusted in future. Estimation for heap lookup during index scans
should be also pluggable, but not yet implemented (TODO).

I've examined code in pluggable-zheap branch and EDB github [1] and I
didn't found anything related to "delete-marking" indexes as stated on
slide #25 of presentation [2]. So, basically contract between heap
and indexes is remain unchanged: once you update one indexed field,
you have to update all the others. Did I understand correctly that
this is postponed?

And couple more notes from me:
* Right now table_fetch_row_version() is called in most of places with
SnapshotAny. That might be working in majority of cases, because in
heap there couldn't be multiple tuples residing the same TID, while
zheap always returns most recent tuple residing this TID. But I think
it would be better to provide some meaningful snapshot instead of
SnapshotAny. If even the best thing we can do is to ask for most
recent tuple on some TID, we need more consistent way for asking table
AM for this. I'm going to elaborate more on this.
* I'm not really sure we need ability to iterate multiple tuples
referenced by index. It seems that the only place, which really needs
this is heap_copy_for_cluster(), which itself is table AM specific.
Also zheap doesn't seem to be able to return more than one tuple by
zheapam_fetch_follow(). So, I'm going to investigate more on this.
And if this iteration is really unneeded, I'll propose a patch to
delete that.

1. https://github.com/EnterpriseDB/zheap
2. http://www.pgcon.org/2018/schedule/attachments/501_zheap-a-new-storage-format-postgresql-5.pdf

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
0001-remove-extra-snapshot-functions.patch application/octet-stream 4.4 KB
0002-add-costing-function-to-API.patch application/octet-stream 15.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Justin Pryzby 2018-10-15 19:11:03 pg11rc1 DROP INDEX: NOTICE: drop_trigger
Previous Message Tom Lane 2018-10-15 18:48:24 Re: Index-only scan returns incorrect results when using a composite GIST index with a gist_trgm_ops column.