WIP: Access method extendability

On 15 October 2014 13:08, Alexander Korotkov <aekorotkov@gmail.com> wrote:

Postgres was initially designed to support access methods extendability.
This extendability lives to present day. However, this is mostly internal
in-core extendability. One can quite easily add new access method into
PostgreSQL core. But if one try to implement access method as external
module, he will be faced with following difficulties:

...

Problem of WAL is a bit more complex. According to previous discussions, we
don't want to let extensions declare their own xlog records. If we let them
then recovery process will depend on extensions. That is much violates
reliability. Solution is to implement some generic xlog record which is able
to represent difference between blocks in some general manner.

Thank you for progressing with these thoughts.

I'm still a little uncertain about the approach, now my eyes are open
to the problems of extendability.

The main problem we had in the past was that GiST and GIN indexes both
had faulty implementations for redo, which in some cases caused severe
issues. Adding new indexes will also suffer the same problems, so I
see a different starting place.

The faults there raised the need for us to be able to mark specific
indexes as corrupt, so that they could be avoided during Hot Standby
and in normal running after promotion.

Here's the order of features I think we need

1. A mechanism to mark an index as corrupt so that it won't be usable
by queries. That needs to work during recovery, so we can persist a
data structure which tells us which indexes are corrupt. Then
something that checks whether an index is known corrupt during
relcache access. So if we decide an index is bad, we record the index
as corrupt and then fire a relcache invalidation.

2. Some additional code in Autovacuum to rebuild corrupt indexes at
startup, using AV worker processes to perform a REINDEX CONCURRENTLY.

This will give us what we need to allow an AM to behave sensibly, even
in the face of its own bugs. It also gives us UNLOGGED indexes for
free. Unlogged indexes means we can change the way unlogged tables
behave to allow them to truncate down to the highest unchanged data at
recovery, so we don't lose all the data when we crash.

3. That then allows us to move towards having indexes that are marked
"changed" when we perform first DML on the table in any checkpoint
cycle. Which allows us to rebuild indexes which were in the middle of
being changed when we crashed. (The way we'd do that is to have an LSN
on the metapage and then only write WAL for the metapage). The
difference here is that they are UNLOGGED but do not get trashed on
recovery unless they were in the process of changing.

If we do those things, then we won't even need to worry about needing
AMs to write their own WAL records. Recovery will be safe AND we won't
need to go through problems of buggy persistence implementations in
new types of index.

Or put it another way, it will be easier to write new index AMs
because we'll be able to skip the WAL part until we know we want it.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Oct 28, 2014 at 10:22 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

Or put it another way, it will be easier to write new index AMs
because we'll be able to skip the WAL part until we know we want it.

I like the feature you are proposing, but I don't think that we should
block Alexander from moving forward with a more-extensible WAL format.
I believe that's a general need even if we get the features you're
proposing, which would reduce the need for it. After all, if somebody
builds an out-of-core index AM, ignoring WAL-logging, and then decides
that it works well enough that they want to add WAL-logging, I think
we should make that possible without requiring them to move the whole
thing in-core.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 28 October 2014 14:53, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Oct 28, 2014 at 10:22 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

Or put it another way, it will be easier to write new index AMs
because we'll be able to skip the WAL part until we know we want it.

I like the feature you are proposing, but I don't think that we should
block Alexander from moving forward with a more-extensible WAL format.
I believe that's a general need even if we get the features you're
proposing, which would reduce the need for it. After all, if somebody
builds an out-of-core index AM, ignoring WAL-logging, and then decides
that it works well enough that they want to add WAL-logging, I think
we should make that possible without requiring them to move the whole
thing in-core.

I'm not proposing an alternate or additional feature.

I'm saying that the first essential step in adding WAL support to new
AMs is to realise that they *will* have bugs (since with the greatest
respect, the last two AMs from our friends did have multiple bugs) and
so we must have a mechanism that prevents such bugs from screwing
everything else up. Which is the mark-corrupt-index and rebuild
requirement.

We skip straight to the add-buggy-AMs part at our extreme peril. We've
got about 10x as many users now since the 8.x bugs and all the new
users like the reputation Postgres has for resilience. I think we
should put the safety net in place first before we start to climb.

The patch as submitted doesn't have any safety checks for whether the
WAL records refer to persistent objects defined by the AM. At the very
least we need to be able to isolate an AM to only screw up their own
objects. Such checks look like they'd require some careful thought and
refactoring first.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

On 2014-10-15 16:08:38 +0400, Alexander Korotkov wrote:

Postgres was initially designed to support access methods extendability.
This extendability lives to present day. However, this is mostly internal
in-core extendability. One can quite easily add new access method into
PostgreSQL core. But if one try to implement access method as external
module, he will be faced with following difficulties:

1. Need to directly insert into pg_am, because of no "CREATE ACCESS
METHOD" command. And no support of dependencies between am and opclasses
etc.
2. Module can't define xlog records. So, new am would be not WAL-logged.

The first problem is purely mechanical. Nothing prevents us to implement
"CREATE ACCESS METHOD" and "DROP ACCESS METHOD" commands and support all
required dependencies.

Problem of WAL is a bit more complex. According to previous discussions, we
don't want to let extensions declare their own xlog records. If we let them
then recovery process will depend on extensions. That is much violates
reliability. Solution is to implement some generic xlog record which is
able to represent difference between blocks in some general manner.

I think this is a somewhat elegant way to attack this problem. But I'm
not so sure it's actually sufficient. Consider e.g. how to deal with hot
standby conflicts? How would you transport the knowledge that there's a
xid conflict to the client?

I guess my question essentially is whether it's actually sufficient for
real world AMs.

The other thing I'm not sure about is that I'm unconvinced that we
really want external AMs...

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Oct 28, 2014 at 7:57 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 28 October 2014 14:53, Robert Haas <robertmhaas@gmail.com> wrote:

On Tue, Oct 28, 2014 at 10:22 AM, Simon Riggs <simon@2ndquadrant.com>

wrote:

Or put it another way, it will be easier to write new index AMs
because we'll be able to skip the WAL part until we know we want it.

I like the feature you are proposing, but I don't think that we should
block Alexander from moving forward with a more-extensible WAL format.
I believe that's a general need even if we get the features you're
proposing, which would reduce the need for it. After all, if somebody
builds an out-of-core index AM, ignoring WAL-logging, and then decides
that it works well enough that they want to add WAL-logging, I think
we should make that possible without requiring them to move the whole
thing in-core.

I'm not proposing an alternate or additional feature.

I'm saying that the first essential step in adding WAL support to new
AMs is to realise that they *will* have bugs (since with the greatest
respect, the last two AMs from our friends did have multiple bugs) and
so we must have a mechanism that prevents such bugs from screwing
everything else up. Which is the mark-corrupt-index and rebuild
requirement.

We skip straight to the add-buggy-AMs part at our extreme peril. We've
got about 10x as many users now since the 8.x bugs and all the new
users like the reputation Postgres has for resilience. I think we
should put the safety net in place first before we start to climb.

agree and we thought about this

The patch as submitted doesn't have any safety checks for whether the
WAL records refer to persistent objects defined by the AM. At the very
least we need to be able to isolate an AM to only screw up their own
objects. Such checks look like they'd require some careful thought and
refactoring first.

the patch Alexander submitted is the PoC, we wanted to hear developers
opinion and
I see no principal objection to work in this direction and we'll continue
to work on all
possible issues.

* Andres Freund (andres@2ndquadrant.com) wrote:

The other thing I'm not sure about is that I'm unconvinced that we
really want external AMs...

I was wondering about this also and curious as to if there's been any
prior on-list discussion about this proposal that I've simply missed..?

Would be happy to go back and review earlier discussions, of course, but
I don't recall there being any.

Thanks,

Stephen

On 28 October 2014 16:19, Stephen Frost <sfrost@snowman.net> wrote:

Would be happy to go back and review earlier discussions, of course, but
I don't recall there being any.

It depends how far back you go.

I think I've had at least 2 tries at writing something, but not in last 5 years.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 28 October 2014 14:22, Simon Riggs <simon@2ndquadrant.com> wrote:

Or put it another way, it will be easier to write new index AMs
because we'll be able to skip the WAL part until we know we want it.

To be clear: I am suggesting you do *less* work, not more.

By allowing AMs to avoid writing WAL we get
* higher performance unlogged indexes
* we get fewer bugs in early days of new AMs
* writers of new AMs are OK to avoid majority of hard work and hard testing

So overall, we get new AMs working faster because we can skip writing
the WAL code until we are certain the new AM code is useful and bug
free.

For example, if GIN had avoided implementing WAL it would have been
easier to change on-disk representation.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Stephen Frost <sfrost@snowman.net> writes:

* Andres Freund (andres@2ndquadrant.com) wrote:

The other thing I'm not sure about is that I'm unconvinced that we
really want external AMs...

I was wondering about this also and curious as to if there's been any
prior on-list discussion about this proposal that I've simply missed..?

We've touched on the issue a few times, but I don't think there's been
any attempt to define a project policy about it.

My own thought is that allowing external AMs is simply a natural
consequence of PG's general approach to extensibility, and it would
be surprising if we were to decide we didn't want to allow that.

But having said that, it's quite unclear to me that we need the
CREATE/DROP ACCESS METHOD infrastructure proposed here. The traditional
theory about that is that if you're competent to develop an AM at all,
you can certainly manage to insert a row into pg_am manually. I'm afraid
that we'd be adopting and maintaining thousands of lines of code that
won't ever come close to pulling their weight in usefulness, or probably
ever be fully debugged. (The submitted patch is about 1K lines in itself,
and it doesn't appear to address any of the consequences of establishing
an expectation that AMs are something that can be dropped or modified.
Can you say "cache flush"?)

So I'd be inclined to put that part of the patch on the back burner until
there are actually multiple externally maintained AMs that could use it.
Even then, I'm not sure we want to buy into DROP ACCESS METHOD.

I think we *do* need some credible method for extensions to emit WAL
records, though. I've not taken any close look at the code proposed
for that, but the two-sentence design proposal in the original post
sounded plausible as far as it went.

So my vote is to pursue the WAL extensibility part of this, but not the
additional SQL commands.

As for the proposed contrib module, we don't need it to test the WAL
extensibility stuff: we could just rewrite some existing core code to emit
the "extensible" WAL records instead of whatever it's emitting now.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-10-28 13:06:52 -0400, Tom Lane wrote:

Stephen Frost <sfrost@snowman.net> writes:

* Andres Freund (andres@2ndquadrant.com) wrote:

The other thing I'm not sure about is that I'm unconvinced that we
really want external AMs...

I was wondering about this also and curious as to if there's been any
prior on-list discussion about this proposal that I've simply missed..?

We've touched on the issue a few times, but I don't think there's been
any attempt to define a project policy about it.

My own thought is that allowing external AMs is simply a natural
consequence of PG's general approach to extensibility, and it would
be surprising if we were to decide we didn't want to allow that.

It'd be entirely politicial. I agree. I'm pretty unhappy with the
thought that we end up with several 'for pay' index ams out there. But
then, PG is BSD style licensed.

What I think we need to make absolutely sure is that we preserve the
freedom to tinker with the AM functions. I think we'll very heavily
curse ourselves if we can't as easily add new features there anymore.

But having said that, it's quite unclear to me that we need the
CREATE/DROP ACCESS METHOD infrastructure proposed here. The traditional
theory about that is that if you're competent to develop an AM at all,
you can certainly manage to insert a row into pg_am manually.

The problem with doing that is that you not only need to add a row in
pg_am, but also pg_depend. And a way to remove that row when the
respective extension is dropped. Especially the latter imo changed the
landscape a fair bit.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@2ndquadrant.com> writes:

On 2014-10-28 13:06:52 -0400, Tom Lane wrote:

But having said that, it's quite unclear to me that we need the
CREATE/DROP ACCESS METHOD infrastructure proposed here. The traditional
theory about that is that if you're competent to develop an AM at all,
you can certainly manage to insert a row into pg_am manually.

The problem with doing that is that you not only need to add a row in
pg_am, but also pg_depend.

(1) That's not that hard; initdb makes pg_depend entries from SQL.
(2) You only need a row in pg_depend if you provide a DROP command
that would need to pay attention to it.

And a way to remove that row when the
respective extension is dropped.

I'm not at all sold on the idea that we need to support dropping AMs.
I think it'd be fine to consider that installing an AM into a given
database is a one-way operation. Then you just need to insert some
pg_depend entries that "pin" the AM's individual functions, and you're
done.

Yeah, sure, CREATE/DROP ACCESS METHOD would be cleaner. But in this
case I'm not buying the "if you build it they will come" argument.
External AMs *can* be built without any such SQL-level support, and if
there were really much demand for them, there would be some out there.
Let's build the essential WAL support first, and leave the syntactic
sugar till we see some demand.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 28 October 2014 17:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

My own thought is that allowing external AMs is simply a natural
consequence of PG's general approach to extensibility, and it would
be surprising if we were to decide we didn't want to allow that.

If it wasn't clear from my two earlier attempts, yes, +1 to that.

I'd like to avoid all of the pain by making persistent AMs that are
recoverable after a crash, rather than during crash recovery.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-10-28 17:45:36 +0000, Simon Riggs wrote:

I'd like to avoid all of the pain by making persistent AMs that are
recoverable after a crash, rather than during crash recovery.

Besides the actual difficulities of supporting this, imo not being
available on HS and directly after a failover essentially makes them
next to useless.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 10/28/14, 9:22 AM, Simon Riggs wrote:

2. Some additional code in Autovacuum to rebuild corrupt indexes at
startup, using AV worker processes to perform a REINDEX CONCURRENTLY.

I don't think loading more functionality into autovac is the right way to do that.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Simon Riggs <simon@2ndQuadrant.com> writes:

On 28 October 2014 17:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

My own thought is that allowing external AMs is simply a natural
consequence of PG's general approach to extensibility, and it would
be surprising if we were to decide we didn't want to allow that.

If it wasn't clear from my two earlier attempts, yes, +1 to that.

I'd like to avoid all of the pain by making persistent AMs that are
recoverable after a crash, rather than during crash recovery.

I think the notion of having AMs that explicitly don't have WAL support
is quite orthogonal to what's being discussed in this thread. It might
be worth doing that just to get the hash AM into a less-weird state
(given that nobody is stepping up to the plate to fix it properly).

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2014-10-28 13:37:33 -0400, Tom Lane wrote:

I'm not at all sold on the idea that we need to support dropping AMs.
I think it'd be fine to consider that installing an AM into a given
database is a one-way operation. Then you just need to insert some
pg_depend entries that "pin" the AM's individual functions, and you're
done.

I think that'd be somewhat ugly. An extension adding such a AM would
then either actively need to block dropping (e.g. by pinned entries, as
you mention) or do rather odd things on recreation. I think that'd be
dropping our own standards.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Oct 28, 2014 at 8:04 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On 28 October 2014 14:22, Simon Riggs <simon@2ndquadrant.com> wrote:

Or put it another way, it will be easier to write new index AMs
because we'll be able to skip the WAL part until we know we want it.

To be clear: I am suggesting you do *less* work, not more.

By allowing AMs to avoid writing WAL we get
* higher performance unlogged indexes
* we get fewer bugs in early days of new AMs
* writers of new AMs are OK to avoid majority of hard work and hard testing

So overall, we get new AMs working faster because we can skip writing
the WAL code until we are certain the new AM code is useful and bug
free.

For example, if GIN had avoided implementing WAL it would have been
easier to change on-disk representation.

Major problem of changing on-disk representation is that we have to support
both binary formats because of pg_upgrade. This problem is even burdened
with WAL, because WAL record redo function also have to support both
formats. However, it's also quite independent of WAL.

Having access methods as extensions can significantly improves situations
here. Imagine, GIN was an extension. One day we decide to change its binary
format. Then we can issue new extension, GIN2 for instance. User can
upgrade from GIN to GIN2 in following steps:

1. CREATE EXTENSION gin2;
2. CREATE INDEX CONCURRENTLY [new_index] USING gin2 ([index_def]);
3. DROP INDEX CONCURRENTLY [old_index];
4. DROP EXTENSION gin;

No need to write and debug the code which reads both old and new binary
format. For sure, we need to support old GIN extension for some time. But,
we have to support old in-core versions too.

Unfortunately, I didn't mention this in the first post because I consider
this as a serious argument for extensible AMs.

Also, I'm not sure that many users have enough of courage to use unlogged
AMs. Absence of durability is only half of trouble, another half is lack of
streaming replication. I think if we have unlogged GIN then external
indexing engines would be used by majority of users instead of GIN.

------
With best regards,
Alexander Korotkov.

On Tue, Oct 28, 2014 at 01:51:21PM -0400, Tom Lane wrote:

Simon Riggs <simon@2ndQuadrant.com> writes:

On 28 October 2014 17:06, Tom Lane <tgl@sss.pgh.pa.us> wrote:

My own thought is that allowing external AMs is simply a natural
consequence of PG's general approach to extensibility, and it would
be surprising if we were to decide we didn't want to allow that.

If it wasn't clear from my two earlier attempts, yes, +1 to that.

I'd like to avoid all of the pain by making persistent AMs that are
recoverable after a crash, rather than during crash recovery.

I think the notion of having AMs that explicitly don't have WAL support
is quite orthogonal to what's being discussed in this thread. It might
be worth doing that just to get the hash AM into a less-weird state
(given that nobody is stepping up to the plate to fix it properly).

regards, tom lane

Hi,

I think that someone is working on the hash index WAL problem, but are
coming up to speed on the whole system, which takes time. I know that
I have not had a large enough block of time to spend on it either. :(

Regards,
Ken

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

* Alexander Korotkov (aekorotkov@gmail.com) wrote:

Having access methods as extensions can significantly improves situations
here. Imagine, GIN was an extension. One day we decide to change its binary
format. Then we can issue new extension, GIN2 for instance. User can
upgrade from GIN to GIN2 in following steps:

We could support this without having GIN be an extension by simply
having a GIN2 in core also, so I don't buy off on this being a good
reason for extensions to provide AMs. For my 2c, I'm pretty happy with
the general idea of "read-old, write-new" to deal with transistions.

It's more complicated, certainly, but I don't think trying to force
users off of an old version is actually going to work all that well and
we'd just end up having to support both the old and new extensions
indefinitely anyway.

Thanks,

Stephen