Re: Re: BUG #10329: Could not read block 0 in file "base/56100265/57047884": read only 0 of 8192 bytes

The following bug has been logged on the website:

Bug reference: 10329
Logged by: Olivier Macchioni
Email address: olivier(dot)macchioni(at)wingo(dot)ch
PostgreSQL version: 9.1.13
Operating system: Debian 7.4 and 6.0.9
Description:

Hello all,

- Streaming Replication setup, 2 DB servers
- 9.1.13 on both sides (from apt.postgresql.org)
- Debian 6.0.9 on master server, 7.4 on standby server

When doing the following query on the standby server:

select * from ir_translation where src = 'a';

We have the following error in the logfiles:

ERROR: could not read block 0 in file "base/56100265/57047884": read only 0
of 8192 bytes

When checking the file usage, the following is reported:

postgres(at)db-stdby:~/9.1/main/base/56100265$
/usr/lib/postgresql/9.1/bin/oid2name -d xx -f 57047884
>From database "xx":
Filenode Table Name
---------------------------------------
57047884 ir_translation_src_hash_idx

ls -la shows that the file has a size of 0.

What's interesting as well is that this file has a last modification date of
April, 29th - when the DB has been created on the Master server much more
recently, by doing a "drop xx; rename xx_new xx;"

root(at)db-stdby:/var/lib/postgresql/9.1/main/base/56100265# stat 57047884
File: `57047884'
Size: 0 Blocks: 0 IO Block: 4096 regular empty
file
Device: 801h/2049d Inode: 1051439 Links: 1
Access: (0600/-rw-------) Uid: ( 106/postgres) Gid: ( 111/postgres)
Access: 2014-05-15 11:15:50.218038536 +0200
Modify: 2014-04-29 16:20:36.645605377 +0200
Change: 2014-04-29 16:20:36.645605377 +0200
Birth: -

root(at)db:/var/lib/postgresql/9.1/main/base/56100265# stat 57047884
File: `57047884'
Size: 2129920 Blocks: 4168 IO Block: 4096 regular file
Device: 801h/2049d Inode: 315480 Links: 1
Access: (0600/-rw-------) Uid: ( 104/postgres) Gid: ( 107/postgres)
Access: 2014-05-15 11:24:29.000000000 +0200
Modify: 2014-05-03 23:08:11.000000000 +0200
Change: 2014-05-03 23:08:11.000000000 +0200

The master server doesn't show this behavior.

The symptoms are similar to some previous (but much older) posts on this
list, for instance
http://www.postgresql.org/message-id/E1TEjPD-0002Yq-7L@wrigleys.postgresql.org

So I think this bug is not fixed yet...

olivier(dot)macchioni(at)wingo(dot)ch writes:
> When doing the following query on the standby server:

> select * from ir_translation where src = 'a';

> We have the following error in the logfiles:

> ERROR: could not read block 0 in file "base/56100265/57047884": read only 0
> of 8192 bytes

> When checking the file usage, the following is reported:

> postgres(at)db-stdby:~/9.1/main/base/56100265$
> /usr/lib/postgresql/9.1/bin/oid2name -d xx -f 57047884
>> From database "xx":
> Filenode Table Name
> ---------------------------------------
> 57047884 ir_translation_src_hash_idx

"hash_idx"? Is this a hash index? Those aren't supported for replication
purposes (no WAL code :-().

regards, tom lane

> [...]
> "hash_idx"? Is this a hash index? Those aren't supported for replication
> purposes (no WAL code :-().
>
> regards, tom lane

Bingo:

xx=# \d ir_translation_src_hash_idx
Index "public.ir_translation_src_hash_idx"
Column | Type | Definition
--------+---------+------------
src | integer | src
hash, for table "public.ir_translation"

(for the record, this is created by the stock version of OpenERP v6)

And it's the only hash index out of 1741 other indexes on my system...

I guess my best bet is to replace it by another kind of indexes... and maybe one day PostgreSQL will be clever enough to issue a warning / error in such a case for the people like me who don't read *all the doc* :P

Thanks a lot and sorry for the noise :)

Olivier

On Thu, May 15, 2014 at 05:20:35PM +0200, Olivier Macchioni wrote:
> > [...]
> > "hash_idx"? Is this a hash index? Those aren't supported for replication
> > purposes (no WAL code :-().
> >
> > regards, tom lane
>
> Bingo:
>
> xx=# \d ir_translation_src_hash_idx
> Index "public.ir_translation_src_hash_idx"
> Column | Type | Definition
> --------+---------+------------
> src | integer | src
> hash, for table "public.ir_translation"
>
>
> (for the record, this is created by the stock version of OpenERP v6)
>
> And it's the only hash index out of 1741 other indexes on my system...
>
> I guess my best bet is to replace it by another kind of indexes... and maybe one day PostgreSQL will be clever enough to issue a warning / error in such a case for the people like me who don't read *all the doc* :P
>
> Thanks a lot and sorry for the noise :)

Yes, streaming replication has made our hash indexes even worse. In the
past, I have suggested we issue a warning for the creation of hash
indexes, but did not get enough agreement.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> On Thu, May 15, 2014 at 05:20:35PM +0200, Olivier Macchioni wrote:
>> I guess my best bet is to replace it by another kind of indexes... and maybe one day PostgreSQL will be clever enough to issue a warning / error in such a case for the people like me who don't read *all the doc* :P

> Yes, streaming replication has made our hash indexes even worse. In the
> past, I have suggested we issue a warning for the creation of hash
> indexes, but did not get enough agreement.

Mainly because it wouldn't be a very helpful message.

I wonder though if we could throw a flat-out error for attempts to use
a hash index on a hot standby server. That would get people's attention
without being mere nagging in other situations. It's not a 100% solution
because you'd still lose if you tried to use a hash index on a slave
since promoted to master. But it would help without being a large
sink for effort.

regards, tom lane

On May 15, 2014, at 21:07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> On Thu, May 15, 2014 at 05:20:35PM +0200, Olivier Macchioni wrote:
>>> I guess my best bet is to replace it by another kind of indexes... and maybe one day PostgreSQL will be clever enough to issue a warning / error in such a case for the people like me who don't read *all the doc* :P
>
>> Yes, streaming replication has made our hash indexes even worse. In the
>> past, I have suggested we issue a warning for the creation of hash
>> indexes, but did not get enough agreement.
>
> Mainly because it wouldn't be a very helpful message.
>
> I wonder though if we could throw a flat-out error for attempts to use
> a hash index on a hot standby server. That would get people's attention
> without being mere nagging in other situations. It's not a 100% solution
> because you'd still lose if you tried to use a hash index on a slave
> since promoted to master. But it would help without being a large
> sink for effort.
>
> regards, tom lane

I am not aware of the technical implementation hurdles, but in this case any of the following behaviors would have been better for me:

- still perform the query, without using the index (preferably issuing a warning of some kind)

- Tom's solution - refuse to perform the query, with a clear error message (looking at the error, I thought I had a broken replication, it's clearly not a nice message to have)

- (possibly replace on-the-fly any hash index by another kind of indexes on the slave when setting up the replication or creating the index on the master? does this even make sense?)

- somehow disallow to setup of streaming replication when there is a hash index in any DB / and refuse the creation of a hash index on a master server, so we don't reach this situation

- I assume there is a usage for hash indexes and we don't simply want to deprecate them

Olivier

Olivier Macchioni <olivier(dot)macchioni(at)wingo(dot)ch> writes:
> - I assume there is a usage for hash indexes and we don't simply want to deprecate them

There's a school of thought that says we *should* deprecate them as
long as we don't have WAL support, but that viewpoint is not universally
subscribed to.

One of the arguments against Bruce's proposal to print a warning at hash
index creation is that it's a particularly ineffective form of
deprecation. In your example, since the hash index was created by some
app not manually, I'll bet nobody would have seen/noticed the warning
even if there had been one.

regards, tom lane

Tom Lane-2 wrote
> Bruce Momjian &lt;

> bruce@

> &gt; writes:
>> On Thu, May 15, 2014 at 05:20:35PM +0200, Olivier Macchioni wrote:
>>> I guess my best bet is to replace it by another kind of indexes... and
>>> maybe one day PostgreSQL will be clever enough to issue a warning /
>>> error in such a case for the people like me who don't read *all the doc*
>>> :P
>
>> Yes, streaming replication has made our hash indexes even worse. In the
>> past, I have suggested we issue a warning for the creation of hash
>> indexes, but did not get enough agreement.
>
> Mainly because it wouldn't be a very helpful message.
>
> I wonder though if we could throw a flat-out error for attempts to use
> a hash index on a hot standby server. That would get people's attention
> without being mere nagging in other situations. It's not a 100% solution
> because you'd still lose if you tried to use a hash index on a slave
> since promoted to master. But it would help without being a large
> sink for effort.

At least a promoted slave can "REINDEX" and get back to functioning with
minimal fuss.

Side question: if one were to do this intentionally is there a recommended
way to have the REINDEX run immediately upon the former slave becoming
promoted?

I have to presume there is some reason why we do not currently resolve;

base/56100265/57047884

into something more useful. It is obviously possible since oid2name exists.
I suspect some of it may just be "hasn't been worth the effort" and some of
it is "expensive to compute and if the error is happening repeatedly it
could bog down the system". But knowing what type of relation is affected,
and conditionally reporting additional diagnostic detail based upon that
type, has value since, as this case shows, when an error like this arises
the typical user is going into a state of panic and very little info is
immediately at hand to couch that for non-critical situations.

OpenERP should be more helpful in their own right since they know they are
using a feature with limitations; though given the lack of complaints we are
not that popular with them and/or people are not using hot-standby slaves
for queries on this particular table.

All that said I don't see how it would really hurt to issue a notice upon
creation of a hash index. Providing multiple opportunities for someone to
see the message, question its meaning, and learn why it is being issued
would decrease the chances of people being surprised; and I cannot imagine
the check for index type, and the resultant logging, would be considered
expensive relative to how long a CREATE INDEX typically would run.

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/BUG-10329-Could-not-read-block-0-in-file-base-56100265-57047884-read-only-0-of-8192-bytes-tp5804037p5804123.html
Sent from the PostgreSQL - bugs mailing list archive at Nabble.com.

On Thu, May 15, 2014 at 8:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> One of the arguments against Bruce's proposal to print a warning at hash
> index creation is that it's a particularly ineffective form of
> deprecation. In your example, since the hash index was created by some
> app not manually, I'll bet nobody would have seen/noticed the warning
> even if there had been one.

I suggested we make a GUC allow_unrecoverable_indexes and default it
to false. If you want to create hash indexes you need to set it to
true or else you just get errors.

A more general solution is to emit a WAL record the first time a
non-crashsafe index is touched after a checkpoint. On a slave that
record could just mark the index invalid.

--
greg

On 2014-05-16 12:58:57 +0100, Greg Stark wrote:
> On Thu, May 15, 2014 at 8:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > One of the arguments against Bruce's proposal to print a warning at hash
> > index creation is that it's a particularly ineffective form of
> > deprecation. In your example, since the hash index was created by some
> > app not manually, I'll bet nobody would have seen/noticed the warning
> > even if there had been one.
>
> I suggested we make a GUC allow_unrecoverable_indexes and default it
> to false. If you want to create hash indexes you need to set it to
> true or else you just get errors.

Fine with me.

> A more general solution is to emit a WAL record the first time a
> non-crashsafe index is touched after a checkpoint. On a slave that
> record could just mark the index invalid.

Not trivially no. Recovery can't write to the catalog.

Greetings,

Andres Freund

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

Greg Stark <stark(at)mit(dot)edu> writes:
> On Thu, May 15, 2014 at 8:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> One of the arguments against Bruce's proposal to print a warning at hash
>> index creation is that it's a particularly ineffective form of
>> deprecation. In your example, since the hash index was created by some
>> app not manually, I'll bet nobody would have seen/noticed the warning
>> even if there had been one.

> I suggested we make a GUC allow_unrecoverable_indexes and default it
> to false. If you want to create hash indexes you need to set it to
> true or else you just get errors.

I still think this is throwing the error at the wrong place. People
will turn on the GUC the first time it gets in their way, and then
much later discover that the index doesn't work on a slave, and we'll
get a bug report exactly like this one. We need a check that is tightly
connected to actual unsafe usage, rather than basically-user-unfriendly
complaints at a point that's not doing anything unsafe. (Well, anything
more unsafe than it ever was.)

regards, tom lane

On Fri, May 16, 2014 at 2:45 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I still think this is throwing the error at the wrong place. People
> will turn on the GUC the first time it gets in their way, and then
> much later discover that the index doesn't work on a slave, and we'll
> get a bug report exactly like this one. We need a check that is tightly
> connected to actual unsafe usage, rather than basically-user-unfriendly
> complaints at a point that's not doing anything unsafe. (Well, anything
> more unsafe than it ever was.)

So there three cases where crash recovery is necessary:

1) A database crashes and does recovery when it starts up

2) A database has a hot backup taken and it's restored in a whole new place

3) A standby is replaying logs and has hot standby queries against it
or is activated and runs normally

The first two are actually safe as long as the index is quiescent.
That is, if the database crashes and there were no writes in progress
or the machine crashes and there were no writes since the last
checkpoint then the index should still be valid after a restart. A hot
backup should be fine if there were no writes to the index during the
hot backup. Obviously this is a precarious state of affairs but I
guess you're right that it's unchanged from the historical situation.

The index on the standby is a bit different. If the index has never
been touched since before the standby was created then it's ok (and
incidentally Heroku does see the occasional customer in that situation
-- they have hash indexes on archived partitions for data warehouse
queries). But if there's been a write ever since then even if it was a
long time ago the standby index is unusable. That's a lot more likely
than a backup taken while the index was being updated.

I take it you're arguing we should have an error in the standby if it
tries to use a hash index? That's seems reasonable. It might be good
to have a guc to override it (unfortunately we can't turn it into a
warning since it m

--
greg

On 2014-05-16 09:45:02 -0400, Tom Lane wrote:
> Greg Stark <stark(at)mit(dot)edu> writes:
> > On Thu, May 15, 2014 at 8:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> One of the arguments against Bruce's proposal to print a warning at hash
> >> index creation is that it's a particularly ineffective form of
> >> deprecation. In your example, since the hash index was created by some
> >> app not manually, I'll bet nobody would have seen/noticed the warning
> >> even if there had been one.
>
> > I suggested we make a GUC allow_unrecoverable_indexes and default it
> > to false. If you want to create hash indexes you need to set it to
> > true or else you just get errors.
>
> I still think this is throwing the error at the wrong place. People
> will turn on the GUC the first time it gets in their way, and then
> much later discover that the index doesn't work on a slave, and we'll
> get a bug report exactly like this one.

If there's one index within many as in this case it's likely to be
accidental, right? So I think such a GUC might have helped to prevent
the problem.

> We need a check that is tightly
> connected to actual unsafe usage, rather than basically-user-unfriendly
> complaints at a point that's not doing anything unsafe. (Well, anything
> more unsafe than it ever was.)

I agree that that'd be nicer, but I haven't seen a nice proposal of how
to do that yet.
The best I can think of is to WAL LOG the removal of the entire relation
the first time a hash index is used in a session, replacing it with a
metapage that errors out when used. That should be doable.

Greetings,

Andres Freund

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

On Fri, May 16, 2014 at 09:45:02AM -0400, Tom Lane wrote:
> Greg Stark <stark(at)mit(dot)edu> writes:
> > On Thu, May 15, 2014 at 8:25 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> One of the arguments against Bruce's proposal to print a warning at hash
> >> index creation is that it's a particularly ineffective form of
> >> deprecation. In your example, since the hash index was created by some
> >> app not manually, I'll bet nobody would have seen/noticed the warning
> >> even if there had been one.
>
> > I suggested we make a GUC allow_unrecoverable_indexes and default it
> > to false. If you want to create hash indexes you need to set it to
> > true or else you just get errors.
>
> I still think this is throwing the error at the wrong place. People
> will turn on the GUC the first time it gets in their way, and then
> much later discover that the index doesn't work on a slave, and we'll
> get a bug report exactly like this one. We need a check that is tightly
> connected to actual unsafe usage, rather than basically-user-unfriendly
> complaints at a point that's not doing anything unsafe. (Well, anything
> more unsafe than it ever was.)

Well, at this point we are decade into having crash-unsafe hash indexes,
and rather than getting better, the issue has gotten worse with
streaming replication. If we can't find the best way to warn people,
let's find _a_ way, at least.

I feel we are waiting for the calvary to come over the hill (and fix
hash indexes), except the calvary never arrives. At some point we have
to take ownership of the situation we are in and actively do something.

If someone today tried to add a crash-unsafe, replication-impotent
index, it would never be accepted, but because hash indexes came from
Berkeley, we go with a warning in the CREATE INDEX manual page and do
nothing more. I can't think of any other foot-gun feature that is
allowed to remain with so little user warning.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Fri, May 16, 2014 at 10:49 AM, Bruce Momjian [via PostgreSQL] <
ml-node+s1045698n5804229h23(at)n5(dot)nabble(dot)com> wrote:

> On Fri, May 16, 2014 at 09:45:02AM -0400, Tom Lane wrote:
>
> > Greg Stark <[hidden email]<http://user/SendEmail.jtp?type=node&node=5804229&i=0>>
> writes:
> > > On Thu, May 15, 2014 at 8:25 PM, Tom Lane <[hidden email]<http://user/SendEmail.jtp?type=node&node=5804229&i=1>>
> wrote:
> > >> One of the arguments against Bruce's proposal to print a warning at
> hash
> > >> index creation is that it's a particularly ineffective form of
> > >> deprecation. In your example, since the hash index was created by
> some
> > >> app not manually, I'll bet nobody would have seen/noticed the warning
> > >> even if there had been one.
> >
> > > I suggested we make a GUC allow_unrecoverable_indexes and default it
> > > to false. If you want to create hash indexes you need to set it to
> > > true or else you just get errors.
> >
> > I still think this is throwing the error at the wrong place. People
> > will turn on the GUC the first time it gets in their way, and then
> > much later discover that the index doesn't work on a slave, and we'll
> > get a bug report exactly like this one. We need a check that is tightly
> > connected to actual unsafe usage, rather than basically-user-unfriendly
> > complaints at a point that's not doing anything unsafe. (Well, anything
> > more unsafe than it ever was.)
>
> Well, at this point we are decade into having crash-unsafe hash indexes,
> and rather than getting better, the issue has gotten worse with
> streaming replication. If we can't find the best way to warn people,
> let's find _a_ way, at least.
>
> I feel we are waiting for the calvary to come over the hill (and fix
> hash indexes), except the calvary never arrives. At some point we have
> to take ownership of the situation we are in and actively do something.
>
> If someone today tried to add a crash-unsafe, replication-impotent
> index, it would never be accepted, but because hash indexes came from
> Berkeley, we go with a warning in the CREATE INDEX manual page and do
> nothing more. I can't think of any other foot-gun feature that is
> allowed to remain with so little user warning.
>
>
​+1

Unless there is a convincing argument to be made why doing it at CREATE
INDEX is FRIGGIN' EVIL then who really cares if its not perfect.

NOTICE: This index IS NOT WAL LOGGED and cannot be used on SLAVE servers or
AFTER RECOVERY. See Documentation for Details!

The goal should be to communicate FUD to the uninformed.

I'm all for additional and improved warnings in other places but this one
at least seems to have the benefit of being relatively simple to implement
and non-obnoxious since it only is issued once per index creation.

As devil's advocate it isn't like anyone is likely to intentionally use
hash indexes without reading the documentation first - if only to know they
exist, how they work, and what syntax to use. If an application is
installing such indexes for the user then a warning at CREATE INDEX is only
a little better than a warning in the documentation - though both are
likely to never be seen.

But that argument doesn't hold any sway for me. If someone knows they are
using a hash index intentionally then the notice/warning will be understood
and ignored while if someone is seeing the notice/warning and are not aware
of the limitations of hash indexes - like the OP - this live/logged notice
will hopefully cause them to become better informed and able to evaluate
their specific situation. If the application doesn't point out it is using
hash indexes then the typical user will not be checking PostgreSQL
documentation for the same; but just maybe the notice that is raised will
end up visible to the end-user or cause the application developers to
further re-examine their usage and/or documentation of hash indexes.

David J.

--
View this message in context: http://postgresql.1045698.n5.nabble.com/BUG-10329-Could-not-read-block-0-in-file-base-56100265-57047884-read-only-0-of-8192-bytes-tp5804037p5804238.html
Sent from the PostgreSQL - bugs mailing list archive at Nabble.com.

On Fri, May 16, 2014 at 08:27:33AM -0700, David G Johnston wrote:
> Well, at this point we are decade into having crash-unsafe hash indexes,
> and rather than getting better, the issue has gotten worse with
> streaming replication.  If we can't find the best way to warn people,
> let's find _a_ way, at least.
>
> I feel we are waiting for the calvary to come over the hill (and fix
> hash indexes), except the calvary never arrives.  At some point we have
> to take ownership of the situation we are in and actively do something.
>
> If someone today tried to add a crash-unsafe, replication-impotent
> index, it would never be accepted, but because hash indexes came from
> Berkeley, we go with a warning in the CREATE INDEX manual page and do
> nothing more.  I can't think of any other foot-gun feature that is
> allowed to remain with so little user warning.
>
> Unless there is a convincing argument to be made why doing it at CREATE INDEX
> is FRIGGIN' EVIL then who really cares if its not perfect.
>
> NOTICE: This index IS NOT WAL LOGGED and cannot be used on SLAVE servers or
> AFTER RECOVERY.  See Documentation for Details!
>
> The goal should be to communicate FUD to the uninformed.
>
> I'm all for additional and improved warnings in other places but this one at
> least seems to have the benefit of being relatively simple to implement and
> non-obnoxious since it only is issued once per index creation.
>
> As devil's advocate it isn't like anyone is likely to intentionally use hash
> indexes without reading the documentation first - if only to know they exist,
> how they work, and what syntax to use.  If an application is installing such
> indexes for the user then a warning at CREATE INDEX is only a little better
> than a warning in the documentation - though both are likely to never be seen.
>  
>
> But that argument doesn't hold any sway for me. If someone knows they are using
> a hash index intentionally then the notice/warning will be understood and
> ignored while if someone is seeing the notice/warning and are not aware of the
> limitations of hash indexes - like the OP - this live/logged notice will
> hopefully cause them to become better informed and able to evaluate their
> specific situation.  If the application doesn't point out it is using hash
> indexes then the typical user will not be checking PostgreSQL documentation for
> the same; but just maybe the notice that is raised will end up visible to the
> end-user or cause the application developers to further re-examine their usage
> and/or documentation of hash indexes.

Here is a patch which implements the warning during CREATE INDEX ...
HASH. If WAL-logging of hash indexes is ever implemented, we can remove
this warning.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian wrote:

> Here is a patch which implements the warning during CREATE INDEX ...
> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
> this warning.

I think we should have CREATE UNLOGGED INDEX, and simply disallow any
hash index from being created unless it's marked as such.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Sat, Sep 6, 2014 at 09:42:45PM -0400, Alvaro Herrera wrote:
> Bruce Momjian wrote:
>
> > Here is a patch which implements the warning during CREATE INDEX ...
> > HASH. If WAL-logging of hash indexes is ever implemented, we can remove
> > this warning.
>
> I think we should have CREATE UNLOGGED INDEX, and simply disallow any
> hash index from being created unless it's marked as such.

Wow, that sounds much more radical than we discussed. Seeing I got
push-back just for the warning, I don't see how disabling "logged" WAL
indexes is going to be accepted.

It is a good idea, though. :-)

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Sat, Sep 6, 2014 at 11:07:43AM -0400, Bruce Momjian wrote:
> Here is a patch which implements the warning during CREATE INDEX ...
> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
> this warning.

Applied, though I used the term "streaming standbys" to match our docs.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Sat, Sep 6, 2014 at 11:07:43AM -0400, Bruce Momjian wrote:
>> Here is a patch which implements the warning during CREATE INDEX ...
>> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
>> this warning.
>
> Applied, though I used the term "streaming standbys" to match our docs.

Hmm. The wording of the warning doesn't seem to really indicate
the full scope of the limitation. Any a standby (warm or hot)
maintained by WAL file copying would also be affected (i.e.,
streaming replication as the WAL delivery mechanism is irrelevant),
and you also have problems after a database crash or PANIC. I'm
not sure how to state that concisely, though.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, Sep 10, 2014 at 02:07:36PM -0700, Kevin Grittner wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > On Sat, Sep 6, 2014 at 11:07:43AM -0400, Bruce Momjian wrote:
> >> Here is a patch which implements the warning during CREATE INDEX ...
> >> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
> >> this warning.
> >
> > Applied, though I used the term "streaming standbys" to match our docs.
>
> Hmm. The wording of the warning doesn't seem to really indicate
> the full scope of the limitation. Any a standby (warm or hot)
> maintained by WAL file copying would also be affected (i.e.,
> streaming replication as the WAL delivery mechanism is irrelevant),
> and you also have problems after a database crash or PANIC. I'm
> not sure how to state that concisely, though.

I am open to improved wording. :-)

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> On Wed, Sep 10, 2014 at 02:07:36PM -0700, Kevin Grittner wrote:

>> Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>>> On Sat, Sep 6, 2014 at 11:07:43AM -0400, Bruce Momjian wrote:
>>>> Here is a patch which implements the warning during CREATE INDEX
> ...
>>>> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
>>>> this warning.
>>>
>>> Applied, though I used the term "streaming standbys" to match our docs.
>>
>> Hmm. The wording of the warning doesn't seem to really indicate
>> the full scope of the limitation. Any a standby (warm or hot)
>> maintained by WAL file copying would also be affected (i.e.,
>> streaming replication as the WAL delivery mechanism is irrelevant),
>> and you also have problems after a database crash or PANIC. I'm
>> not sure how to state that concisely, though.
>
> I am open to improved wording. :-)

Instead of:

hash indexes are not WAL-logged so they are not crash-safe and cannot be used on streaming standbys

how about?:

hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers

That seems consistent with the terminology used here:

http://www.postgresql.org/docs/current/interactive/warm-standby.html

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 09/08/2014 03:45 PM, Bruce Momjian wrote:
> On Sat, Sep 6, 2014 at 09:42:45PM -0400, Alvaro Herrera wrote:
>> Bruce Momjian wrote:
>>
>>> Here is a patch which implements the warning during CREATE INDEX ...
>>> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
>>> this warning.
>>
>> I think we should have CREATE UNLOGGED INDEX, and simply disallow any
>> hash index from being created unless it's marked as such.
>
> Wow, that sounds much more radical than we discussed. Seeing I got
> push-back just for the warning, I don't see how disabling "logged" WAL
> indexes is going to be accepted.
>
> It is a good idea, though. :-)

I agree there - implementing CREATE UNLOGGED INDEX and use THAT for hash
indexes seems like a fairly clean thing to me, hash indexes _are_
unlogged so lets reflect that directly.
I could even envision pg_dump doing that conversion automatically...

Stefan

On 2014-09-11 19:25:13 +0200, Stefan Kaltenbrunner wrote:
> On 09/08/2014 03:45 PM, Bruce Momjian wrote:
> > On Sat, Sep 6, 2014 at 09:42:45PM -0400, Alvaro Herrera wrote:
> >> Bruce Momjian wrote:
> >>
> >>> Here is a patch which implements the warning during CREATE INDEX ...
> >>> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
> >>> this warning.
> >>
> >> I think we should have CREATE UNLOGGED INDEX, and simply disallow any
> >> hash index from being created unless it's marked as such.
> >
> > Wow, that sounds much more radical than we discussed. Seeing I got
> > push-back just for the warning, I don't see how disabling "logged" WAL
> > indexes is going to be accepted.
> >
> > It is a good idea, though. :-)
>
> I agree there - implementing CREATE UNLOGGED INDEX and use THAT for hash
> indexes seems like a fairly clean thing to me, hash indexes _are_
> unlogged so lets reflect that directly.
> I could even envision pg_dump doing that conversion automatically...

I think this did came up as a solution before. It's just that nobody
found a reasonably easy and clean way to do unlogged indexes on logged
tables so far. It's far from trivial.

Greetings,

Andres Freund

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

On Wed, Sep 10, 2014 at 02:54:39PM -0700, Kevin Grittner wrote:
> > I am open to improved wording. :-)
>
> Instead of:
>
> hash indexes are not WAL-logged so they are not crash-safe and cannot be used on streaming standbys
>
> how about?:
>
> hash indexes are not WAL-logged and thus are not crash-safe and cannot be used on standby servers
>
> That seems consistent with the terminology used here:
>
> http://www.postgresql.org/docs/current/interactive/warm-standby.html

Change applied. Thanks.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On Thu, Sep 11, 2014 at 07:29:23PM +0200, Andres Freund wrote:
> On 2014-09-11 19:25:13 +0200, Stefan Kaltenbrunner wrote:
> > On 09/08/2014 03:45 PM, Bruce Momjian wrote:
> > > On Sat, Sep 6, 2014 at 09:42:45PM -0400, Alvaro Herrera wrote:
> > >> Bruce Momjian wrote:
> > >>
> > >>> Here is a patch which implements the warning during CREATE INDEX ...
> > >>> HASH. If WAL-logging of hash indexes is ever implemented, we can remove
> > >>> this warning.
> > >>
> > >> I think we should have CREATE UNLOGGED INDEX, and simply disallow any
> > >> hash index from being created unless it's marked as such.
> > >
> > > Wow, that sounds much more radical than we discussed. Seeing I got
> > > push-back just for the warning, I don't see how disabling "logged" WAL
> > > indexes is going to be accepted.
> > >
> > > It is a good idea, though. :-)
> >
> > I agree there - implementing CREATE UNLOGGED INDEX and use THAT for hash
> > indexes seems like a fairly clean thing to me, hash indexes _are_
> > unlogged so lets reflect that directly.
> > I could even envision pg_dump doing that conversion automatically...
>
> I think this did came up as a solution before. It's just that nobody
> found a reasonably easy and clean way to do unlogged indexes on logged
> tables so far. It's far from trivial.

And practically, how would we implement this for upgrades? Would we have
pg_dump emit UNLOGGED for any hash creation command? That seems to
defeat the purpose of this.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

On 2014-09-11 13:41:37 -0400, Bruce Momjian wrote:
> > > I agree there - implementing CREATE UNLOGGED INDEX and use THAT for hash
> > > indexes seems like a fairly clean thing to me, hash indexes _are_
> > > unlogged so lets reflect that directly.
> > > I could even envision pg_dump doing that conversion automatically...
> >
> > I think this did came up as a solution before. It's just that nobody
> > found a reasonably easy and clean way to do unlogged indexes on logged
> > tables so far. It's far from trivial.
>
> And practically, how would we implement this for upgrades? Would we have
> pg_dump emit UNLOGGED for any hash creation command?

That seems like an almost trivial problem in comparison to the actual
difficulty of implementing UNLOGGED indexed on LOGGED tables. Yes, I
think forbidding unlogged hash tables + teaching pg_dump a heuristic to
treat any < 9.x hash index as unlogged would be ok.

> That seems to defeat the purpose of this.

Why? It makes hash indexes usable for the cases where it's safe to do
so. Great! It also adds a feature which is really interesting for other
types of indexes.

Greetings,

Andres Freund

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

On 09/11/2014 07:46 PM, Andres Freund wrote:
> On 2014-09-11 13:41:37 -0400, Bruce Momjian wrote:
>>>> I agree there - implementing CREATE UNLOGGED INDEX and use THAT for hash
>>>> indexes seems like a fairly clean thing to me, hash indexes _are_
>>>> unlogged so lets reflect that directly.
>>>> I could even envision pg_dump doing that conversion automatically...
>>>
>>> I think this did came up as a solution before. It's just that nobody
>>> found a reasonably easy and clean way to do unlogged indexes on logged
>>> tables so far. It's far from trivial.
>>
>> And practically, how would we implement this for upgrades? Would we have
>> pg_dump emit UNLOGGED for any hash creation command?
>
> That seems like an almost trivial problem in comparison to the actual
> difficulty of implementing UNLOGGED indexed on LOGGED tables. Yes, I
> think forbidding unlogged hash tables + teaching pg_dump a heuristic to
> treat any < 9.x hash index as unlogged would be ok.

yeah that is what I would vote for as well, and this is way cleaner than
emitting a textual warning.

>
>> That seems to defeat the purpose of this.
>
> Why? It makes hash indexes usable for the cases where it's safe to do
> so. Great! It also adds a feature which is really interesting for other
> types of indexes.

Definitly a feature that is interesting for other types of indexes, it
would also lower the barrier for adding new index types to pg or at
least make it cleaner for people playing with new ideas ones.

Stefan

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-09-11 13:41:37 -0400, Bruce Momjian wrote:
>> And practically, how would we implement this for upgrades? Would we have
>> pg_dump emit UNLOGGED for any hash creation command?

> That seems like an almost trivial problem in comparison to the actual
> difficulty of implementing UNLOGGED indexed on LOGGED tables.

Yeah, we'd need to somehow mark such indexes as INVALID during crash
recovery, which sounds pretty messy. We've stayed away from the whole
idea of automatically-applied catalog updates during recovery, for
good reasons.

> Yes, I
> think forbidding unlogged hash tables + teaching pg_dump a heuristic to
> treat any < 9.x hash index as unlogged would be ok.

My own opinion, if we had the infrastructure, is that "CREATE INDEX
... USING hash" should result in (1) automatically adding the UNLOGGED
attribute and (2) issuing a NOTICE or WARNING stating that we've done so.
Throwing an error, when we do not do so today, is just being a nag.
This approach also eliminates the need to do anything special in pg_dump,
which is a good thing since we have no way to retroactively fix existing
dump files.

regards, tom lane

On 2014-09-11 14:58:13 -0400, Tom Lane wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > On 2014-09-11 13:41:37 -0400, Bruce Momjian wrote:
> >> And practically, how would we implement this for upgrades? Would we have
> >> pg_dump emit UNLOGGED for any hash creation command?
>
> > That seems like an almost trivial problem in comparison to the actual
> > difficulty of implementing UNLOGGED indexed on LOGGED tables.
>
> Yeah, we'd need to somehow mark such indexes as INVALID during crash
> recovery, which sounds pretty messy. We've stayed away from the whole
> idea of automatically-applied catalog updates during recovery, for
> good reasons.

Right. There were some ideas floating around, but nothing agreed upon.

I think we can stay away from doing anything *during* recovery, because
surely nothing UNLOGGED will ever be usable during it. So we should just
skip unlogged indexes during planning/executing, the same as !indisvalid/!indisready indexes
are skipped during planning/DML.

But obviously there has to be some way to mark them as invalid at the
end of recovery. I don't think we want to do a disk access during
planning to see if there's some special file system marker marking a
relation as reset everytime a unlogged index is involed in a plan.
There's enough ugly ways to deal with that, but a acceptable one ...

> > Yes, I
> > think forbidding unlogged hash tables + teaching pg_dump a heuristic to
> > treat any < 9.x hash index as unlogged would be ok.
>
> My own opinion, if we had the infrastructure, is that "CREATE INDEX
> ... USING hash" should result in (1) automatically adding the UNLOGGED
> attribute and (2) issuing a NOTICE or WARNING stating that we've done so.
> Throwing an error, when we do not do so today, is just being a nag.
> This approach also eliminates the need to do anything special in pg_dump,
> which is a good thing since we have no way to retroactively fix existing
> dump files.

I think that might be less pain in the short term, but will bite us
*and* our users afterwards. For one, it really really isn't the same
thing to have a unlogged index as a logged index. Any user using
UNLOGGED indexes will need a explicit way to trigger rebuilding them
after a crash-restart. They're not served well by us automatically
hiding that fact from them. For another I think we at some point we're
going to get hash indexes that are logged and then we'll have to
uninstall that heuristic which will just confuse users agin.

Greetings,

Andres Freund

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