Thread: BUG #4952: commit_delay ignored because CountActiveBackends always returns zero
BUG #4952: commit_delay ignored because CountActiveBackends always returns zero
From
"Jeff Janes"
Date:
The following bug has been logged online: Bug reference: 4952 Logged by: Jeff Janes Email address: jeff.janes@gmail.com PostgreSQL version: 8.4.0 Operating system: Linux 2.4.21-15.0.3.ELsmp Description: commit_delay ignored because CountActiveBackends always returns zero Details: I've found that commit_delay never has an effect. By instrumenting src/backend/access/transam/xact.c, I see that this is because CountActiveBackends always returns zero. This seems to be a bug introduced by: http://archives.postgresql.org/pgsql-committers/2009-03/msg00259.php into file src/backend/storage/ipc/procarray.c I believe the source of the bug is the addition to that file of + if (proc != NULL) + continue; The sense of this test should be inverted, as it is NULLs, not non-nulls that need to be skipped. Due to this test all truly active backends get skipped, so CountActiveBackends() always returns zero. Also, I believe without evidence that the change fails to correct the (hard to reproduce) bug it was originally introduced to correct, as a proc that is NULL does not get skipped, and goes on to be dereferenced. If I change this code to: + if (proc == NULL) + continue; Then I find that commit_delay now does have an effect. (The effect is to make "pgbench -c 15" slower when commit_delay is set to the max value of 100000. This is what I thought would happen, and was surprised when it originally did not.) thanks, Jeff
Re: BUG #4952: commit_delay ignored because CountActiveBackends always returns zero
From
Tom Lane
Date:
"Jeff Janes" <jeff.janes@gmail.com> writes: > This seems to be a bug introduced by: > http://archives.postgresql.org/pgsql-committers/2009-03/msg00259.php > I believe the source of the bug is the addition to that file of > + if (proc != NULL) > + continue; > The sense of this test should be inverted, as it is NULLs, not non-nulls > that need to be skipped. Ooops, how embarrassing. Will fix, thanks for the report! regards, tom lane