Re: [HACKERS] Open 6.5 items

>Tom Lane wrote:
>>
>> Vadim Mikheev <vadim(at)krs(dot)ru> writes:
>> >> If I recall the dynahash.c code correctly, a null return value
>> >> indicates either damage to the structure of the table (ie someone
>> >> stomped on memory that didn't belong to them) or running out of memory
>> >> to add entries to the table. The latter should be impossible if we
>>
>> > Quite different cases and should result in different reactions.
>>
>> I agree; will see about cleaning up hash_search's call convention after
>> 6.5 is done. Actually, maybe I should do it now? I'm not convinced yet
>> whether the reports we're seeing are due to memory clobber or running
>> out of space... fixing this may be the easiest way to find out.
>
>Imho, we have to fix it in some way before 6.5
>Either by changing dynahash.c (to return 0x1 if table is
>corrupted and 0x0 if out of space) or by changing
>elog(NOTICE) to elog(ERROR).
>
>>
>> > #define NLOCKS_PER_XACT 40
>> > ^^
>> > Isn't it too low?
>>
>> You tell me ... that was the number that was in the 6.4 code, but I
>> have no idea if it's right or not. (Does MVCC require more locks
>> than the old stuff?) What is a good upper bound on the number
>> of concurrently existing locks?
>
>Probably yes, because of writers can continue to work and lock
>other tables instead of sleeping of first lock due to concurrent
>select. I'll change it to 64, but this should be configurable
>thing.
>
>>
>> > /* xidHash table */
>> > size += hash_estimate_size(maxBackends,
>> > ^^^^^^^^^^^
>> > SHMEM_XIDTAB_KEYSIZE,
>> > SHMEM_XIDTAB_DATASIZE);
>>
>> > Why just maxBackends is here? NLOCKENTS should be used too
>> > (each transaction lock requieres own xidhash entry).
>>
>> Should it be NLOCKENTS(maxBackends) xid entries, or do you mean
>> NLOCKENTS(maxBackends) + maxBackends? Feel free to stick in any
>> estimates that you like better --- what's there now is an interpretation
>> of what the 6.4 code was trying to do (but it was sufficiently buggy and
>> unreadable that it was probably coming out with different numbers in
>> the end...)
>
>Just NLOCKENTS(maxBackends) - I'll change it now.

I have just done cvs update and saw your changes. I tried the same
testing as I did before (64 conccurrent connections, and each
connection excutes 100 transactions), but it failed again.

(1) without -B 1024, it failed: out of free buffers: time to abort!

(2) with -B 1024, it went into stuck spin lock

So I looked into sources a little bit, and made a minor change to
include/storage/lock.h:

#define INIT_TABLE_SIZE 100

to:

#define INIT_TABLE_SIZE 4096

then restarted postmaster with -B 1024 (this will prevent
out-of-free-buffers problem, I guess). Now everything seems to work
great!

I suspect that huge INIT_TABLE_SIZE prevented dynamic expanding the
hash tables and seems there's something wrong in the routines
responsible for that.

Comments?
--
Tatsuo Ishii