| From: | Manu <manuelreyesbravo(at)gmail(dot)com> |
|---|---|
| To: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com> |
| Cc: | Daniel Gustafsson <daniel(at)yesql(dot)se>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Nikolay Samokhvalov <nik(at)postgres(dot)ai>, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: autovacuum: automatically propagate updated parameters |
| Date: | 2026-09-24 21:35:36 |
| Message-ID: | 179028573649.493682.4321152895319402838@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
Hi,
I ran v3 on REL_19_STABLE (e60ee52841d) with --enable-cassert and
--enable-injection-points, next to Nikolay's v2 and two cut-down copies
of v3, to check the new tests and the polling question from upthread.
1. Each part of the fix has a test that fails without it
v3 tests and injection points, no fix: tests 1-3 pass, 4 fails
v3 without the SetLatch() in
autovac_recalculate_workers_for_balance(): tests 1-4 pass, 5 fails
v3: all pass
2. Stability
test_autovacuum passed 30 of 30 runs on an idle machine, and 20 of 20
with every CPU busy (one busy loop per core), at about 10 s and 16 s a
run.
3. A case the tests don't cover: a worker leaving the balance
Test 5 checks a worker joining while the leader waits. A worker
leaving goes through another path: FreeWorkerInfo() sets
AutoVacRebalance, and the launcher recalculates the count and so does
the SetLatch(). I added it as test 6 (attached, on top of v3): the
second worker joins and is held as in test 5, the leader goes to 400,
then the second worker is released and exits. With v3 the leader,
still in ParallelFinish, logs cost_limit=800 by the first check after
the worker is gone, and the parallel worker ends at 800.
Without the SetLatch() the test stops earlier, at the join, like test
5, so it does not tell the two paths apart. It does show that the
launcher path works, which the current tests don't exercise.
4. Polling vs. waking, measured
With the parallel worker held before its index and the leader waiting
for it for 20 s, the leader's voluntary context switches were:
v3: 0
Nikolay's v2: 199 (9.9 per second)
The time from pg_reload_conf() to the leader logging the new limit was
7 ms with v3 and 8 ms with v2. So the timed wait costs about ten
wakeups a second for as long as the largest index takes, and makes no
difference to how fast a reload is picked up.
5. Things I checked in the code, in case they come up
- The extra SetLatch() does not cut short a busy worker's cost delay:
vacuum_delay_point() sleeps with pg_usleep(), not on the latch, so
"a worker that is busy vacuuming ignores the extra wakeup" holds.
- No wakeup is lost in the wait. The refresh runs at the top of each
pass, before WaitLatch(). A SetLatch() after it makes WaitLatch()
return at once, and one between WaitLatch() and ResetLatch() is
followed by another pass that reads the current count anyway, since
the count is written before the SetLatch().
- The leader also wakes for its workers' messages, but
parallel_vacuum_propagate_shared_delay_params() bumps the generation
only when a value changed, so those wakeups make no worker re-read
its parameters.
The scripts and all outputs are in the second attachment.
Regards,
Manu
| Attachment | Content-Type | Size |
|---|---|---|
| nocfbot-v3-add-test-worker-leaves.diff.txt | text/plain | 3.4 KB |
| nocfbot-av-cost-wait-review.txt | text/plain | 12.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Michael Paquier | 2026-09-24 22:52:07 | Re: BUG #19715: pg_restore_attribute_stats() rejects range statistics for a domain over int4multirange |
| Previous Message | Manu | 2026-09-24 20:49:27 | Re: BUG #19695: JSON_VALUE ... RETURNING jsonb returns NULL for later evaluation once one evaluation returns NULL |