Обсуждение: [COMMITTERS] pgsql: hash: Add write-ahead logging support.
hash: Add write-ahead logging support. The warning about hash indexes not being write-ahead logged and their use being discouraged has been removed. "snapshot too old" is now supported for tables with hash indexes. Most importantly, barring bugs, hash indexes will now be crash-safe and usable on standbys. This commit doesn't yet add WAL consistency checking for hash indexes, as we now have for other index types; a separate patch has been submitted to cure that lack. Amit Kapila, reviewed and slightly modified by me. The larger patch series of which this is a part has been reviewed and tested by Álvaro Herrera, Ashutosh Sharma, Mark Kirkwood, Jeff Janes, and Jesper Pedersen. Discussion: http://postgr.es/m/CAA4eK1JOBX=YU33631Qh-XivYXtPSALh514+jR8XeD7v+K3r_Q@mail.gmail.com Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/c11453ce0aeaa377cbbcc9a3fc418acb94629330 Modified Files -------------- contrib/pageinspect/expected/hash.out | 1 - contrib/pgstattuple/expected/pgstattuple.out | 2 - doc/src/sgml/backup.sgml | 13 - doc/src/sgml/config.sgml | 7 +- doc/src/sgml/high-availability.sgml | 6 - doc/src/sgml/indices.sgml | 12 - doc/src/sgml/ref/create_index.sgml | 13 - src/backend/access/hash/Makefile | 2 +- src/backend/access/hash/README | 138 +++- src/backend/access/hash/hash.c | 81 ++- src/backend/access/hash/hash_xlog.c | 963 +++++++++++++++++++++++++ src/backend/access/hash/hashinsert.c | 59 +- src/backend/access/hash/hashovfl.c | 209 +++++- src/backend/access/hash/hashpage.c | 236 +++++- src/backend/access/hash/hashsearch.c | 5 + src/backend/access/rmgrdesc/hashdesc.c | 134 +++- src/backend/commands/indexcmds.c | 5 - src/backend/utils/cache/relcache.c | 12 +- src/include/access/hash_xlog.h | 232 ++++++ src/test/regress/expected/create_index.out | 5 - src/test/regress/expected/enum.out | 1 - src/test/regress/expected/hash_index.out | 2 - src/test/regress/expected/macaddr.out | 1 - src/test/regress/expected/replica_identity.out | 1 - src/test/regress/expected/uuid.out | 1 - 25 files changed, 2001 insertions(+), 140 deletions(-)
Robert Haas <rhaas@postgresql.org> writes:
> hash: Add write-ahead logging support.
This bit in access/hash/README, lines 368ff, appears obsolete:
Although we can survive a failure to split a bucket, a crash is likely to
corrupt the index, since hash indexes are not yet WAL-logged.
Perhaps we can just delete that para. If not, what should it say now?
regards, tom lane
On Tue, Mar 14, 2017 at 1:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <rhaas@postgresql.org> writes: >> hash: Add write-ahead logging support. > > This bit in access/hash/README, lines 368ff, appears obsolete: > > Although we can survive a failure to split a bucket, a crash is likely to > corrupt the index, since hash indexes are not yet WAL-logged. > > Perhaps we can just delete that para. If not, what should it say now? That's obsolete, and can just be removed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Mar 14, 2017 at 1:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <rhaas@postgresql.org> writes:
>>> hash: Add write-ahead logging support.
>> This bit in access/hash/README, lines 368ff, appears obsolete:
>>
>> Although we can survive a failure to split a bucket, a crash is likely to
>> corrupt the index, since hash indexes are not yet WAL-logged.
>>
>> Perhaps we can just delete that para. If not, what should it say now?
> That's obsolete, and can just be removed.
You gonna do it, or you want me to?
regards, tom lane
On Tue, Mar 14, 2017 at 2:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Mar 14, 2017 at 1:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Robert Haas <rhaas@postgresql.org> writes: >>>> hash: Add write-ahead logging support. > >>> This bit in access/hash/README, lines 368ff, appears obsolete: >>> >>> Although we can survive a failure to split a bucket, a crash is likely to >>> corrupt the index, since hash indexes are not yet WAL-logged. >>> >>> Perhaps we can just delete that para. If not, what should it say now? > >> That's obsolete, and can just be removed. > > You gonna do it, or you want me to? If you're feeling motivated, have at it. Otherwise, I'll do it in a day or two after I see what other complaints get reported that might be addressed at the same time. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Mar 14, 2017 at 2:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> You gonna do it, or you want me to?
> If you're feeling motivated, have at it. Otherwise, I'll do it in a
> day or two after I see what other complaints get reported that might
> be addressed at the same time.
Nah, long as it's on your to-do list that's good enough for me.
regards, tom lane
On Tue, Mar 14, 2017 at 11:26 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Mar 14, 2017 at 1:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Robert Haas <rhaas@postgresql.org> writes: >>> hash: Add write-ahead logging support. >> >> This bit in access/hash/README, lines 368ff, appears obsolete: >> >> Although we can survive a failure to split a bucket, a crash is likely to >> corrupt the index, since hash indexes are not yet WAL-logged. >> >> Perhaps we can just delete that para. If not, what should it say now? > > That's obsolete, and can just be removed. > Agreed and posted a patch on hackers [1] [1] - https://www.postgresql.org/message-id/CAA4eK1LeVu8rSvA%3DhfbezqjNv5x3nU-rPbHCHHbbsCrLX-0zAg%40mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Wed, Mar 15, 2017 at 3:23 PM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Tue, Mar 14, 2017 at 11:26 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> On Tue, Mar 14, 2017 at 1:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> Robert Haas <rhaas@postgresql.org> writes: >>>> hash: Add write-ahead logging support. >>> >>> This bit in access/hash/README, lines 368ff, appears obsolete: >>> >>> Although we can survive a failure to split a bucket, a crash is likely to >>> corrupt the index, since hash indexes are not yet WAL-logged. >>> >>> Perhaps we can just delete that para. If not, what should it say now? >> >> That's obsolete, and can just be removed. >> > > Agreed and posted a patch on hackers [1] Also the obsolete warning message appears in .po files and the mb tests: $ git grep 'and their use is discouraged' src/backend/po/de.po:msgid "hash indexes are not WAL-logged and their use is discouraged" src/backend/po/es.po:msgid "hash indexes are not WAL-logged and their use is discouraged" src/backend/po/fr.po:msgid "hash indexes are not WAL-logged and their use is discouraged" src/backend/po/it.po:msgid "hash indexes are not WAL-logged and their use is discouraged" src/backend/po/pl.po:msgid "hash indexes are not WAL-logged and their use is discouraged" src/backend/po/ru.po:msgid "hash indexes are not WAL-logged and their use is discouraged" src/backend/po/zh_CN.po:msgid "hash indexes are not WAL-logged and their use is discouraged" src/test/mb/expected/big5.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/euc_jp.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/euc_kr.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/euc_tw.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/gb18030.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/mule_internal.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/mule_internal.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/sjis.out:WARNING: hash indexes are not WAL-logged and their use is discouraged src/test/mb/expected/utf8.out:WARNING: hash indexes are not WAL-logged and their use is discouraged -- Thomas Munro http://www.enterprisedb.com
Thomas Munro <thomas.munro@enterprisedb.com> writes:
> Also the obsolete warning message appears in .po files and the mb tests:
The .po files won't get updated until the next import from the translation
repo. No need to worry about them.
However, we probably do need to fix the src/test/mb/expected/ files.
Has anyone tried running that test suite?
regards, tom lane
On Wed, Mar 15, 2017 at 8:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Thomas Munro <thomas.munro@enterprisedb.com> writes: >> Also the obsolete warning message appears in .po files and the mb tests: > > The .po files won't get updated until the next import from the translation > repo. No need to worry about them. > > However, we probably do need to fix the src/test/mb/expected/ files. > Has anyone tried running that test suite? > I haven't tried. Does this run as part of any regression tests or is the only way is run it manually (sh mbregress.sh)? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Amit Kapila <amit.kapila16@gmail.com> writes:
> On Wed, Mar 15, 2017 at 8:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> However, we probably do need to fix the src/test/mb/expected/ files.
>> Has anyone tried running that test suite?
> I haven't tried. Does this run as part of any regression tests or is
> the only way is run it manually (sh mbregress.sh)?
I don't believe it's connected up to any top-level test target ---
if it were, the buildfarm would likely have noticed this.
regards, tom lane
On Wed, Mar 15, 2017 at 9:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Amit Kapila <amit.kapila16@gmail.com> writes: >> On Wed, Mar 15, 2017 at 8:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> However, we probably do need to fix the src/test/mb/expected/ files. >>> Has anyone tried running that test suite? > >> I haven't tried. Does this run as part of any regression tests or is >> the only way is run it manually (sh mbregress.sh)? > > I don't believe it's connected up to any top-level test target --- > if it were, the buildfarm would likely have noticed this. > I also think so. In any case, I have run those now and they are failing as expected. I will send a patch to fix these tests. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
On Wed, Mar 15, 2017 at 9:44 AM, Amit Kapila <amit.kapila16@gmail.com> wrote: > On Wed, Mar 15, 2017 at 9:18 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Amit Kapila <amit.kapila16@gmail.com> writes: >>> On Wed, Mar 15, 2017 at 8:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>>> However, we probably do need to fix the src/test/mb/expected/ files. >>>> Has anyone tried running that test suite? >> >>> I haven't tried. Does this run as part of any regression tests or is >>> the only way is run it manually (sh mbregress.sh)? >> >> I don't believe it's connected up to any top-level test target --- >> if it were, the buildfarm would likely have noticed this. >> > > I also think so. In any case, I have run those now and they are > failing as expected. I will send a patch to fix these tests. > Attached are two patches, one to fix multi-byte regression tests and another is spurious function reference in one of the comments in hash index code. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com