On Tue, Aug 19, 2014 at 4:27 PM, Amit Kapila <amit.kapil...@gmail.com> wrote: > > Few more comments: > Some more comments:
1. I could see one shortcoming in the way the patch has currently parallelize the work for --analyze-in-stages. Basically patch is performing the work for each stage for multiple tables in concurrent connections that seems okay for the cases when number of parallel connections is less than equal to number of tables, but for the case when user has asked for more number of connections than number of tables, then I think this strategy will not be able to use the extra connections. 2. Similarly for the case of multiple databases, currently it will not be able to use connections more than number of tables in each database because the parallelizing strategy is to just use the conncurrent connections for tables inside single database. I am not completely sure whether current strategy is good enough or we should try to address the above problems. What do you think? 3. + do + { + i = select_loop(maxFd, &slotset); + Assert(i != 0); Could you explain the reason of using this loop, I think you want to wait for data on socket descriptor, but why for maxFd? Also it is better if you explain this logic in comments. 4. + for (i = 0; i < max_slot; i++) + { + if (!FD_ISSET(pSlot[i].sock, &slotset)) + continue; + + PQconsumeInput(pSlot[i].connection); + if (PQisBusy(pSlot[i].connection)) + continue; I think it is better to call PQconsumeInput() only if you find connection is busy. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com