From: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, pgsql-hackers(at)postgresql(dot)org, Teodor Sigaev <teodor(at)sigaev(dot)ru> |
Subject: | Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors |
Date: | 2018-05-08 10:58:32 |
Message-ID: | 20180508105832.6o3uf3npfpjgk5m7@alvherre.pgsql |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Fabien COELHO wrote:
> I think that I'll have time for a round of review in the first half of July.
> Providing a rebased patch before then would be nice.
Note that even in the absence of a rebased patch, you can apply to an
older checkout if you have some limited window of time for a review.
Looking over the diff, I find that this patch tries to do too much and
needs to be split up. At a minimum there is a preliminary patch that
introduces the error reporting stuff (errstart etc); there are other
thread-related changes (for example to the random generation functions)
that probably belong in a separate one too. Not sure if there are other
smaller patches hidden inside the rest.
On elog/errstart: we already have a convention for what ereport() calls
look like; I suggest to use that instead of inventing your own. With
that, is there a need for elog()? In the backend we have it because
$HISTORY but there's no need for that here -- I propose to lose elog()
and use only ereport everywhere. Also, I don't see that you need
errmsg_internal() at all; let's lose it too.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From | Date | Subject | |
---|---|---|---|
Next Message | Justin Pryzby | 2018-05-08 11:05:42 | Re: doc fixes: vacuum_cleanup_index_scale_factor |
Previous Message | Andrew Gierth | 2018-05-08 10:15:23 | Re: Optimze usage of immutable functions as relation |