On 2017-09-11 15:02:21 -0400, Andrew Dunstan wrote:
>
>
> On 09/11/2017 01:58 PM, Tom Lane wrote:
> > Andrew Dunstan writes:
> >> On 09/08/2017 09:40 AM, Tom Lane wrote:
> >>> Like you, I'm a bit worried about the code for extracting an exit
> >>> status from IPC::Run::run. We'll have to keep a
On 09/11/2017 01:58 PM, Tom Lane wrote:
> Andrew Dunstan writes:
>> On 09/08/2017 09:40 AM, Tom Lane wrote:
>>> Like you, I'm a bit worried about the code for extracting an exit
>>> status from IPC::Run::run. We'll have to keep an eye on the buildfarm
>>> for a bit. If there's any trouble, I'd
Andrew Dunstan writes:
> On 09/08/2017 09:40 AM, Tom Lane wrote:
>> Like you, I'm a bit worried about the code for extracting an exit
>> status from IPC::Run::run. We'll have to keep an eye on the buildfarm
>> for a bit. If there's any trouble, I'd be inclined to drop it down
>> to just success/
On 09/08/2017 09:40 AM, Tom Lane wrote:
> Fabien COELHO writes:
>> [ pgbench-tap-12.patch ]
> Pushed, with some minor fooling with comments and after running it
> through perltidy. (I have no opinions about Perl code formatting,
> but perltidy does ...)
>
> The only substantive change I made wa
Hello Tom,
Pushed, with some minor fooling with comments and after running it
through perltidy. (I have no opinions about Perl code formatting,
but perltidy does ...)
Why not. I do not like the result much, but it homogeneity is not a bad
thing.
The only substantive change I made was to
Fabien COELHO writes:
> [ pgbench-tap-12.patch ]
Pushed, with some minor fooling with comments and after running it
through perltidy. (I have no opinions about Perl code formatting,
but perltidy does ...)
The only substantive change I made was to drop the test that attempted
to connect to no-su
Fabien COELHO writes:
>>> I run the test, figure out the number it found in the resulting
>>> error message, and update the number in the source.
>> Yeah, but then what error is all that work protecting you from?
> I'm not sure I understand your point. I agree that Perl doing the counting
> may
Michael Paquier wrote:
> On Thu, Aug 31, 2017 at 9:29 PM, Nikolay Shaplov wrote:
> > I am about to set "Ready for commit" status to this patch. So there is my
> > summary for commiter, so one does not need to carefully read all the thread.
> >
> > This patch is consists of three parts. May be they
Hello Tom,
sub pgbench($)
My concern is basically about maintaining coding style consistency.
Yes, I'm fine with that. I have removed it in the v12 patch.
reasons why it's not like that already. I do have to say that many of
the examples I've seen look more like line noise than reada
Fabien COELHO writes:
>> * In the same vein, I don't know what this does:
>> sub pgbench($)
>> and I see no existing instances of it elsewhere in our tree. I think it'd
>> be better not to require advanced degrees in Perl programming in order to
>> read the test cases.
> It just says that 5
Anyway, done with the addition of a "chomp" parameter, leaving only the
TAP test changes to consider.
Ok, thanks.
I'll set the CF entry back to "waiting on author" pending your
revisions of those.
See attached.
Removed scalar/array ref hack, plenty [] added everywhere to match.
Removed p
Fabien COELHO writes:
>> * I do not think we need expr_scanner_chomp_substring. Of the three
>> existing callers of expr_scanner_get_substring, one is doing a manual
>> chomp afterwards, and the other two need chomps per your patch.
> Ok. I thought that I would get a slap on the hand if I change
Hello Tom,
As for included bug fixes, I can do separate patches, but I think that it
is enough to first commit the pgbench files and then the tap-test files,
in that order. I'll see what committers say.
Starting to look at this. I concur that the bug fixes ought to be
committed separately, b
Nikolay Shaplov writes:
> 2. Patch for -t/-R/-L case.
> Current pgbench does not process -t/-R/-L case correctly. This was also fixed
> in this patch.
This portion of the patch seems uncontroversial, so I've pushed it, with
some minor cosmetic adjustments (mostly comments).
Fabien COELHO writes:
> As for included bug fixes, I can do separate patches, but I think that it
> is enough to first commit the pgbench files and then the tap-test files,
> in that order. I'll see what committers say.
Starting to look at this. I concur that the bug fixes ought to be
committe
On Thu, Aug 31, 2017 at 9:29 PM, Nikolay Shaplov wrote:
> I am about to set "Ready for commit" status to this patch. So there is my
> summary for commiter, so one does not need to carefully read all the thread.
>
> This patch is consists of three parts. May be they should be commited
> separately,
Hello Nikolay,
Thanks for the review!
As for function names, committers can have their say. I'm somehow not
dissatisfied with the current version, but I also agree with you that they
are imperfect.
As for included bug fixes, I can do separate patches, but I think that it
is enough to first
Hi All!
I am about to set "Ready for commit" status to this patch. So there is my
summary for commiter, so one does not need to carefully read all the thread.
This patch is consists of three parts. May be they should be commited
separately, then Fabien will split them, I think.
1. The tests.
While reviewing another patch, I figured out at least one potential issue
on windows when handling execution status.
The attached v11 small updates is more likely to pass on windows.
--
Fabien.diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index dc1367b..8255eff 100644
-
Hello Nikolay,
I've applied the patch to the current master, and it seems that one test now
fails:
Indeed, Tom removed the -M option order constraint, so the test which
check for that does not apply anymore.
Here is a v10 without this test.
--
Fabien.diff --git a/src/bin/pgbench/exprscan.
В письме от 25 июня 2017 20:42:58 пользователь Fabien COELHO написал:
> > may be this it because of "end_offset + 1" in expr_scanner_chomp_substring
> > ? Why there is + 1 there?
>
> Thanks for the debug! Here is a v9 which does a rebase after the
> reindentation and fixes the bad offset.
Sorry I
may be this it because of "end_offset + 1" in expr_scanner_chomp_substring ?
Why there is + 1 there?
Thanks for the debug! Here is a v9 which does a rebase after the
reindentation and fixes the bad offset.
--
Fabien.diff --git a/src/bin/pgbench/exprscan.l b/src/bin/pgbench/exprscan.l
index
В письме от 15 июня 2017 21:10:12 Вы написали:
> > As for me, I would do expr_scanner_chomp_substring(PsqlScanState, int,
> > int&); that changes end_offset as desired...
>
> Why not.
>
> > And use it instead of end_offset = expr_scanner_offset(sstate) - 1;
>
> I removed these?
>
> > The second
В письме от 25 июня 2017 19:03:54 пользователь Fabien COELHO написал:
> Hello Nikolay,
>
> >> Is the attached version better to your test?
> >
> > I've expected from expr_scanner_chomp_substring to decrement end_offset,
> > so it would work more like perl chomp function, but the way you've done
>
As for me, I would do expr_scanner_chomp_substring(PsqlScanState, int, int&);
that changes end_offset as desired...
Why not.
And use it instead of end_offset = expr_scanner_offset(sstate) - 1;
I removed these?
The second issue: you are removing all trailing \n and \r. I think you should
В письме от 8 июня 2017 19:56:02 пользователь Fabien COELHO написал:
> > So this should be fixed in both expr_scanner_get_substring cases, and keep
> > last symbol only if it is not "\n".
>
> Indeed, this is a mess. The code assumes all stuff is a line ending with
> '\n', but this is not always t
Hello Nikolay,
I did some investigation: The code there really suppose that there is always
\n at the end of the line, and truncates the line. It is done in
/* Get location of the ending newline */
end_offset = expr_scanner_offset(sstate) - 1;
just two lines above the code we are discussing.
В письме от 30 мая 2017 17:24:26 Вы написали:
> > I still have three more questions. A new one:
> >
> >
> >
> >my_command->line = expr_scanner_get_substring(sstate,
> >
> > start_offset,
> >
> > -
Hello Nikolay,
Year, this is much more clear for me. Now I understand this statistics code.
Great.
I still have three more questions. A new one:
my_command->line = expr_scanner_get_substring(sstate,
start_offset,
-
Hello,
st->cnt -- number of transactions finished successed or failed, right?
Or *skipped*. That is why I changed the declaration comment.
one iteration of for(;;) is for one transaction or really less. Right?
No, under -R -L late schedules are simply skipped.
We can't process two tansac
В письме от 8 мая 2017 22:08:51 пользователь Fabien COELHO написал:
> >while (thread->throttle_trigger < now_us -
> >latency_limit &&
> >
> > /* with -t, do not overshoot */
> > (nxacts
Hello Alvaro,
Here is a v3, with less files. I cannot say I find it better, but it
still works.
The "command_likes" function has been renamed "command_checks".
Do parts of this need to be backpatched?
I would not bother too much about backpatching.
I notice that you're patching pgbench.
Fabien COELHO wrote:
>
> Here is a v3, with less files. I cannot say I find it better, but it still
> works.
>
> The "command_likes" function has been renamed "command_checks".
Do parts of this need to be backpatched? I notice that you're patching
pgbench.c, probably to fix some bug(s); is the
Here is a v3, with less files. I cannot say I find it better, but it
still works.
The "command_likes" function has been renamed "command_checks".
--
Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index ae36247..749b16d 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/sr
Hello Robert,
1. This patch makes assorted cosmetic and non-cosmetic changes to
pgbench.c. That is not expected for a testing patch.
Indeed, cosmetic changes should be avoided.
If those changes need to be made because they are bug fixes or whatever,
Yep, this is the case, minor bugs, plus
Hello Nikolay,
- I agree to add a generic command TestLib & a wrapper in PostgresNode,
instead of having pgbench specific things in the later, then call
them from pgbench test script.
- I still think that moving the pgbench scripts inside the test script
is a bad idea (tm).
My sum up
Hello,
Nope. TestLib does not know about PostgresNode, the idea is rather that
PostgresNode knows and wraps around TestLib when needed.
Actually as I look at this part, I feeling an urge to rewrite this code, and
change it so, that all command_like calls were called in a context of certain
no
On Mon, Apr 24, 2017 at 4:45 PM, Nikolay Shaplov wrote:
> I can tell this about this code, if:
>
> - There is no pgbench specific staff in src/test/perl. Or there should be
> _really_big_ reason for it.
>
> - All the testing is done via calls of TestLib.pm functions. May be these
> functions shoul
В письме от 24 апреля 2017 09:01:18 пользователь Fabien COELHO написал:
> > To sum up:
> >
> > - I agree to add a generic command TestLib & a wrapper in PostgresNode,
> >
> > instead of having pgbench specific things in the later, then call
> > them from pgbench test script.
> >
> > - I stil
В письме от 23 апреля 2017 22:02:25 пользователь Fabien COELHO написал:
> Hello Nikolay,
>
> >> Hmmm. The pre-existing TAP test in pgbench is about concurrent commits,
> >> it
> >> is not to test pgbench itself. Pgbench allows to run some programmable
> >> clients in parallel very easily, which ca
To sum up:
- I agree to add a generic command TestLib & a wrapper in PostgresNode,
instead of having pgbench specific things in the later, then call
them from pgbench test script.
- I still think that moving the pgbench scripts inside the test script
is a bad idea (tm).
Here is a v2 al
Hello Nikolay,
Hmmm. The pre-existing TAP test in pgbench is about concurrent commits, it
is not to test pgbench itself. Pgbench allows to run some programmable
clients in parallel very easily, which cannot be done simply otherwise. I
think that having it there would encourage such uses.
Sin
В письме от 20 апреля 2017 19:14:34 пользователь Fabien COELHO написал:
> >> (1) extends the existing perl tap test infrastructure with methods to
> >> test
> >> pgbench, i.e. "pgbench" which runs a pgbench test and "pgbench_likes"
> >> which allows to check for expectations.
> >
> > I do not thi
Hello Nikolay,
Hi! Since I used to be good at perl, I like tests, and I've dealt with
postgres TAP tests before, I think I can review you patch. If it is OK for
everyone.
I think that all good wills are welcome, especially concerning code
reviews.
For now I've just gave this patch a quick
В письме от 17 апреля 2017 14:51:45 пользователь Fabien COELHO написал:
> When developping new features for pgbench, I usually write some tests
> which are lost when the feature is committed. Given that I have submitted
> some more features and that part of pgbench code may be considered for
> sha
Hello,
When developping new features for pgbench, I usually write some tests
which are lost when the feature is committed. Given that I have submitted
some more features and that part of pgbench code may be considered for
sharing with pgsql, I think that improving the abysmal state of tests
46 matches
Mail list logo
