dropdb --force

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
136 messages Options
1 ... 4567
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

Pavel Stehule


pá 22. 11. 2019 v 10:46 odesílatel vignesh C <[hidden email]> napsal:
On Fri, Nov 22, 2019 at 12:10 AM Pavel Stehule <[hidden email]> wrote:
>
>
>
> čt 21. 11. 2019 v 6:33 odesílatel vignesh C <[hidden email]> napsal:
>>
>> On Mon, Nov 18, 2019 at 6:30 PM Pavel Stehule <[hidden email]> wrote:
>> >>
>> >> I'll send this test today
>> >
>> >
>> > here is it
>> >
>>
>> Thanks for adding the test.
>> Few comments:
>> This function is same as in test/recovery/t/013_crash_restart.pl, we
>> can add to a common file and use in both places:
>> +# Pump until string is matched, or timeout occurs
>> +sub pump_until
>> +{
>> +    my ($proc, $stream, $untl) = @_;
>> +    $proc->pump_nb();
>> +    while (1)
>> +    {
>> +        last if $$stream =~ /$untl/;
>> +        if ($psql_timeout->is_expired)
>> +        {
>
>
> yes, I know - probably it can be moved to testlib.pm. Unfortunately it is perl, and I am not able to this correctly. More it use global object - timer
>
>> This can be Tests drop database with force option:
>
>
> done
>
>> +#
>> +# Tests killing session connected to dropped database
>> +#
>>
>> This can be changed to check database foobar1 does not exist.
>
>
> done
>
>> +# and there is not a database with this name
>> +is($node->safe_psql('postgres', qq[SELECT EXISTS(SELECT * FROM
>> pg_database WHERE datname='foobar1');]),
>> +    'f', 'database foobar1 was removed');
>>
>> This can be changed to check the connections on foobar1 database
>> +
>> +# and it is connected to foobar1 database
>> +is($node->safe_psql('postgres', qq[SELECT pid FROM pg_stat_activity
>> WHERE datname='foobar1' AND pid = $pid;]),
>> +    $pid, 'database foobar1 is used');
>
>
> done
>
>>
>> This can be changed to restart psql as the previous connection will be
>> terminated by previous drop database.
>> +
>
>
> done
>
> new patch attached
>

Thanks for fixing the comments. The changes looks fine to me. I have
fixed the first comment, attached patch has the changes for the same.

thank you

looks well

Pavel


Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

akapila
In reply to this post by vignesh C
On Fri, Nov 22, 2019 at 3:16 PM vignesh C <[hidden email]> wrote:
>
> Thanks for fixing the comments. The changes looks fine to me. I have
> fixed the first comment, attached patch has the changes for the same.
>

Few comments:
--------------------------
1. There is a lot of duplicate code in this test.  Basically, almost
the same code is used once to test Drop Database command and dropdb.
I think we can create a few sub-functions to avoid duplicate code, but
do we really need to test the same thing once via command and then by
dropdb utility?   I don't think it is required, but even if we want to
do so, then I am not sure if this is the right test script.  I suggest
removing the command related test.

2.
ok( TestLib::pump_until(
+ $killme,
+ $psql_timeout,
+ \$killme_stderr,
+ qr/FATAL:  terminating connection due to administrator command/m
+ ),
+ "psql query died successfully after SIGTERM");

Extra space before TestLib.

3.
+=item pump_until(proc, psql_timeout, stream, untl)

I think moving pump_until to TestLib is okay, but if you are making it
a generic function to be used from multiple places, it is better to
name the variable as 'timeout' instead of 'psql_timeout'

4. Have you ran perltidy, if not, can you please run it?  See
src/test/perl/README for the recommendation.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

vignesh C
On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila <[hidden email]> wrote:

>
> On Fri, Nov 22, 2019 at 3:16 PM vignesh C <[hidden email]> wrote:
> >
> > Thanks for fixing the comments. The changes looks fine to me. I have
> > fixed the first comment, attached patch has the changes for the same.
> >
>
> Few comments:
> --------------------------
> 1. There is a lot of duplicate code in this test.  Basically, almost
> the same code is used once to test Drop Database command and dropdb.
> I think we can create a few sub-functions to avoid duplicate code, but
> do we really need to test the same thing once via command and then by
> dropdb utility?   I don't think it is required, but even if we want to
> do so, then I am not sure if this is the right test script.  I suggest
> removing the command related test.
>
Pavel: What is your opinion on this?

> 2.
> ok( TestLib::pump_until(
> + $killme,
> + $psql_timeout,
> + \$killme_stderr,
> + qr/FATAL:  terminating connection due to administrator command/m
> + ),
> + "psql query died successfully after SIGTERM");
>
> Extra space before TestLib.
Ran perltidy, perltidy adds an extra space. I'm not sure which version
is right whether to include space or without space. I had noticed
similarly in 001_stream_rep.pl, in few places space is present and in
few places it is not present. If required I can update based on
suggestion.

>
> 3.
> +=item pump_until(proc, psql_timeout, stream, untl)
>
> I think moving pump_until to TestLib is okay, but if you are making it
> a generic function to be used from multiple places, it is better to
> name the variable as 'timeout' instead of 'psql_timeout'
>

Fixed.

> 4. Have you ran perltidy, if not, can you please run it?  See
> src/test/perl/README for the recommendation.
>

I have verified by running perltidy.

Please find the updated patch attached. 1st patch is same as previous,
2nd patch includes the fix for comments 2,3 & 4.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

0001-Add-tests-for-the-support-of-f-option-in-dropdb-util.patch (5K) Download Attachment
0002-Made-pump_until-usable-as-a-common-subroutine.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

Pavel Stehule


ne 24. 11. 2019 v 11:25 odesílatel vignesh C <[hidden email]> napsal:
On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila <[hidden email]> wrote:
>
> On Fri, Nov 22, 2019 at 3:16 PM vignesh C <[hidden email]> wrote:
> >
> > Thanks for fixing the comments. The changes looks fine to me. I have
> > fixed the first comment, attached patch has the changes for the same.
> >
>
> Few comments:
> --------------------------
> 1. There is a lot of duplicate code in this test.  Basically, almost
> the same code is used once to test Drop Database command and dropdb.
> I think we can create a few sub-functions to avoid duplicate code, but
> do we really need to test the same thing once via command and then by
> dropdb utility?   I don't think it is required, but even if we want to
> do so, then I am not sure if this is the right test script.  I suggest
> removing the command related test.
>

Pavel: What is your opinion on this?

I have not any problem with this reduction.

We will see in future years what is benefit of this test.


> 2.
> ok( TestLib::pump_until(
> + $killme,
> + $psql_timeout,
> + \$killme_stderr,
> + qr/FATAL:  terminating connection due to administrator command/m
> + ),
> + "psql query died successfully after SIGTERM");
>
> Extra space before TestLib.

Ran perltidy, perltidy adds an extra space. I'm not sure which version
is right whether to include space or without space. I had noticed
similarly in 001_stream_rep.pl, in few places space is present and in
few places it is not present. If required I can update based on
suggestion.

>
> 3.
> +=item pump_until(proc, psql_timeout, stream, untl)
>
> I think moving pump_until to TestLib is okay, but if you are making it
> a generic function to be used from multiple places, it is better to
> name the variable as 'timeout' instead of 'psql_timeout'
>

Fixed.

> 4. Have you ran perltidy, if not, can you please run it?  See
> src/test/perl/README for the recommendation.
>

I have verified by running perltidy.

Please find the updated patch attached. 1st patch is same as previous,
2nd patch includes the fix for comments 2,3 & 4.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

akapila
In reply to this post by vignesh C
On Sun, Nov 24, 2019 at 3:55 PM vignesh C <[hidden email]> wrote:

>
> On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila <[hidden email]> wrote:
> >
>
> > 2.
> > ok( TestLib::pump_until(
> > + $killme,
> > + $psql_timeout,
> > + \$killme_stderr,
> > + qr/FATAL:  terminating connection due to administrator command/m
> > + ),
> > + "psql query died successfully after SIGTERM");
> >
> > Extra space before TestLib.
>
> Ran perltidy, perltidy adds an extra space. I'm not sure which version
> is right whether to include space or without space. I had noticed
> similarly in 001_stream_rep.pl, in few places space is present and in
> few places it is not present. If required I can update based on
> suggestion.
>

You can try by running perltidy on other existing .pl files where you
find the usage "without space" and see if it adds the extra space in
all places.  I think keeping the version after running perltidy would
be a better choice.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

vignesh C
In reply to this post by Pavel Stehule
On Sun, Nov 24, 2019 at 5:06 PM Pavel Stehule <[hidden email]> wrote:

>
>
>
> ne 24. 11. 2019 v 11:25 odesílatel vignesh C <[hidden email]> napsal:
>>
>> On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila <[hidden email]> wrote:
>> >
>> > On Fri, Nov 22, 2019 at 3:16 PM vignesh C <[hidden email]> wrote:
>> > >
>> > > Thanks for fixing the comments. The changes looks fine to me. I have
>> > > fixed the first comment, attached patch has the changes for the same.
>> > >
>> >
>> > Few comments:
>> > --------------------------
>> > 1. There is a lot of duplicate code in this test.  Basically, almost
>> > the same code is used once to test Drop Database command and dropdb.
>> > I think we can create a few sub-functions to avoid duplicate code, but
>> > do we really need to test the same thing once via command and then by
>> > dropdb utility?   I don't think it is required, but even if we want to
>> > do so, then I am not sure if this is the right test script.  I suggest
>> > removing the command related test.
>> >
>>
>> Pavel: What is your opinion on this?
>
>
> I have not any problem with this reduction.
>
> We will see in future years what is benefit of this test.
>
Fixed, removed dropdb utility test.

>>
>> > 2.
>> > ok( TestLib::pump_until(
>> > + $killme,
>> > + $psql_timeout,
>> > + \$killme_stderr,
>> > + qr/FATAL:  terminating connection due to administrator command/m
>> > + ),
>> > + "psql query died successfully after SIGTERM");
>> >
>> > Extra space before TestLib.
>>
>> Ran perltidy, perltidy adds an extra space. I'm not sure which version
>> is right whether to include space or without space. I had noticed
>> similarly in 001_stream_rep.pl, in few places space is present and in
>> few places it is not present. If required I can update based on
>> suggestion.
>>
>> >
>> > 3.
>> > +=item pump_until(proc, psql_timeout, stream, untl)
>> >
>> > I think moving pump_until to TestLib is okay, but if you are making it
>> > a generic function to be used from multiple places, it is better to
>> > name the variable as 'timeout' instead of 'psql_timeout'
>> >
>>
>> Fixed.
>>
>> > 4. Have you ran perltidy, if not, can you please run it?  See
>> > src/test/perl/README for the recommendation.
>> >
>>
>> I have verified by running perltidy.
>>
>> Please find the updated patch attached. 1st patch is same as previous,
>> 2nd patch includes the fix for comments 2,3 & 4.
>>
Attached patch handles the fix for the above comments.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

0002-Made-pump_until-usable-as-a-common-subroutine.patch (8K) Download Attachment
0001-Add-tests-for-the-support-of-drop-database-forcefull.patch (3K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

akapila
On Mon, Nov 25, 2019 at 11:22 PM vignesh C <[hidden email]> wrote:

>
> On Sun, Nov 24, 2019 at 5:06 PM Pavel Stehule <[hidden email]> wrote:
> >
> >
> >
> > ne 24. 11. 2019 v 11:25 odesílatel vignesh C <[hidden email]> napsal:
> >>
> >> On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila <[hidden email]> wrote:
> >> >
> >> > On Fri, Nov 22, 2019 at 3:16 PM vignesh C <[hidden email]> wrote:
> >> > >
> >> > > Thanks for fixing the comments. The changes looks fine to me. I have
> >> > > fixed the first comment, attached patch has the changes for the same.
> >> > >
> >> >
> >> > Few comments:
> >> > --------------------------
> >> > 1. There is a lot of duplicate code in this test.  Basically, almost
> >> > the same code is used once to test Drop Database command and dropdb.
> >> > I think we can create a few sub-functions to avoid duplicate code, but
> >> > do we really need to test the same thing once via command and then by
> >> > dropdb utility?   I don't think it is required, but even if we want to
> >> > do so, then I am not sure if this is the right test script.  I suggest
> >> > removing the command related test.
> >> >
> >>
> >> Pavel: What is your opinion on this?
> >
> >
> > I have not any problem with this reduction.
> >
> > We will see in future years what is benefit of this test.
> >
>
> Fixed, removed dropdb utility test.
>

Hmm, you have done the opposite of what I have asked.   This test file
is in rc/bin/scripts/t/  where we generally keep the tests for
utilities. So, retaining the dropdb utility test in that file seems
more sensible.

+ok( TestLib::pump_until(
+ $killme, $psql_timeout, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m),
+ 'acquired pid');

How about changing 'acquired pid' to 'acquired pid for SIGTERM'?

> >> >
> >>
> >> I have verified by running perltidy.
> >>

I think we don't need to run perltidy on the existing code.  So, I am
not sure if it is a good idea to run it for file 013_crash_restart.pl
as it changes some existing code formatting.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

vignesh C
On Tue, Nov 26, 2019 at 11:37 AM Amit Kapila <[hidden email]> wrote:

>
> On Mon, Nov 25, 2019 at 11:22 PM vignesh C <[hidden email]> wrote:
> >
> > On Sun, Nov 24, 2019 at 5:06 PM Pavel Stehule <[hidden email]> wrote:
> > >
> > >
> > >
> > > ne 24. 11. 2019 v 11:25 odesílatel vignesh C <[hidden email]> napsal:
> > >>
> > >> On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila <[hidden email]> wrote:
> > >> >
> > >> > On Fri, Nov 22, 2019 at 3:16 PM vignesh C <[hidden email]> wrote:
> > >> > >
> > >> > > Thanks for fixing the comments. The changes looks fine to me. I have
> > >> > > fixed the first comment, attached patch has the changes for the same.
> > >> > >
> > >> >
> > >> > Few comments:
> > >> > --------------------------
> > >> > 1. There is a lot of duplicate code in this test.  Basically, almost
> > >> > the same code is used once to test Drop Database command and dropdb.
> > >> > I think we can create a few sub-functions to avoid duplicate code, but
> > >> > do we really need to test the same thing once via command and then by
> > >> > dropdb utility?   I don't think it is required, but even if we want to
> > >> > do so, then I am not sure if this is the right test script.  I suggest
> > >> > removing the command related test.
> > >> >
> > >>
> > >> Pavel: What is your opinion on this?
> > >
> > >
> > > I have not any problem with this reduction.
> > >
> > > We will see in future years what is benefit of this test.
> > >
> >
> > Fixed, removed dropdb utility test.
> >
>
> Hmm, you have done the opposite of what I have asked.   This test file
> is in rc/bin/scripts/t/  where we generally keep the tests for
> utilities. So, retaining the dropdb utility test in that file seems
> more sensible.
>
Fixed. Retained the test of dropdb utility and removed drop database
sql command test.

> +ok( TestLib::pump_until(
> + $killme, $psql_timeout, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m),
> + 'acquired pid');
>
> How about changing 'acquired pid' to 'acquired pid for SIGTERM'?
>

Fixed. Changed as suggested.

> > >> >
> > >>
> > >> I have verified by running perltidy.
> > >>
>
> I think we don't need to run perltidy on the existing code.  So, I am
> not sure if it is a good idea to run it for file 013_crash_restart.pl
> as it changes some existing code formatting.
>

I have retained the format same as old format, one additional change I
added was to break the line if the line is lengthy in the modified
code.

Attached patch has the fixes for the above comments.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

0002-Add-tests-for-the-support-of-f-option-in-dropdb-util.patch (3K) Download Attachment
0001-Made-pump_until-usable-as-a-common-subroutine.patch (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

akapila
On Wed, Nov 27, 2019 at 10:15 AM vignesh C <[hidden email]> wrote:
>
>
> Attached patch has the fixes for the above comments.
>

I have pushed the refactoring patch.  In the second patch, I have a
few more comments.  I am not completely sure if it is a good idea to
write a new test file 060_dropdb_force.pl when we already have an
existing file 050_dropdb.pl for dropdb tests, but I think if we want
to do that, then lets move existing test for dropdb '-f' from
050_dropdb.pl to new file and it might be better to name new file as
051_dropdb_force.pl.  I see that in some other cases like vacuumdb and
clusterdb, we have separate test files to cover a different kinds of
scenarios, so it should be okay to have a new file for dropdb tests.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

vignesh C
On Thu, Nov 28, 2019 at 8:54 AM Amit Kapila <[hidden email]> wrote:

>
> On Wed, Nov 27, 2019 at 10:15 AM vignesh C <[hidden email]> wrote:
> >
> >
> > Attached patch has the fixes for the above comments.
> >
>
> I have pushed the refactoring patch.  In the second patch, I have a
> few more comments.  I am not completely sure if it is a good idea to
> write a new test file 060_dropdb_force.pl when we already have an
> existing file 050_dropdb.pl for dropdb tests, but I think if we want
> to do that, then lets move existing test for dropdb '-f' from
> 050_dropdb.pl to new file and it might be better to name new file as
> 051_dropdb_force.pl.  I see that in some other cases like vacuumdb and
> clusterdb, we have separate test files to cover a different kinds of
> scenarios, so it should be okay to have a new file for dropdb tests.
>
Thanks for pushing the patch. Please find the attached patch having
the fixes for the comments suggested.

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

0001-Add-tests-for-the-support-of-f-option-in-dropdb-util.patch (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

Michael Paquier-2
In reply to this post by akapila
On Thu, Nov 28, 2019 at 08:53:56AM +0530, Amit Kapila wrote:
> I have pushed the refactoring patch.  In the second patch, I have a
> few more comments.  I am not completely sure if it is a good idea to
> write a new test file 060_dropdb_force.pl when we already have an
> existing file 050_dropdb.pl for dropdb tests, but I think if we want
> to do that, then lets move existing test for dropdb '-f' from
> 050_dropdb.pl to new file and it might be better to name new file as
> 051_dropdb_force.pl.  I see that in some other cases like vacuumdb and
> clusterdb, we have separate test files to cover a different kinds of
> scenarios, so it should be okay to have a new file for dropdb tests.

Amit, as most of the patch set has been committed, would it make sense
to mark this entry as committed in the CF app?
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

Juan José Santamaría Flecha


On Fri, Nov 29, 2019 at 7:30 AM Michael Paquier <[hidden email]> wrote:
On Thu, Nov 28, 2019 at 08:53:56AM +0530, Amit Kapila wrote:
> I have pushed the refactoring patch.  In the second patch, I have a
> few more comments.  I am not completely sure if it is a good idea to
> write a new test file 060_dropdb_force.pl when we already have an
> existing file 050_dropdb.pl for dropdb tests, but I think if we want
> to do that, then lets move existing test for dropdb '-f' from
> 050_dropdb.pl to new file and it might be better to name new file as
> 051_dropdb_force.pl.  I see that in some other cases like vacuumdb and
> clusterdb, we have separate test files to cover a different kinds of
> scenarios, so it should be okay to have a new file for dropdb tests.

Amit, as most of the patch set has been committed, would it make sense
to mark this entry as committed in the CF app?


Test 051_dropdb_force.pl is failing on Windows critters in the build farm, e.g:


Regards,

Juan José Santamaría Flecha 


Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

akapila
On Fri, Nov 29, 2019 at 1:36 PM Juan José Santamaría Flecha
<[hidden email]> wrote:

>
>
> On Fri, Nov 29, 2019 at 7:30 AM Michael Paquier <[hidden email]> wrote:
>>
>> On Thu, Nov 28, 2019 at 08:53:56AM +0530, Amit Kapila wrote:
>> > I have pushed the refactoring patch.  In the second patch, I have a
>> > few more comments.  I am not completely sure if it is a good idea to
>> > write a new test file 060_dropdb_force.pl when we already have an
>> > existing file 050_dropdb.pl for dropdb tests, but I think if we want
>> > to do that, then lets move existing test for dropdb '-f' from
>> > 050_dropdb.pl to new file and it might be better to name new file as
>> > 051_dropdb_force.pl.  I see that in some other cases like vacuumdb and
>> > clusterdb, we have separate test files to cover a different kinds of
>> > scenarios, so it should be okay to have a new file for dropdb tests.
>>
>> Amit, as most of the patch set has been committed, would it make sense
>> to mark this entry as committed in the CF app?
>>

It might be better to move it to next CF as the discussion is still active.

>
> Test 051_dropdb_force.pl is failing on Windows critters in the build farm, e.g:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=drongo&dt=2019-11-29%2003%3A54%3A06
>

Yeah,  I have proposed something for it on pgsql-committers [1].

[1] - https://www.postgresql.org/message-id/CAA4eK1%2BGNyjaPK77y%2Beuh5eAgM75pncG1JYZhxYZF%2BSgS6NpjA%40mail.gmail.com

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

vignesh C
In reply to this post by Juan José Santamaría Flecha
On Fri, Nov 29, 2019 at 1:36 PM Juan José Santamaría Flecha
<[hidden email]> wrote:

>
>
>
> On Fri, Nov 29, 2019 at 7:30 AM Michael Paquier <[hidden email]> wrote:
>>
>> On Thu, Nov 28, 2019 at 08:53:56AM +0530, Amit Kapila wrote:
>> > I have pushed the refactoring patch.  In the second patch, I have a
>> > few more comments.  I am not completely sure if it is a good idea to
>> > write a new test file 060_dropdb_force.pl when we already have an
>> > existing file 050_dropdb.pl for dropdb tests, but I think if we want
>> > to do that, then lets move existing test for dropdb '-f' from
>> > 050_dropdb.pl to new file and it might be better to name new file as
>> > 051_dropdb_force.pl.  I see that in some other cases like vacuumdb and
>> > clusterdb, we have separate test files to cover a different kinds of
>> > scenarios, so it should be okay to have a new file for dropdb tests.
>>
>> Amit, as most of the patch set has been committed, would it make sense
>> to mark this entry as committed in the CF app?
>>
>
> Test 051_dropdb_force.pl is failing on Windows critters in the build farm, e.g:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=drongo&dt=2019-11-29%2003%3A54%3A06
>
Attached patch includes the fix for the following failure in buildfarm:
Nov 28 09:00:01 #   Failed test 'database foobar1 is used'
Nov 28 09:00:01 #   at t/051_dropdb_force.pl line 71.
Nov 28 09:00:01 #          got: '7380'
Nov 28 09:00:01 #     expected: '7380
'
Nov 28 09:00:01 # aborting wait: program died

This test passes in most buildfarm environment, but it fails in few
windows environment randomly. The  attached patch removes the query
which is not really needed for this test. Alternatively we could also
modify something like below as in PostgresNode.pm:
$pid =~ s/\r//g if $TestLib::windows_os;
I do not have an environment in which I could reproduce and I felt
this is not really needed as part of this testcase.

Any thoughts?

Regards,
Vignesh
EnterpriseDB: http://www.enterprisedb.com

0001-Drop-db-test-chomp-build-farm-failure-fix.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

Michael Paquier-2
In reply to this post by akapila
On Fri, Nov 29, 2019 at 03:31:21PM +0530, Amit Kapila wrote:
> It might be better to move it to next CF as the discussion is still active.

OK, just did that.
--
Michael

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: dropdb --force

akapila
On Sat, Nov 30, 2019 at 7:46 AM Michael Paquier <[hidden email]> wrote:
>
> On Fri, Nov 29, 2019 at 03:31:21PM +0530, Amit Kapila wrote:
> > It might be better to move it to next CF as the discussion is still active.
>
> OK, just did that.
>

I have marked this as committed in CF.  This was committed some time
back[1][2].  I was just waiting for the conclusion on what we want to
do about Windows behavior related to socket closure which we
discovered while testing this feature.  It is not very clear whether
we want to do anything about it, see discussion on thread [3], so I
closed this.

[1] - https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=1379fd537f9fc7941c8acff8c879ce3636dbdb77
[2] - https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=80e05a088e4edd421c9c0374d54d787c8a4c0d86
[3] - https://www.postgresql.org/message-id/CALDaNm2tEvr_Kum7SyvFn0%3D6H3P0P-Zkhnd%3DdkkX%2BQ%3DwKutZ%3DA%40mail.gmail.com

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


1 ... 4567