Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ] - Mailing list pgsql-hackers

From Dilip kumar
Subject Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]
Date
Msg-id 4205E661176A124FAF891E0A6BA9135266363726@szxeml509-mbs.china.huawei.com
Whole thread
In response to Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]
List pgsql-hackers

On 26 September 2014 12:24, Amit Kapila Wrote,

 

>I don't think this can handle cancel requests properly because

>you are just setting it in GetIdleSlot() what if the cancel

>request came during GetQueryResult() after sending sql for

>all connections (probably thats the reason why Jeff is not able

>to cancel the vacuumdb when using parallel option).

 

You are right, I have fixed, it in latest patch, please check latest patch @ (4205E661176A124FAF891E0A6BA9135266363710@szxeml509-mbs.china.huawei.com)

 

dilip@linux-ltr9:/home/dilip/9.4/install/bin> ./vacuumdb -z -a -j 8 -p 9005

vacuumdb: vacuuming database "db1"

vacuumdb: vacuuming database "postgres"

Cancel request sent

vacuumdb: vacuuming of database "postgres" failed: ERROR:  canceling statement due to user request

 

>Few other points

>1.

>+ vacuum_parallel(const char *dbname, bool full, bool verbose,

>{

>..

>+        connSlot = (ParallelSlot*)pg_malloc(concurrentCons * sizeof(ParallelSlot));

>+        connSlot[0].connection = conn;

 

Fixed

 

>a.

>Does above memory gets freed anywhere, if not isn't it

>good idea to do the same

>b. For slot 0, you are not seeting it as PQsetnonblocking,

>where as I think it can be used to run commands like any other

>connection.

 

Yes, this was missing in the code, I have fixed it..

 

>2.

>+        /*

>+        * If user has given the vacuum of complete db, then if

>+        * any of the object vacuum

>failed it can be ignored and vacuuming

>+        * of other object can be continued, this is the same

>behavior as

>+        * vacuuming of complete db is handled without --jobs option

>+        */

>s/object/object's

 

FIXED

 

>3.

>+                    if(!completedb ||

>+                                (sqlState && strcmp(sqlState,

>ERRCODE_UNDEFINED_TABLE) != 0))

>+                    {

>+

>+                                fprintf(stderr, _("%s: vacuuming of

>database \"%s\" failed: %s"),

>+                                                                    progname, dbname, PQerrorMessage

>(conn));

>Indentation on both places is wrong.  Check other palces for

>similar issues.

 

FIXED

 

>4.

>+                                                        bool analyze_only, bool freeze, int numAsyncCons,

>In code still there is reference to AsyncCons, as decided lets

>change it to concurrent_connections | conc_cons

 

FIXED

 

Regards,

Dilip

 

pgsql-hackers by date:

Previous
From: Dilip kumar
Date:
Subject: Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]
Next
From: Feike Steenbergen
Date:
Subject: Re: Add regression tests for autocommit-off mode for psql and fix some omissions