On 04/05/2017 01:09 PM, Kuntal Ghosh wrote:
On Wed, Apr 5, 2017 at 4:13 PM, Tomas Vondra
<tomas.von...@2ndquadrant.com> wrote:
The comment says that the counters are allowed to overflow, i.e. after a
long uptime you might get these values
parallel_register_count = UINT_MAX + 1 = 1
parallel_terminate_count = UINT_MAX
which is fine, because the C handles the overflow during subtraction and
so
parallel_register_count - parallel_terminate_count = 1
But the assert is not doing subtraction, it's comparing the values
directly:
Assert(parallel_register_count >= parallel_terminate_count);
and the (perfectly valid) values trivially violate this comparison.
Thanks for the explanation. So, we can't use the above assert
statement. Even the following assert statement will not be helpful:
Assert(parallel_register_count - parallel_terminate_count >= 0);
Because, it'll fail to track the scenario when parallel_register_count
is not overflowed, still less than parallel_terminate_count. :(
Actually, that assert would work, because C does handle overflows on uint
values during subtraction just fine. That is,
(UINT_MAX+x) - UINT_MAX = x
Also, the comment about overflows before BackgroundWorkerArray claims this
is the case.
Agreed on the overflowed case. But, my concern is when an overflow has
not yet happened:
Suppose,
uint parallel_register_count = 1; /* Didn't overflow* /
uint parallel_terminate_count = 2; /* Didn't overflow */
Assert(parallel_register_count - parallel_terminate_count >= 0);
We want the assert statement to fail here, but I think it won't since
-1 has a valid representation in unsigned int and it is greater than
0, no?
I don't follow. How exactly do you get into this situation, assuming you
actually enforce the (register_count > terminate_count) invariant
consistently? In the modulo arithmetic of course.
But now that I'm thinking about it, the subtraction actually happens in
unsigned ints, so the result will be unsigned int too, i.e. always >=0.
Whether it makes sense depends on the invariant being true.
But I think this would work:
Assert(parallel_register_count - parallel_terminate_count <=
max_parallel_workers);
If the difference gets 'negative' and wraps around, it'll be close to
UINT_MAX.
But man, this unsigned int arithmetic makes my brain hurt ...
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
