On Sat, Oct 25, 2014 at 5:52 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > > > *************** > *** 358,363 **** handle_sigint(SIGNAL_ARGS) > --- 358,364 ---- > > /* Send QueryCancel if we are processing a database query */ > if (cancelConn != NULL) > { > + inAbort = true; > if (PQcancel(cancelConn, errbuf, sizeof(errbuf))) > fprintf(stderr, _("Cancel request sent\n")); > else > > Do we need to set inAbort flag incase PQcancel is successful? > Basically if PQCancel fails due to any reason, I think behaviour > can be undefined as the executing thread can assume that cancel is > done. > > *** 391,396 **** consoleHandler(DWORD dwCtrlType) > --- 392,399 ---- > EnterCriticalSection > (&cancelConnLock); > if (cancelConn != NULL) > { > + inAbort = > true; > + > > You have set this flag in case of windows handler, however the same > is never used incase of windows, are you expecting any use of this > flag for windows?
Going further with verification of this patch, I found below issue: Run the testcase.sql file at below link: http://www.postgresql.org/message-id/4205e661176a124faf891e0a6ba9135266347...@szxeml509-mbs.china.huawei.com ./vacuumdb --analyze-in-stages -j 8 -d postgres Generating minimal optimizer statistics (1 target) Segmentation fault Server Log: ERROR: syntax error at or near "minimal" at character 12 STATEMENT: ANALYZE ng minimal optimizer statistics (1 target) LOG: could not receive data from client: Connection reset by peer Fixed below issues and attached an updated patch with mail: 1. make check for docs gives below errors: { \ echo "<!ENTITY version \"9.5devel\">"; \ echo "<!ENTITY majorversion \"9.5\">"; \ } > version.sgml '/usr/bin/perl' ./mk_feature_tables.pl YES ../../../src/backend/catalog/sql_feature_packages.txt ../../../src/backend/catalog/sql_features.txt > features-supported.sgml '/usr/bin/perl' ./mk_feature_tables.pl NO ../../../src/backend/catalog/sql_feature_packages.txt ../../../src/backend/catalog/sql_features.txt > features-unsupported.sgml '/usr/bin/perl' ./generate-errcodes-table.pl ../../../src/backend/utils/errcodes.txt > errcodes-table.sgml onsgmls -wall -wno-unused-param -wno-empty -wfully-tagged -D . -D . -s postgres.sgml onsgmls:ref/vacuumdb.sgml:224:15:E: end tag for "LISTITEM" omitted, but OMITTAG NO was specified onsgmls:ref/vacuumdb.sgml:209:8: start tag was here onsgmls:ref/vacuumdb.sgml:224:15:E: end tag for "VARLISTENTRY" omitted, but OMITTAG NO was specified onsgmls:ref/vacuumdb.sgml:206:5: start tag was here onsgmls:ref/vacuumdb.sgml:224:15:E: end tag for "VARIABLELIST" omitted, but OMITTAG NO was specified onsgmls:ref/vacuumdb.sgml:79:4: start tag was here onsgmls:ref/vacuumdb.sgml:225:18:E: end tag for element "LISTITEM" which is not open onsgmls:ref/vacuumdb.sgml:226:21:E: end tag for element "VARLISTENTRY" which is not open onsgmls:ref/vacuumdb.sgml:228:18:E: document type does not allow element "VARLISTENTRY" here; assuming missing "VARIABLELIST" start-tag onsgmls:ref/vacuumdb.sgml:260:9:E: end tag for element "PARA" which is not open make: *** [check] Error 1 Fixed. 2. Below multi-line comment is wrong: /* Otherwise, we got a stage from vacuum_all_databases(), so run * only that one. */ Fixed. 3. fprintf(stderr, _("%s: vacuuming of database \"%s\" failed: %s"), progname, dbname, PQerrorMessage(conn)); indentation of fprintf is not proper. Fixed. 4. /* This can only happen if user has sent the cacle request using * Ctrl+C, Cancle is handled by 0th slot, so fetch the error result */ spelling of cancel is wrong and multi-line comment is not proper. Fixed 5. /* This can only happen if user has sent the cacle request using * Ctrl+C, Cancle is handled by 0th slot, so fetch the error result */ GetQueryResult(pSlot[0].connection, dbname, progname, completedb); indentation of completedb parameter is wrong. Fixed. 6. /* * vacuum_parallel * This function will open the multiple concurrent connections as * suggested by used, it will derive the table list using server call * if table list is not given by user and perform the vacuum call */ s/used/user Fixed. In general, I think you can once go through all the comments and code to see if similar issues exist at other places as well. I have done some performance test with the patch, data for which is as below: Performance Data ------------------------------ IBM POWER-7 16 cores, 64 hardware threads RAM = 64GB max_connections = 128 checkpoint_segments=256 checkpoint_timeout =15min shared_buffers = 1GB Before each test, run the testcase.sql file at below link: http://www.postgresql.org/message-id/4205e661176a124faf891e0a6ba9135266347...@szxeml509-mbs.china.huawei.com Un-patched - time ./vacuumdb -t t1 -t t2 -t t3 -t t4 -t t5 -t t6 -t t7 -t t8 -d postgres real 0m2.454s user 0m0.002s sys 0m0.006s Patched - time ./vacuumdb -t t1 -t t2 -t t3 -t t4 -t t5 -t t6 -t t7 -t t8 -j 4 -d postgres real 0m1.691s user 0m0.001s sys 0m0.004s time ./vacuumdb -t t1 -t t2 -t t3 -t t4 -t t5 -t t6 -t t7 -t t8 -j 8 -d postgres real 0m1.496s user 0m0.002s sys 0m0.004s Above data indicates that the patch improves performance when used with more number of concurrent connections. I have done similar tests for whole database as well and the results are quite similar to above. I think you can once run the performance test for --analyze-in-stages option as well after fixing the issue reported above in this mail. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
vacuumdb_parallel_v16.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers