On 3/28/20 5:27 AM, Fabien COELHO wrote:
Hello Tom,
Thanks for your feedback,
I'd be rather unclear about what the actual feedback is, though. I'd
interpret it as "pg does not care much about code coverage". Most
clients
are in the red on coverage.postgresql.org. I'd like pgbench at least
t
Hello Tom,
Thanks for your feedback,
I'd be rather unclear about what the actual feedback is, though. I'd
interpret it as "pg does not care much about code coverage". Most clients
are in the red on coverage.postgresql.org. I'd like pgbench at least to be
in the green, but it does not look that
Fabien COELHO writes:
>> It does not look like the remainder of this patch is going to be committed
>> and I don't think it makes sense to keep moving the patch indefinitely.
>> Unless something changes by the end of this CF I'll mark it Returned With
>> Feedback.
> I'd be rather unclear about
This patch is registered in 2020-01, but the last message in the thread
seems to be from 2019/05/23. The patch itself seems to be OK (it applies
fine etc.) What do we need to get it over the line, instead of just
moving it to the next one CF over and over?
It does not look like the remainder
Hi Fabien,
On 1/4/20 5:00 PM, Tomas Vondra wrote:
This patch is registered in 2020-01, but the last message in the thread
seems to be from 2019/05/23. The patch itself seems to be OK (it applies
fine etc.) What do we need to get it over the line, instead of just
moving it to the next one CF ove
Hi,
This patch is registered in 2020-01, but the last message in the thread
seems to be from 2019/05/23. The patch itself seems to be OK (it applies
fine etc.) What do we need to get it over the line, instead of just
moving it to the next one CF over and over?
regards
--
Tomas Vondra
V11 is just a rebase after the reindentation patches.
Indeed, yet again, I forgot the attachement:-(
I stared at the new test case for a while, and I must say it looks very
cryptic. It's not exactly this patch's fault - all the pgbench tests are
cryptic -
Perl is cryptic. Regexprs are cryp
Hello Heikki,
Indeed, yet again, I forgot the attachement:-(
I stared at the new test case for a while, and I must say it looks very
cryptic. It's not exactly this patch's fault - all the pgbench tests are
cryptic -
Perl is cryptic. Regexprs are cryptic.
but I think we need to do something
Hello Heikki,
I stared at the new test case for a while, and I must say it looks very
cryptic. It's not exactly this patch's fault - all the pgbench tests are
cryptic -
Perl is cryptic. Regexprs are cryptic.
but I think we need to do something about that before adding any more
tests. I'm
On 25/03/2019 20:13, Fabien COELHO wrote:
Attached is the remainder of the patch rebased to current head.
I stared at the new test case for a while, and I must say it looks very
cryptic. It's not exactly this patch's fault - all the pgbench tests are
cryptic - but I think we need to do someth
I pushed the refactoring now, to extract the progress reporting to a separate
function. That seems like an improvement independently from the rest of the
patch.
Sure.
I'll take another look at the rest, probably tomorrow.
Ok.
Attached is the remainder of the patch rebased to current head
On 10/03/2019 13:26, Fabien COELHO wrote:
Attached is a rebase.
I pushed the refactoring now, to extract the progress reporting to a
separate function. That seems like an improvement independently from the
rest of the patch. I'll take another look at the rest, probably tomorrow.
- Heikki
Attached is a rebase.
--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 5df54a8e57..1db6b75823 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -6032,6 +6032,96 @@ main(int argc, char **argv)
return exit_code;
}
+/* display the progr
Hello Tom,
Unfortunately, there was no activity over the last few commitfests and the
proposed patch pgbench-tap-progress-6 can't be applied anymore without
conflicts. Fabien, what are your plans about it, could you please post a
rebased version?
Here it is.
I'm confused about the intende
Fabien COELHO writes:
>> Unfortunately, there was no activity over the last few commitfests and the
>> proposed patch pgbench-tap-progress-6 can't be applied anymore without
>> conflicts. Fabien, what are your plans about it, could you please post a
>> rebased version?
> Here it is.
I'm confused
On Sat, Nov 24, 2018 at 05:58:38PM +0100, Fabien COELHO wrote:
>> Unfortunately, there was no activity over the last few commitfests and the
>> proposed patch pgbench-tap-progress-6 can't be applied anymore without
>> conflicts. Fabien, what are your plans about it, could you please post a
>> rebas
Unfortunately, there was no activity over the last few commitfests and the
proposed patch pgbench-tap-progress-6 can't be applied anymore without
conflicts. Fabien, what are your plans about it, could you please post a
rebased version?
Here it is.
--
Fabien.diff --git a/src/bin/pgbench/pgbenc
> On Thu, Jul 19, 2018 at 2:24 AM Fabien COELHO wrote:
>
>
> Hello Heikki,
>
> >> [...]
> >> So threadRun() would not have the opportunity to stop the scheduled
> >> transaction, even if beyond the end of run, because it would not have got
> >> out of doCustom, in the case I outlined above.
> >
>
Hello Heikki,
[...]
So threadRun() would not have the opportunity to stop the scheduled
transaction, even if beyond the end of run, because it would not have got
out of doCustom, in the case I outlined above.
I see. Instead of moving to FINISHED state, then, we could stay in THROTTLE
state,
On 18/07/18 23:29, Fabien COELHO wrote:
Hmm. How about we just remove this special case from doCustom():
case CSTATE_START_THROTTLE:
// ...
if (duration > 0 && st->txn_scheduled > end_time)
{
st->state = CSTATE_FINISHED;
break;
}
That way, we let the cli
Hmm. How about we just remove this special case from doCustom():
case CSTATE_START_THROTTLE:
// ...
if (duration > 0 && st->txn_scheduled > end_time)
{
st->state = CSTATE_FINISHED;
break;
}
That way, we let the client go into CSTATE_THROTTLE state, even tho
On 18/07/18 22:56, Fabien COELHO wrote:
Hello Heikki,
Yep. The attached version does only the tailing stuff under -R and not all
threads were stopped on errors, with comments to tell about the why.
Hmm. How about we just remove this special case from doCustom():
case CSTATE_START_THROTTL
Hello Heikki,
Yep. The attached version does only the tailing stuff under -R and not all
threads were stopped on errors, with comments to tell about the why.
Hmm. How about we just remove this special case from doCustom():
case CSTATE_START_THROTTLE:
// ...
if (duration > 0 && st->t
On 18/07/18 16:01, Fabien COELHO wrote:
I don't think you want to wait in that situation. I think we should wait at
the end only if there some threads still alive, with nothing to do only
because of --rate.
Yep. The attached version does only the tailing stuff under -R and not all
threads were
Hello Heikki,
I did that in the attached version: no more environment variable hack, and
no execution shortcut even if there is nothing to do.
I also had to reproduce the progress logic to keep on printing report of
(no) progress in this tailing phase.
On second thoughts, there's one problem
On 18/07/18 01:43, Fabien COELHO wrote:
The more reasonable alternative could be to always last 2 seconds under
-T 2, even if the execution can be shorten because there is nothing to do
at all, i.e. remove the environment-based condition but keep the sleep.
That sounds reasonable. It's a bit si
Hello Heikki,
[...] Let's keep it that way. I think the only change we need to make in
the logic is to check at the end, if *any* progress reports at all have
been printed, and print one if not.
Ok, this simplifies the condition.
And do that only when the -P option is smaller than the -T op
On 12/07/18 21:27, Fabien COELHO wrote:
For the testing, we just need to make sure that at least one progress report
is always printed, if -P is used. Right?
Yep. That is the first condition above the last_report is set to
thread_start meaning that there has been no report.
So where does the
Indeed… but then throttling would not be tested:-) The point of the test
is to exercise all time-related options, including throttling with a
reasonable small value.
Ok. I don't think that's really worthwhile. If we add some code that only
runs in testing, then we're not really testing the re
I don't understand the 0.5 second rule. For the tests, we only need to
ensure that at least one progress report is printed, right?
[...]
I still don't understand.
Let's look at the code:
if (progress && thread->tid == 0)
{
...
if (last_report == thread_start || now - l
On 12/07/18 19:00, Fabien COELHO wrote:
How pgbenchs prints a progress if none were printed, or if the last
progress was over 0.5 seconds ago, so as to have kind of a catchup in the
end.
I don't understand the 0.5 second rule. For the tests, we only need to ensure
that at least one progress rep
Hello Heikki,
Thanks for having a look at this small patch which aim at improving
pgbench coverage.
How pgbenchs prints a progress if none were printed, or if the last
progress was over 0.5 seconds ago, so as to have kind of a catchup in the
end.
I don't understand the 0.5 second rule. For
On 18/01/18 12:26, Fabien COELHO wrote:
Hm. Could we get somewhere by making the test look for that, and
adjusting the loop logic inside pgbench so that (maybe only with the
tested switch values) it's guaranteed to print at least one progress
output regardless of timing, because it won't check f
Minor rebase after vpath fix (e94f2bc809a0c684185666f19d81f6496e732a3a).
--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 5c07dd9..1fd2451 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -5163,6 +5163,96 @@ main(int argc, char **argv)
Hello Tom,
From my previous answer, to motivate these tests:
The compromise I'm proposing is to skip time-related stuff if too slow. The
value I see is that it provides coverage for all time-related features. I can
also add a check that the run is *at least* 2 seconds when two seconds are
Hello Tom,
Thanks for having a look at this attempt.
Attached is an attempt at improving the situation:
I took a quick look at this and can't really convince myself that it
improves our situation. The fact that it prints nothing if the runtime
is outside of a tightly constrained range seems
Fabien COELHO writes:
> Attached is an attempt at improving the situation:
I took a quick look at this and can't really convince myself that it
improves our situation. The fact that it prints nothing if the runtime
is outside of a tightly constrained range seems likely to hide the sort
of bug yo
On Sun, Sep 24, 2017 at 11:30 PM, Fabien COELHO wrote:
> Attached is an attempt at improving the situation:
>
> (1) there are added comments to explain the whys, which is to provide
> coverage for pgbench time-related features... while still not being
> time-sensitive, which is a challenge.
>
> (2
38 matches
Mail list logo
