Re: WIP: Access method extendability

From: Jim Nasby <Jim(dot)Nasby(at)BlueTreble(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: WIP: Access method extendability
Date: 2016-02-02 01:09:15
Message-ID: 20160202010915.1291.94510.pgcf@coridan.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, failed
Spec compliant: not tested
Documentation: not tested

There are currently no docs or unit tests. I suspect this patch is still WIP?

create-am.5.patch:
General notes:
Needs catversion bump.

IndexQualInfo and GenericCosts have been moved to src/include/utils/selfuncs.h.

METHOD becomes an unreserved keyword.

generic-xlog.5.patch:
generic_xlog.c: At least needs a bunch of explanatory comments, if not info in a README. Since I don't really understand what the design here is my review is just cursory.

static memoryMove() - seems like an overly-generic function name to me...

writeCopyFlagment(), writeMoveFlagment(): s/Fl/Fr/?

bloom-control.5:
README:
+ CREATE INDEX bloomidx ON tbloom(i1,i2,i3)
+ WITH (length=5, col1=2, col2=2, col3=4);
+
+ Here, we create bloom index with signature length 80 bits and attributes
+ i1, i2 mapped to 2 bits, attribute i3 - to 4 bits.

It's not clear to me where 80 bits is coming from...
bloom.h:
+ #define BITBYTE (8)
ISTR seeing this defined somewhere in the Postgres headers; dunno if it's worth using that definition instead.

Testing:
I ran the SELECT INTO from the README, as well as CREATE INDEX bloomidx. I then ran

insert into tbloom select
(generate_series(1,1000)*random())::int as i1,
(generate_series(1,1000)*random())::int as i2,
(generate_series(1,1000)*random())::int as i3,
(generate_series(1,1000)*random())::int as i4,
(generate_series(1,1000)*random())::int as i5,
(generate_series(1,1000)*random())::int as i6,
(generate_series(1,1000)*random())::int as i7,
(generate_series(1,1000)*random())::int as i8,
(generate_series(1,1000)*random())::int as i9,
(generate_series(1,1000)*random())::int as i10,
(generate_series(1,1000)*random())::int as i11,
(generate_series(1,1000)*random())::int as i12,
(generate_series(1,1000)*random())::int as i13
from generate_series(1,999);

and kill -9'd the backend. After restart I did VACUUM VERBOSE without issue. I ran the INSERT INTO, kill -9 and VACUUM VERBOSE sequence again. This time I got an assert:

TRAP: FailedAssertion("!(((bool) (((const void*)((ItemPointer) left) != ((void*)0)) && (((ItemPointer) left)->ip_posid != 0))))", File: "vacuumlazy.c", Line: 1823)

That line is

lblk = ItemPointerGetBlockNumber((ItemPointer) left);

which does

AssertMacro(ItemPointerIsValid(pointer)), \

which is the assert that's failing.

I've squirreled this install away for now, in case you can't repro this failure.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2016-02-02 01:15:14 Re: Freeze avoidance of very large table.
Previous Message Curtis Ruck 2016-02-02 01:05:46 PostgreSQL Auditing