Re: Patch: Write Amplification Reduction Method (WARM)
Andres Freund <andres@anarazel.de>
From: Andres Freund <andres@anarazel.de>
To: Pavan Deolasee <pavan.deolasee@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Alvaro Herrera <alvherre@2ndquadrant.com>, Bruce Momjian <bruce@momjian.us>, Jaime Casanova <jaime.casanova@2ndquadrant.com>, Haribabu Kommi <kommi.haribabu@gmail.com>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-04-10T23:34:50Z
Lists: pgsql-hackers
Hi, On 2017-04-08 23:36:13 +0530, Pavan Deolasee wrote: > On Wed, Apr 5, 2017 at 11:57 PM, Andres Freund <andres@anarazel.de> wrote: > > > On 2017-04-05 09:36:47 -0400, Robert Haas wrote: > > > By the way, the "Converting WARM chains back to HOT chains" section of > > > README.WARM seems to be out of date. Any chance you could update that > > > to reflect the current state and thinking of the patch? > > > > I propose we move this patch to the next CF. That shouldn't prevent you > > working on it, although focusing on review of patches that still might > > make it wouldn't hurt either. > > > > > Thank you all for the reviews, feedback, tests, criticism. And apologies > for keep pushing it till the last minute even though it was clear to me > quite some time back the patch is not going to make it. What confuses me about that position is that people were advocating to actually commit till literally hours before the CF closed. > But if I'd given > up, it would have never received whatever little attention it got. The only > thing that disappoints me is that the patch was held back on no strong > technical grounds - at least none were clear to me. There were concerns > about on-disk changes etc, but most on-disk changes were known for 7 months > now. Reminds me of HOT development, when it would not receive adequate > feedback for quite many months, probably for very similar reasons - complex > patch, changes on-disk format, risky, even though performance gains were > quite substantial. I was much more hopeful this time because we have many > more experts now as compared to then, but we probably have equally more > amount of complex patches to review/commit. I don't think it's realistic to expect isolated in-depth review of on-disk changes, when the rest of the patch isn't in a close-to-ready shape. The likelihood that further work on the patch invalidates such in-depth review is significant. It's not like only minor details changed in the last few months. I do agree that it's hard to get qualified reviewers on bigger patches. But I think part of the reaction to that has to be active work on that front: If your patch needs reviews by committers or other topical experts, you need to explicitly reach out. There's a lot of active threads, and nobody has time to follow all of them in sufficient detail to know that certain core parts of an actively developed patch are ready for review. Offer tit-for-tat reviews. Announce that your patch is ready, that you're only waiting for review. Post a summary of open questions... > Finally, my apologies for not spending enough time reviewing other > patches. I know its critical, and I'll try to improve on that. I do find it a more than a bit ironic to lament early lack of attention to your patch, while also being aware of not having done much review. This can only scale if everyone reviews each others patches, not if there's a few individuals that have to review everyones patches. Greetings, Andres Freund
Commits
-
Implement SortSupport for macaddr data type
- f90d23d0c518 10.0 cited
-
Simplify check of modified attributes in heap_update
- 2fd8685e7fd9 10.0 landed
-
Remove direct uses of ItemPointer.{ip_blkid,ip_posid}
- ce96ce60ca22 10.0 landed
-
Fix CatalogTupleInsert/Update abstraction for case of shared indstate.
- aedd554f84bb 10.0 landed
-
Provide CatalogTupleDelete() as a wrapper around simple_heap_delete().
- ab02896510e2 10.0 landed
-
Band-aid fix for incorrect use of view options as StdRdOptions.
- e3e66d8a9813 10.0 cited
-
Update visibility map in the second phase of vacuum.
- fdf9e21196a6 9.3.0 cited
-
Avoid having two copies of the HOT-chain search logic.
- 4da99ea4231e 9.2.0 cited
-
Postgres95 1.01 Distribution - Virgin Sources
- d31084e9d111 7.1.1 cited