Pushed, thanks.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi Fabien,
>> See the attached latest patch.
>> The attached patch applies, builds cleanly, and passes the regression
>> tests.
>
> No problems on my part as I find the changes logical.
> This also needs a confirmation from Alvaro.
>
> Ok.
>
> You switched the patch to "waiting on author": What
Hello Kirk,
so I tried to apply the patch, but part of the chunk failed,
because of the unused line below which was already removed in the
recent related commit.
PGResult*res;
I removed the line and fixed the other trailing whitespaces.
Indeed. Thanks for the fix.
See
Hi Fabien and Alvaro,
I found that I have already reviewed this thread before,
so I tried to apply the patch, but part of the chunk failed,
because of the unused line below which was already removed in the
recent related commit.
> PGResult*res;
I removed the line and fixed the
Hello Alvaro,
I just pushed this. I hope not to have upset you too much with the
subroutine thing.
Sorry for the feedback delay, my week was kind of overloaded…
Thanks for the push.
About the patch you committed, a post-commit review:
- the state and function renamings are indeed a good
I just pushed this. I hope not to have upset you too much with the
subroutine thing.
Thanks for the submission and Kirk for the review.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-Nov-20, Fabien COELHO wrote:
> > On INSTR_TIME_SET_CURRENT_LAZY(), you cannot just put an "if" inside a
> > macro -- consider this:
> > if (foo)
> > INSTR_TIME_SET_CURRENT_LAZY(bar);
> > else
> > something_else();
> > Which "if" is the else now attached to?
On 2018-Nov-20, Fabien COELHO wrote:
> Hmm. It is somehow, but the aim of the refactoring is to make *ALL* state
> transitions to happen in doCustom's switch (st->state) and nowhere else,
> which is defeated by creating the separate function.
>
> Although it improves readability at one level, it
On 2018-Nov-20, Fabien COELHO wrote:
>
> > I didn't quite understand this hunk. Why does it remove the
> > is_latencies conditional? (The preceding comment shown here should be
> > updated obviously if this change is correct, but I'm not sure it is.)
>
> Pgbench runs benches a collects
I didn't quite understand this hunk. Why does it remove the
is_latencies conditional? (The preceding comment shown here should be
updated obviously if this change is correct, but I'm not sure it is.)
Pgbench runs benches a collects performance data about it.
I simplified the code to
On 2018-Nov-20, Fabien COELHO wrote:
> Hmm. It is somehow, but the aim of the refactoring is to make *ALL* state
> transitions to happen in doCustom's switch (st->state) and nowhere else,
> which is defeated by creating the separate function.
>
> Although it improves readability at one level, it
Hello Alvaro,
Thanks for the review and improvements.
Attached v4 is a rebase after 409231919443984635b7ae9b7e2e261ab984eb1e
Attached v5. I thought that separating the part that executes the
command was an obvious readability improvement.
Hmm. It is somehow, but the aim of the
On 2018-Nov-17, Fabien COELHO wrote:
>
> > Attached is a v3, where I have updated inaccurate comments.
>
> Attached v4 is a rebase after 409231919443984635b7ae9b7e2e261ab984eb1e
Attached v5. I thought that separating the part that executes the
command was an obvious readability improvement.
Attached is a v3, where I have updated inaccurate comments.
Attached v4 is a rebase after 409231919443984635b7ae9b7e2e261ab984eb1e
--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 73d3de0677..ed6ff75426 100644
--- a/src/bin/pgbench/pgbench.c
+++
Hello Kirk,
I have decided to take a look into this patch.
Thanks for the feedback.
I noticed in your refactored code that CSTATE_CHOOSE_SCRIPT may now also
change to CSTATE_FINISHED when timer is exceeded. (Before, it only
changes to either CSTATE_START_THROTTLE or CSTATE_START_TX) But
Hello Fabien,
I have decided to take a look into this patch.
--
patching file src/bin/pgbench/pgbench.c
Hunk #1 succeeded at 296 (offset 29 lines).
[…Snip…]
Hunk #21 succeeded at 5750 (offset 108 lines).
patching file src/include/portability/instr_time.h
….
===
All 189 tests
16 matches
Mail list logo
