Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]

Поиск
Список
Период
Сортировка
От Dilip kumar
Тема Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]
Дата
Msg-id 4205E661176A124FAF891E0A6BA913526637B7FE@szxeml509-mbs.china.huawei.com
обсуждение исходный текст
Ответ на Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]  (Amit Kapila <amit.kapila16@gmail.com>)
Ответы Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]  (Amit Kapila <amit.kapila16@gmail.com>)
Список pgsql-hackers

On 23 November 2014 14:45, Amit Kapila Wrote

 

Thanks a lot for debugging and fixing the issue..

 

>The stacktrace of crash is as below:

 

>#0  0x00000080108cf3a4 in .strlen () from /lib64/libc.so.6

>#1  0x00000080108925bc in ._IO_vfprintf () from /lib64/libc.so.6

>#2  0x00000080108bc1e0 in .__GL__IO_vsnprintf_vsnprintf () from /lib64/libc.so.6

>#3  0x00000fff7e581590 in .appendPQExpBufferVA () from 

>/data/akapila/workspace/master/installation/lib/libpq.so.5

>#4  0x00000fff7e581774 in .appendPQExpBuffer () from 

>/data/akapila/workspace/master/installation/lib/libpq.so.5

>#5  0x0000000010003748 in .run_parallel_vacuum ()

>#6  0x0000000010003f60 in .vacuum_parallel ()

>#7  0x0000000010002ae4 in .main ()

>(gdb) f 5

>#5  0x0000000010003748 in .run_parallel_vacuum ()

 

>So now the real reason here is that the list of tables passed to

>function is corrupted.  The below code seems to be the real

>culprit:

>vacuum_parallel()
>{
>..
>if (!tables || !tables->head)
>{
>SimpleStringList dbtables = {NULL, NULL};
>...
>..
>           tables = &dbtables;
>}
>..
>}

 

>In above code dbtables is local to if loop and code

>is using the address of same to assign it to tables which

>is used out of if block scope, moving declaration to the

>outer scope fixes the problem in my environment.  Find the

>updated patch that fixes this problem attached with this

>mail.  Let me know your opinion about the same.

 

Yes, that’s the reason of corruption, this must be causing both the issues, sending corrupted query to server as well as crash at client side.

 

 

>While looking at this problem, I have noticed couple of other

>improvements:

 

>a. In prepare_command() function, patch is doing init of sql

>buffer (initPQExpBuffer(sql);) which I think is not required

>as both places from where this function is called, it is done by

>caller.  I think this will lead to memory leak.

 

Fixed..

 

>b. In prepare_command() function, for fixed strings you can

>use appendPQExpBufferStr() which is what used in original code

>as well.

 

Changed as per comment..

 

>c. 

>run_parallel_vacuum()

>{

>..

>prepare_command(connSlot[free_slot].connection, full, verbose,
>and_analyze, analyze_only, freeze, &sql);
>
>appendPQExpBuffer(&sql, " %s", cell->val);
>..

>}

 

>I think it is better to end command with ';' by using

>appendPQExpBufferStr(&sql, ";"); in above code.

 

Done

 

Latest patch is attached, please have a look.

 

Regards,

Dilip Kumar

 

 

 

 

Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Andres Freund
Дата:
Сообщение: Re: group locking: incomplete patch, just for discussion
Следующее
От: Amit Kapila
Дата:
Сообщение: Re: TODO : Allow parallel cores to be used by vacuumdb [ WIP ]