MIME-Version: 1.0
References: <20230329.132155.629765142788133576.horikyota.ntt@gmail.com>
 <20230329.173456.1185961934810139447.horikyota.ntt@gmail.com>
 <CAAKRu_Yg4-kWH_EN7wtpm3z9B+H27UTmOxzzYv1Vg=YdZ6v_rw@mail.gmail.com>
 <CAD21AoCGdxYAJHmE_nXw_MFw1+Qtn3vcThRu5FiadukgQnpWeQ@mail.gmail.com>
 <935191FB-083B-4060-89FC-466F61FAA391@yesql.se> <CAAKRu_aoa0YT36q2n3xa9MZYdepeH4Jyf2EVJj=7YdC_1c974Q@mail.gmail.com>
 <CAAKRu_bSsChKsyEy1ZV2XJQ22FoDdA56efjNr9==m2TO94557w@mail.gmail.com>
 <CAD21AoDi4i4tj1Cf9BG0q2iyVhHisQ2xxVBZajEqExX6t6_b3g@mail.gmail.com>
 <CAAKRu_bkg+0WAGTxsy0HP75RzFbPwqz3ctYq1kThjFhSL69upw@mail.gmail.com>
 <1252075.1680547394@sss.pgh.pa.us> <20230403190837.qubpnwugfe2k2g46@awork3.anarazel.de>
 <CAAKRu_ap7nhmqtKQ1TRo3LzHG3FK=YpGO=O0019M3BbN7ysm3A@mail.gmail.com>
 <7E06F879-7E20-4A6A-862F-CA72CDC9A323@yesql.se> <CAAKRu_b1HjGCTsFpUnmwLNS8NeXJ+JnrDLhT1osP+Gq9HCU+Rw@mail.gmail.com>
 <CBF0083E-5893-4B35-9498-142A835D03DC@yesql.se> <CAAKRu_bJ2DJxYoMkyMo9UpoX74LY5d2Jir86QWFUftrGKWP5Bw@mail.gmail.com>
 <CA+TgmoYDYHht9zsHye3tywx7oWGMP4s_6AGefHkDsMuGHLzT7w@mail.gmail.com>
 <CAAKRu_auVL_uE794zwiX6t18LJvKtJ5mrmV9tkbpWzS8AYeChQ@mail.gmail.com>
 <CAAKRu_aX+MLn32kPnAQ30WChbiLTkKKmY9e8+B1ZY1BokpQZYA@mail.gmail.com>
 <CAAKRu_aH-UbvNCQq-EouNUTjtGexQTRVo7ik_vYR4WUocVny7w@mail.gmail.com>
 <CA+TgmoY3EaCz3_7F3+Sb3FUHonf4nxdC-_UWnn6fWEPtXU7t5g@mail.gmail.com>
 <D0A88FB1-56A6-4CD5-B886-8539B7BA7F58@yesql.se> <CAAKRu_YmQg_i=0DBfPzy1MMc2GjfuJFo=0rFv4fCzRQStLhjvw@mail.gmail.com>
 <CAAKRu_beQECOV4WfU6nG0hhEVobCnH88Y8MQi1LrTwMfRyxuCw@mail.gmail.com>
 <5028847C-937A-4CC2-B127-AFF5F9FB79C5@yesql.se> <CAAKRu_Znk6WCCQqEw8hv3+X1Fm8gG0x4KKRAkKmO2JOcA-77-g@mail.gmail.com>
 <2B703533-102A-47E5-A4F7-C6A68C09BB27@yesql.se>
In-Reply-To: <2B703533-102A-47E5-A4F7-C6A68C09BB27@yesql.se>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Fri, 7 Apr 2023 15:52:49 +0900
Message-ID: <CAD21AoBS7o6Ljt_vfqPQPf67AhzKu3fR0iqk8B=vVYczMugKMQ@mail.gmail.com>
Subject: Re: Should vacuum process config file reload more often
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Melanie Plageman <melanieplageman@gmail.com>, Robert Haas <robertmhaas@gmail.com>, 
	Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, 
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, 
	Amit Kapila <amit.kapila16@gmail.com>
Content-Type: multipart/mixed; boundary="00000000000056fcf705f8b97b2a"
Archived-At: <https://www.postgresql.org/message-id/CAD21AoBS7o6Ljt_vfqPQPf67AhzKu3fR0iqk8B%3DvVYczMugKMQ%40mail.gmail.com>
Precedence: bulk

--00000000000056fcf705f8b97b2a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, Apr 7, 2023 at 8:08=E2=80=AFAM Daniel Gustafsson <daniel@yesql.se> =
wrote:
>
> > On 7 Apr 2023, at 00:12, Melanie Plageman <melanieplageman@gmail.com> w=
rote:
> >
> > On Thu, Apr 6, 2023 at 5:45=E2=80=AFPM Daniel Gustafsson <daniel@yesql.=
se> wrote:
> >>
> >>> On 6 Apr 2023, at 23:06, Melanie Plageman <melanieplageman@gmail.com>=
 wrote:
> >>
> >>> Autovacuum workers, at the end of VacuumUpdateCosts(), check if cost
> >>> limit or cost delay have been changed. If they have, they assert that
> >>> they don't already hold the AutovacuumLock, take it in shared mode, a=
nd
> >>> do the logging.
> >>
> >> Another idea would be to copy the values to local temp variables while=
 holding
> >> the lock, and release the lock before calling elog() to avoid holding =
the lock
> >> over potential IO.
> >
> > Good idea. I've done this in attached v19.
> > Also I looked through the docs and everything still looks correct for
> > balancing algo.
>
> I had another read-through and test-through of this version, and have app=
lied
> it with some minor changes to comments and whitespace.  Thanks for the qu=
ick
> turnaround times on reviews in this thread!

Cool!

Regarding the commit 7d71d3dd08, I have one comment:

+       /* Only log updates to cost-related variables */
+       if (vacuum_cost_delay =3D=3D original_cost_delay &&
+           vacuum_cost_limit =3D=3D original_cost_limit)
+           return;

IIUC by default, we log not only before starting the vacuum but also
when changing cost-related variables. Which is good, I think, because
logging the initial values would also be helpful for investigation.
However, I think that we don't log the initial vacuum cost values
depending on the values. For example, if the
autovacuum_vacuum_cost_delay storage option is set to 0, we don't log
the initial values. I think that instead of comparing old and new
values, we can write the log only if
message_level_is_interesting(DEBUG2) is true. That way, we don't need
to acquire the lwlock unnecessarily. And the code looks cleaner to me.
I've attached the patch (use_message_level_is_interesting.patch)

Also, while testing the autovacuum delay with relopt
autovacuum_vacuum_cost_delay =3D 0, I realized that even if we set
autovacuum_vacuum_cost_delay =3D 0 to a table, wi_dobalance is set to
true. wi_dobalance comes from the following expression:

        /*
         * If any of the cost delay parameters has been set individually fo=
r
         * this table, disable the balancing algorithm.
         */
        tab->at_dobalance =3D
            !(avopts && (avopts->vacuum_cost_limit > 0 ||
                         avopts->vacuum_cost_delay > 0));

The initial values of both avopts->vacuum_cost_limit and
avopts->vacuum_cost_delay are -1. I think we should use ">=3D 0" instead
of "> 0". Otherwise, we include the autovacuum worker working on a
table whose autovacuum_vacuum_cost_delay is 0 to the balancing
algorithm. Probably this behavior has existed also on back branches
but I haven't checked it yet.


Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

--00000000000056fcf705f8b97b2a
Content-Type: application/octet-stream; name="fix.patch"
Content-Disposition: attachment; filename="fix.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lg66w1db1>
X-Attachment-Id: f_lg66w1db1

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3Bvc3RtYXN0ZXIvYXV0b3ZhY3V1bS5jIGIvc3JjL2Jh
Y2tlbmQvcG9zdG1hc3Rlci9hdXRvdmFjdXVtLmMKaW5kZXggNTNjOGY4ZDc5Yy4uMjAzNmIzOWFk
NSAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcG9zdG1hc3Rlci9hdXRvdmFjdXVtLmMKKysrIGIv
c3JjL2JhY2tlbmQvcG9zdG1hc3Rlci9hdXRvdmFjdXVtLmMKQEAgLTI5NTEsOCArMjk1MSw4IEBA
IHRhYmxlX3JlY2hlY2tfYXV0b3ZhYyhPaWQgcmVsaWQsIEhUQUIgKnRhYmxlX3RvYXN0X21hcCwK
IAkJICogdGhpcyB0YWJsZSwgZGlzYWJsZSB0aGUgYmFsYW5jaW5nIGFsZ29yaXRobS4KIAkJICov
CiAJCXRhYi0+YXRfZG9iYWxhbmNlID0KLQkJCSEoYXZvcHRzICYmIChhdm9wdHMtPnZhY3V1bV9j
b3N0X2xpbWl0ID4gMCB8fAotCQkJCQkJIGF2b3B0cy0+dmFjdXVtX2Nvc3RfZGVsYXkgPiAwKSk7
CisJCQkhKGF2b3B0cyAmJiAoYXZvcHRzLT52YWN1dW1fY29zdF9saW1pdCA+PSAwIHx8CisJCQkJ
CQkgYXZvcHRzLT52YWN1dW1fY29zdF9kZWxheSA+PSAwKSk7CiAJfQogCiAJaGVhcF9mcmVldHVw
bGUoY2xhc3NUdXApOwo=
--00000000000056fcf705f8b97b2a
Content-Type: application/octet-stream; 
	name="use_message_level_is_interesting.patch"
Content-Disposition: attachment; 
	filename="use_message_level_is_interesting.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_lg66w1d90>
X-Attachment-Id: f_lg66w1d90

ZGlmZiAtLWdpdCBhL3NyYy9iYWNrZW5kL3Bvc3RtYXN0ZXIvYXV0b3ZhY3V1bS5jIGIvc3JjL2Jh
Y2tlbmQvcG9zdG1hc3Rlci9hdXRvdmFjdXVtLmMKaW5kZXggNTNjOGY4ZDc5Yy4uNDQ0OTdmNjcz
NCAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQvcG9zdG1hc3Rlci9hdXRvdmFjdXVtLmMKKysrIGIv
c3JjL2JhY2tlbmQvcG9zdG1hc3Rlci9hdXRvdmFjdXVtLmMKQEAgLTE3ODUsOSArMTc4NSw2IEBA
IEZyZWVXb3JrZXJJbmZvKGludCBjb2RlLCBEYXR1bSBhcmcpCiB2b2lkCiBWYWN1dW1VcGRhdGVD
b3N0cyh2b2lkKQogewotCWRvdWJsZQkJb3JpZ2luYWxfY29zdF9kZWxheSA9IHZhY3V1bV9jb3N0
X2RlbGF5OwotCWludAkJCW9yaWdpbmFsX2Nvc3RfbGltaXQgPSB2YWN1dW1fY29zdF9saW1pdDsK
LQogCWlmIChNeVdvcmtlckluZm8pCiAJewogCQlpZiAoYXZfc3RvcmFnZV9wYXJhbV9jb3N0X2Rl
bGF5ID49IDApCkBAIC0xODIxLDE2ICsxODE4LDExIEBAIFZhY3V1bVVwZGF0ZUNvc3RzKHZvaWQp
CiAJCVZhY3V1bUNvc3RCYWxhbmNlID0gMDsKIAl9CiAKLQlpZiAoTXlXb3JrZXJJbmZvKQorCWlm
IChNeVdvcmtlckluZm8gJiYgbWVzc2FnZV9sZXZlbF9pc19pbnRlcmVzdGluZyhERUJVRzIpKQog
CXsKIAkJT2lkCQkJZGJvaWQsCiAJCQkJCXRhYmxlb2lkOwogCi0JCS8qIE9ubHkgbG9nIHVwZGF0
ZXMgdG8gY29zdC1yZWxhdGVkIHZhcmlhYmxlcyAqLwotCQlpZiAodmFjdXVtX2Nvc3RfZGVsYXkg
PT0gb3JpZ2luYWxfY29zdF9kZWxheSAmJgotCQkJdmFjdXVtX2Nvc3RfbGltaXQgPT0gb3JpZ2lu
YWxfY29zdF9saW1pdCkKLQkJCXJldHVybjsKLQogCQlBc3NlcnQoIUxXTG9ja0hlbGRCeU1lKEF1
dG92YWN1dW1Mb2NrKSk7CiAKIAkJTFdMb2NrQWNxdWlyZShBdXRvdmFjdXVtTG9jaywgTFdfU0hB
UkVEKTsK
--00000000000056fcf705f8b97b2a--