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

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