Re: [WIP] Effective storage of duplicates in B-tree index.
Claudio Freire <klaussfreire@gmail.com>
On Thu, Mar 24, 2016 at 7:12 PM, Peter Geoghegan <pg@heroku.com> wrote: > On Thu, Mar 24, 2016 at 7:17 AM, Robert Haas <robertmhaas@gmail.com> wrote: >> I really like this idea, and the performance results seem impressive, >> but I think we should push this out to 9.7. A btree patch that didn't >> have WAL support until two and a half weeks into the final CommitFest >> just doesn't seem to me like a good candidate. First, as a general >> matter, if a patch isn't code-complete at the start of a CommitFest, >> it's reasonable to say that it should be reviewed but not necessarily >> committed in that CommitFest. This patch has had some review, but I'm >> not sure how deep that review is, and I think it's had no code review >> at all of the WAL logging changes, which were submitted only a week >> ago, well after the CF deadline. Second, the btree AM is a >> particularly poor place to introduce possibly destabilizing changes. >> Everybody depends on it, all the time, for everything. And despite >> new tools like amcheck, it's not a particularly easy thing to debug. > > Regrettably, I must agree. I don't see a plausible path to commit for > this patch in the ongoing CF. > > I think that Anastasia did an excellent job here, and I wish I could > have been of greater help sooner. Nevertheless, it would be unwise to > commit this given the maturity of the code. There have been very few > instances of performance improvements to the B-Tree code for as long > as I've been interested, because it's so hard, and the standard is so > high. The only example I can think of from the last few years is > Kevin's commit 2ed5b87f96 and Tom's commit 1a77f8b63d both of which > were far less invasive, and Simon's commit c7111d11b1, which we just > outright reverted from 9.5 due to subtle bugs (and even that was > significantly less invasive than this patch). Improving nbtree is > something that requires several rounds of expert review, and that's > something that's in short supply for the B-Tree code in particular. I > think that a new testing strategy is needed to make this easier, and I > hope to get that going with amcheck. I need help with formalizing a > "testing first" approach for improving the B-Tree code, because I > think it's the only way that we can move forward with projects like > this. It's *incredibly* hard to push forward patches like this given > our current, limited testing strategy. I've been toying (having gotten nowhere concrete really) with prefix compression myself, I agree that messing with btree code is quite harder than it ought to be. Perhaps trying experimental format changes in a separate experimental am wouldn't be all that bad (say, nxbtree?). People could opt-in to those, by creating the indexes with nxbtree instead of plain btree (say in development environments) and get some testing going without risking much. Normally the same effect should be achievable with mere flags, but since format changes to btree tend to be rather invasive, ensuring the patch doesn't change behavior with the flag off is hard as well, hence the wholly separate am idea.
Commits
-
Teach pageinspect about nbtree deduplication.
- 93ee38eade1b 13.0 landed
-
Doc: Fix deduplicate_items index term.
- e537aed61db7 13.0 landed
-
Revise BTP_HAS_GARBAGE nbtree VACUUM comments.
- 4b25f5d0ba01 13.0 cited
-
Remove unneeded "pin scan" nbtree VACUUM code.
- 9f83468b3536 13.0 landed
-
Cleanup code in reloptions.h regarding reloption handling
- 50d22de9325f 13.0 cited
-
Catch invalid typlens in a couple of places
- 8557a6f10ca6 13.0 cited
-
Compute XID horizon for page level index vacuum on primary.
- 558a9165e081 12.0 cited
-
Make heap TID a tiebreaker nbtree index column.
- dd299df8189b 12.0 cited
-
Avoid pin scan for replay of XLOG_BTREE_VACUUM in all cases
- 3e4b7d87988f 9.6.0 cited
-
Revert buggy optimization of index scans
- c7111d11b188 9.6.0 cited
-
Restructure index access method API to hide most of it at the C level.
- 65c5fcd353a8 9.6.0 cited
-
Reduce pinning and buffer content locking for btree scans.
- 2ed5b87f96d4 9.5.0 cited
-
Avoid scanning nulls at the beginning of a btree index scan.
- 1a77f8b63d15 9.2.0 cited