Re: review: autovacuum_work_mem

On Mon, Nov 18, 2013 at 11:36 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> Please reply to the original thread in future (even if the Reply-to
> Message-ID is the same, I see this as a separate thread).
>
sorry about that, when i added "review" to the subject gmail removed
the thread info.
for reference the original thread started here:
<http://www.postgresql.org/message-id/CAM3SWZTwLA8Ef2DTvbwM1b1zEVU_eN3N4rReGNU5_zFyjNGi6w@mail.gmail.com>

>
> Revision attached.
> --
> Peter Geoghegan

Review for Peter Geoghegan's v2 patch in CF 2013-11:
https://commitfest.postgresql.org/action/patch_view?id=1262

Submission review
-----------------
* Is the patch in a patch format which has context?
Yes

* Does it apply cleanly to the current git master
(04eee1fa9ee80dabf7cf4b8b9106897272e9b291)?
patching file src/backend/commands/vacuumlazy.c
Hunk #2 succeeded at 1582 (offset 1 line).

* Does it include reasonable tests, necessary doc patches, etc?
Documentation patches included.
No additional tests.

Usability review
-----------------
* Does the patch actually implement that?
Yes.

* Do we want that?
Yes. The original thread has references, see
<http://www.postgresql.org/message-id/CAM3SWZTwLA8Ef2DTvbwM1b1zEVU_eN3N4rReGNU5_zFyjNGi6w@mail.gmail.com>

* Do we already have it?
No.

* Does it follow SQL spec, or the community-agreed behavior?
SQL spec: n/a
community: Yes. The original thread has references, see
<http://www.postgresql.org/message-id/CAM3SWZTwLA8Ef2DTvbwM1b1zEVU_eN3N4rReGNU5_zFyjNGi6w@mail.gmail.com>

* Does it include pg_dump support (if applicable)?
n/a

Feature test
-----------------
* Does the feature work as advertised?
Yes.

* Are there corner cases the author has failed to consider?
None that i can see.

* Are there any assertion failures or crashes?
No.

Performance review
-----------------
* Does the patch slow down simple tests?
No.

* If it claims to improve performance, does it?
n/a

* Does it slow down other things?
No.

Coding review
-----------------
* Does it follow the project coding guidelines?
Yes.

* Are there portability issues?
None that i can see.

* Will it work on Windows/BSD etc?
None that i can see. (I only tested it on linux though)

* Does it do what it says, correctly?
Yes.

* Does it produce compiler warnings?
No.

* Can you make it crash?
No.

Architecture review
-----------------
* Is everything done in a way that fits together coherently with other
features/modules?
Yes.

* Are there interdependencies that can cause problems?
No.

-nigel.